Lint only the imported files #28
Conversation
jsg2021
changed the title
Codecov Report
@@ Coverage Diff @@
## master #28 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 8 7 -1
Lines 159 168 +9
Branches 39 41 +2
=========================================
+ Hits 159 168 +9
Continue to review full report at Codecov.
|
|
I went ahead and just refactored this to run on the import graph. So, not running on all files in the project/context could be considered a breaking change. |
jsg2021
changed the title
|
/cc @evilebottnawi |
This comment has been minimized.
This comment has been minimized.
| // From my testing of compiler.watch() ... compiler.watching is always | ||
| // undefined (webpack 4 doesn't define it either) I'm leaving it out | ||
| // for now. | ||
| compiler.hooks.watchRun.tapPromise(ESLINT_PLUGIN, this.run); |
There was a problem hiding this comment.
/cc @ricardogobbosouza What do you think about improving it for auto detecting watch only on webpack@5?
There was a problem hiding this comment.
@evilebottnawi Sorry, but I didn't understand your question lol
|
I'm going to do some tests on webpack 4 and 5 |
|
@ricardogobbosouza Feel free to ping me |
|
Hi @jsg2021
|
|
@ricardogobbosouza huh, I didn't expect the that. I've added a test to reproduce it and fixed it. This was caused because I assumed each run call from |
|
Is there a tap option to remove after called? or a way to remove a tap? My current solution is okay, but idk, feels dirty. |
yes should avoid it, it reduces performance |
Is that true for once per-run? If I were to use it, I would only use it at the top so that each top-level run only had one use of this plugin. My current solution has the same effect. Its just a bit hacky in my opinion. |
|
@jsg2021 Apparently now works as expected 😄 @evilebottnawi I wonder if we have to have an option to linter all files and not just imported ones 🤔 |
In my opinion, that's beyond the scope of webpack. Files not touched by the import graph shouldn't touched. If the user wants to lint the entire project then the linter itself can do that as a separate task. |
Ideally yes, but it is edge case |
|
Why is this merge blocked? |
|
Hi @naulacambra |
This is attempting to implement ideas in #23
This PR contains a:
Motivation / Use-Case
Allow the linter to run on only files imported into the module graph. Ignoring any other files.
Breaking Changes
None so far. However, this change will likely make the existing paradigm redundant.
Additional Info
I'm just opening this to get started on it. It's not done.Looking at the original implementation, this feature feels like a mental shift and may make sense to remove the existing "what to lint" logic to further simplify the configuration. idk, thoughts?