Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot use ag-grid with ts-loader >= 0.9.3 #388

Closed
Wykks opened this issue Nov 21, 2016 · 9 comments
Closed

Cannot use ag-grid with ts-loader >= 0.9.3 #388

Wykks opened this issue Nov 21, 2016 · 9 comments

Comments

@Wykks
Copy link

Wykks commented Nov 21, 2016

When using ag-grid and ts-loader >= 0.9.3 with allowJs set to true, I get lots of error like this :

Module '"[...]/node_modules/ag-grid/main"' has no exported member '[...]'.

It's fine with ts-loader 0.9.2

Maybe it's related to this issue: #250
Since ag-grid use the typings field as well https://github.com/ceolter/ag-grid/blob/master/package.json#L9

@johnnyreilly
Copy link
Member

Since 0.9.3 is when we added support for allowJs I would guess it's related to that. Have you tried setting allowJs to false in your tsconfig.json?

@Wykks
Copy link
Author

Wykks commented Nov 22, 2016

Unfortunately, I can't do that, because my project is too big, and rely on this option for now.
It generate a lots of error. No ag-grid related error though, but i'm not sure if it's because it skip some files.

@johnnyreilly
Copy link
Member

Do you want to try something for me? If you tweak this line:

from:

    // if allowJs is set then we should accept js(x) files
    const scriptRegex = configFile.config.compilerOptions.allowJs
        ? /\.tsx?$|\.jsx?$/i
        : /\.tsx?$/i;

to:

    const scriptRegex = /\.tsx?$/i;

does it resolve your issue?

@Wykks
Copy link
Author

Wykks commented Nov 28, 2016

Yes, it works fine with this change (using v1.2.2)

@johnnyreilly
Copy link
Member

OK. So that's a little frustrating. Essentially, when allowJs is set to true we allow both both js(x) and ts(x) files to be processed. For most people this works in the way they would hope. Not in your case though; I'm not sure why.

I'd possibly be open to supporting this behaviour in some way in ts-loader but there's a couple of things we'd need first:

  1. A simple repro of the issue. That repro should be turned into an integration test that becomes part of our testpack. With the tweak in place the test passes, without it, it fails.
  2. We then need to think of how this is wisely implemented. It would be possible to just stick this behind a flag and have done but I'm not keen. I want to understand what the issue is. I'd really rather have allowJs just work. If we need to add an extra flag to cater for this we need to have a good understanding of why and, crucially, a clear name for it so other people picking over the issue will "fall into the pit of success".

Over to you!

@johnnyreilly
Copy link
Member

Related to #316 cc @dschnare

@Wykks
Copy link
Author

Wykks commented Dec 2, 2016

Here you go #397
Probably not clean as I do not fully understand your test system, but it does reproduce the issue

@johnnyreilly
Copy link
Member

I'm waiting for input from @pqr but it seems that the change introduced in 0.9.3 may have been problematic. The behaviour introduced there (details here #316 ) effectively caters for an edge case: when the entry point is a js file. Because this is an edge case and also because it seems to be causing problems for others I'm considering putting the new behaviour behind a flag. That will make it still available for @dschnare and others that rely on it. I want a little more data before I commit to this but I wanted to let you know what I was thinking.

@johnnyreilly
Copy link
Member

johnnyreilly commented Dec 5, 2016

Should be resolved with 1.3.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants