eslint-webpack-plugin
eslint-webpack-plugin copied to clipboard
webpack-contrib
Module lint errors are reported when module is not included in webpack bundle
- Operating System: N/R
- Node Version: N/R
- NPM Version: N/R
- webpack Version: N/R
- eslint-webpack-plugin Version: 2.4.3 (the same issue occurs on branch pref-use-finish-modules)
Let's assume scenario: webpack entry point: index.js
- index.js
- lintError.js
- index.js:
import "lintError.js";
webpack is erroring with lintError.js's linting error
- let's change index.js (lintErrorjs.js is no longer imported in the bundle):
// import "lintErrorjs";
again webpack is erroring with lintError.js's linting error
Expected Behavior / Situation
after index.js is changed (2) webpack shouldn't throw error
Actual Behavior / Situation
webpack is throwing an error
Modification Proposal
the issue is this line: https://github.com/webpack-contrib/eslint-webpack-plugin/blob/4daddf5335b2c78203482d7e7f6d82a909277212/src/linter.js#L84
I propose getting rid of cache in src/linter.js, as we are not using it as cache at all, maybe create a real caching mechanism
the issue is this line: https://github.com/webpack-contrib/eslint-webpack-plugin/blob/4daddf5335b2c78203482d7e7f6d82a909277212/src/linter.js#L84
I propose getting rid of cache in src/linter.js, as we are not using it as cache at all, maybe create a real caching mechanism
It is being used as a cache. It would be detrimental to performance to remove that... what we really need to do is listen for invalidations and remove entries from there. @ricardogobbosouza
Sorry, but I can't see how it's used as cache:
Firstly we delete all files that need to be linted, in the time of writing this issue this list of files was obtained through compilation.succeedModule so those list contains only modules that have changed. https://github.com/webpack-contrib/eslint-webpack-plugin/blob/4daddf5335b2c78203482d7e7f6d82a909277212/src/linter.js#L59-L61
Then we save the results of lining into it. https://github.com/webpack-contrib/eslint-webpack-plugin/blob/4daddf5335b2c78203482d7e7f6d82a909277212/src/linter.js#L80-L82
And we retrieve ALL of the results: https://github.com/webpack-contrib/eslint-webpack-plugin/blob/4daddf5335b2c78203482d7e7f6d82a909277212/src/linter.js#L84
As you can see there is no caching involved, so completely removing crossRunResultStorage won't hurt performance at all. From my perspective it's only holding results of files that have been linted in the past but haven't changed.
Of course changing the webpack hook to use compilation.finishModules implies creating some persistant cache because finishModules returns list of all modules, this is what we are tying to do in #79
@watjurk Yes, in the current state of the plugin, it is useless. However, the plugin is also currently in a performance regression. See the state of it before 2.5.0...
When I originally authored this cache, the plugin ran eslint on files as modules were discovered in a worker pool. The initial compile had all results stored so that on invalidations we only would re-run the linter on changed files... the results from prior runs completed the report so that watch mode didn't drop old errors/warnings that had yet to be fixed.
See #83
I've seen this issue I've tied to come with a solution and this is what I came up with: https://github.com/webpack-contrib/eslint-webpack-plugin/pull/79#issuecomment-778366981.
See #83
I've seen this issue I've tied to come with a solution and this is what I came up with: #79 (comment).
Yes, I see that... I'm not sure what the impetus of the original changes were, but If we roll back and just add the finishModules when threading is false, and use succeedModule when threading, you just have to delete entries for invalidations from invalid.tap (I didn't know of that event at the time of implementing the result storage)
Honestly, I don't care how it's done... I just care that when threading is on, linting happens in parallel to the build as files are discovered... my build goes from 90s on 2.4.3 to 200+s on 2.5.x.
Okay, sorry for your build times, I'll try getting #79 fixes and merged as fast as possible.
@jsg2021 The original issue https://github.com/webpack-contrib/eslint-webpack-plugin/issues/74#issue-801635525, this is still happening.
Ah, I get the scenario now... an error in a file that is not fixed, but instead removed from the graph ...
This is because the results are cached (by file) and only invalidated when that file is changed... if a link in the chain to that file is removed, the plugin does not know the error stored from prior runs has been resolved by removal.
I'm not sure how to tackle this efficiently... Maybe keep a list of files from each run and purge entries from the cache that are not in the graph? (or don't cache, but that would have severe effects on performance.)
This issue was discussed on: #73 #78 #79 So meaby there are some possible solutions