Strange "Cannot find module" error when using with TSLint and the no-unused-variable rule about fork-ts-checker-webpack-plugin HOT 18 CLOSED

@andrewbranch I changed the title

from fork-ts-checker-webpack-plugin.

realized the project I was working with extended a few different configs, so I put no-unused-variable in as false and its working.

from fork-ts-checker-webpack-plugin.

I stepped through the code and it looks like it happens for packages that specify typings in their package.json. Because no-unused-variable pollutes the module resolution cache in SourceFile instances, if they are reused after linting, and if the set of changed files does not cause typescript to re-resolve these packages, it fails to resolve them altogether.

A quick workaround is to do this:

diff --git src/IncrementalChecker.ts src/IncrementalChecker.ts
index 56fdb70..ca69bda 100644
--- src/IncrementalChecker.ts
+++ src/IncrementalChecker.ts
@@ -115,6 +115,11 @@ class IncrementalChecker {
       return files.getData(filePath).source;
     };
 
+    // after linting, cached `SourceFile` instances may not be safely reused
+    files.keys().forEach(filePath => {
+      files.remove(filePath);
+    });
+
     return ts.createProgram(
       programConfig.fileNames,
       programConfig.options,

But obviously this has some perf impacts. I'm not sure how this can be properly fixed.

from fork-ts-checker-webpack-plugin.

Thank you for reporting this issue with reproduction repo :) I will check it, but I'm busy so it would be probably in the next week. But maybe someone else could help? PR's are welcome :)

from fork-ts-checker-webpack-plugin.

Related tslint issue: palantir/tslint#2763

from fork-ts-checker-webpack-plugin.

Thanks for the information @univerio - as I understand it this only occurs when the no-unused-variable is in play.

It's cool there's a workaround. Given this problem only occurs for the one rule I'm not that keen to use the workaround in all cases. I'm kinda reticent to use it at all given it's working around a problem in another package which might be fixed in future.

That said, I'm wondering if it would be useful to put it behind a flag so you could "opt-in" to that behaviour. It could be driven directly from the tslint.json of course too. But if the problem was fixed upstream in the future then we'd need to ship another version the remove it.

What do you think @piotr-oles ?

from fork-ts-checker-webpack-plugin.

Thanks for looking into this and filing a bug with tslint, @univerio. Speaking for myself, due to this issue and the fact that lint rules which require type checking don’t show up in VSCode, I’m currently opting instead for tsc’s noUnusedLocals and noUnusedParameters. I really hope tslint fixes this rule someday because I want it to be autofixable, but in the meantime there are too many reasons why it’s not worth it.

from fork-ts-checker-webpack-plugin.

@pelotom makes an excellent point; the various tsc noUnused... compiler options make the no-unused-variable less useful. If memory serves, tslint do retire rules when the Typescript compiler offers an "in-the-box" alternative. I wonder if this rule will be retired as a result?

from fork-ts-checker-webpack-plugin.

@johnnyreilly It will not. palantir/tslint#1481

I actually dug around a little more last night, and it seems that the proper fix would be in tslint. The reason I say this is that I've noticed similar behavior in tslint-language-service. I have filed an issue with them with a (seemingly okay) patch. This fix works for tslint-language-service as well.

from fork-ts-checker-webpack-plugin.

As I commented on that issue, I don't want this rule to go away. I just want them to fix it!

from fork-ts-checker-webpack-plugin.

Is there any way a temporary fix could be applied in fork-ts-checker-webpack-plugin? This package is now used as the webpack-loader by react-script-ts which might surface this problem to many, especially new, users.

from fork-ts-checker-webpack-plugin.

@clentfort You're right - I was just affected by this on a new create-react-app project using the new beta version of material-ui (https://material-ui-next.com/). It took me quite a while to troubleshoot it and find this thread.

from fork-ts-checker-webpack-plugin.

Oh, I'm so glad I finally found this—maybe a maintainer would like to add “Cannot find module” to the issue title, because I gave up on this plugin before coming back to it weeks later to discover this.

from fork-ts-checker-webpack-plugin.

so what is the actual fix for this? just ran into it, and am glad to have found the core issue at least.

from fork-ts-checker-webpack-plugin.

@kelly-tock my fix has been to abandon the no-unused-variable lint rule and instead use TypeScript's noUnusedLocals and noUnusedParameters options instead. I still believe this is the kind of thing a linter ought to handle, but at the moment the happy path seems to be letting the compiler do it. This lint rule is just buggy and problematic in a number of different ways and I've seen no indications that it will be fixed soon.

from fork-ts-checker-webpack-plugin.

@kelly-tock The actual problem exists in tslint itself, and, specifically, the way the no-unused-variable rule is implemented. I've filed palantir/tslint#3671, but heard no response.

In the meantime I've resorted to using a local fork of that rule with the patch in the linked issue.

I've also come across the no-unused rule from ajafff/tslint-consistent-codestyle, which is advertised to be immune from this problem, but I have not tested it myself.

from fork-ts-checker-webpack-plugin.

hmmmm...i'm still getting this issue actually.

• from tslint.json I took out the no-unused-variable
• from tsconfig.json I added "noUnusedLocals": true,

I could not turn on noUnusedParameters at this time.

new ForkTsCheckerWebpackPlugin({
      formatter: 'default',
      logger: {
        error: console.error,
        warn: console.warn,
        info: function() {},
      },
      checkSyntacticErrors: true,
      tslint: path.resolve(__dirname, '../tslint.json'),
      memoryLimit: 3072,
    }),

on tslint 5.10.0.

and after I run webpack, make an obvious change to cause lint error, I get

ERROR in /...GoogleMap.tsx(3,64):
TS2307: Cannot find module 'react-google-maps'.
ERROR in /....GoogleMap.tsx(4,23):
TS2307: Cannot find module 'react-google-maps/lib/components/places/SearchBox'.

any thoughts?

from fork-ts-checker-webpack-plugin.

I've noticed that this is only an issue when changing certain top-level files (at least in my current project). When changing most React components it doesn't occur.

from fork-ts-checker-webpack-plugin.