Module lint errors are reported when module is not included in webpack bundle

[watjurk]

• 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

1. index.js:

import "lintError.js";

webpack is erroring with lintError.js's linting error

2. 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:

eslint-webpack-plugin/src/linter.js

Line 84 in 4daddf5

results = Object.values(crossRunResultStorage);

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

[ricardogobbosouza]

Solving it now. =)

[jsg2021]

the issue is this line:

eslint-webpack-plugin/src/linter.js

Line 84 in 4daddf5

results = Object.values(crossRunResultStorage);

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

[watjurk]

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.

eslint-webpack-plugin/src/linter.js

Lines 59 to 61 in 4daddf5

	for (const file of asList(files)) {
	delete crossRunResultStorage[file];
	}

Then we save the results of lining into it.

eslint-webpack-plugin/src/linter.js

Lines 80 to 82 in 4daddf5

	for (const result of results) {
	crossRunResultStorage[result.filePath] = result;
	}

And we retrieve ALL of the results:

eslint-webpack-plugin/src/linter.js

Line 84 in 4daddf5

results = Object.values(crossRunResultStorage);

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

[jsg2021]

@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.

[jsg2021]

See #83

[jsg2021]

Its my opinion that this is still needed, once threading is restored.

[watjurk]

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).

[watjurk]

Sorry that was missclicked...

[jsg2021]

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.

[watjurk]

Okay, sorry for your build times, I'll try getting #79 fixes and merged as fast as possible.

[watjurk]

Hi, any update on the original issue ?

[jsg2021]

Was it not solved?

[ricardogobbosouza]

Not yet @jsg2021 , I will try to look into this soon.

[watjurk]

@jsg2021 The original issue #74 (comment), this is still happening.

[jsg2021]

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.)

[watjurk]

This issue was discussed on: #73 #78 #79
So meaby there are some possible solutions