feat: ESLint class migration #11
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Refactor the linter method to run asynchronously because many methods have been changed to run asynchronously in the new ESLint class. Preperation for migration to the ESLint class, see webpack-contrib#10.
Refactor to asynchronous formatter loading because the new ESLint class loads formatters asynchronously. Move formatter loading to the linter file for access to the ESLint instance because the new ESLint loadFormatter method is not static. Refactor formatter loading to use a CLIEngine instance. Refactor the linting function to load the formatters. Preperation for migration to the ESLint class, see webpack-contrib#10.
Refactor to filter out loader options before passing the options to ESLint because the new ESLint class throws when unknown options are passed to it. Add method for filtering out loader options and unit tests for it. Rename the getOptions file options because it now contains option utilities. It was possible to have getESLintOptions as a private function in getCLIEngine, but having it exported helps testing. There is no way to query the ESLint class for the options it received so there is no way to test it through the getCLIEngine/ESLint method. Part of the migration to the new ESLint class, see webpack-contrib#10.
Refactor to simplify and remove unnecessary operations. After the formatter loading refactor the getCLIEngine is no longer used without starting a CLIEngine. Remove no longer used options parameter reassignments. Remove unused eslintPath property from the return object. Preperation for migration to the ESLint class, see webpack-contrib#10.
Refactor the custom formatter tests to use a mock. Refactor to test the error output to make sure the selected formatter is being used. The test used to always pass because formatter loading fails silently and returns the default formatter. Add a mock formatter. Remove eslint-friendly-formatter that was only used for the test. This refactor helps decouple the tests from the results format. Making the migration to the new ESLint class easier. Preperation for migration to the ESLint class, see webpack-contrib#10.
Update ESLint development dependency to version 7 to prepare for migration to the new ESLint class that was introduced with it. Part of the migration to the new ESLint class, see webpack-contrib#10.
Refactor the ESLint mock to imitate the new ESLint class. Refactor to simplify mock formatter loading. Refactor ESLint options because the new API introduces a new options format. The tests do not pass yet as the plugin has not been refactored. Part of the migration to the new ESLint class, see webpack-contrib#10.
Refactor linter to the new ESLint class. Refactor the getCLIEngine method to getESLint. Refactor LintDirtyModulesPlugin to remove use of resolveFileGlobPatterns which is not available in the new API. File paths should work fine as globs and do not need to be converted. Completing the migration to the new ESLint class, see webpack-contrib#10.
Update plugin ESLint peer dependency to remove version 6 support. Update the readme to reflect the changes to the plugin. Add warning that the plugin used to use CLIEngine in version 1. Finishing details of the ESLint class migration. Closes webpack-contrib#10.
|
|
Codecov Report
@@ Coverage Diff @@
## master #11 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 7 7
Lines 154 155 +1
Branches 37 41 +4
=========================================
+ Hits 154 155 +1
Continue to review full report at Codecov.
|
ricardogobbosouza
requested changes
Jun 14, 2020
src/LintDirtyModulesPlugin.js
Outdated
| .resolveFileGlobPatterns(dirtyOptions.files) | ||
| .join('|') | ||
| .replace(/\\/g, '/'); | ||
| const glob = dirtyOptions.files.join('|').replace(/\\/g, '/'); |
There was a problem hiding this comment.
Thank you for the links, I will investigate further.
There was a problem hiding this comment.
Quick update as I have been silent all week. Thank you for pointing out the issue.
It is indeed causing an issue when watching folder paths with the lintDirtyModulesOnly option enabled.
I have added tests and reimplemented the functionality of resolveFileGlobPatterns. I am still refactoring and rebasing, it should be ready for review in a day or two.
Refactor to remove an unnecessary callback from the linter function. It is unnecessary because linter is an async function returning a promise so it can be used directly with a promise tap. Preparation for requested change in webpack-contrib#11.
LKummer
added a commit
to LKummer/eslint-webpack-plugin
that referenced
this pull request
Jun 21, 2020
Refactor LintDirtyModulesPlugin to only handle dirty file filtering. The refactor allows for easier testing without the need for mocks. Rename LintDirtyModulesPlugin to DirtyFileWatcher to make the name more descriptive of the new functionality. Refactor the tests for the simplified DirtyFileWatcher. Refactor the tests to use absolute paths because Webpack passes absolute paths in the compiler.fileTimestamps map. Refactor to only test system specific paths. The functionality of the requested change (webpack-contrib#11) requires using real paths for some tests which causes issues with testing both Windows and Unix paths on the same machine. As the CI runs on Windows, Mac and Linux paths will be tested on all platforms. Preparation for requested change in webpack-contrib#11.
LKummer
added a commit
to LKummer/eslint-webpack-plugin
that referenced
this pull request
Jun 21, 2020
Add failing tests that ensure dirty file watching is matching patterns from the files options exactly like ESLint. Add folder fixture because handling folder paths requires checking existence of folders. Preparation for requested change in webpack-contrib#11.
LKummer
added a commit
to LKummer/eslint-webpack-plugin
that referenced
this pull request
Jun 21, 2020
Fix issue with folder patterns not working with the lintDirtyModulesOnly option. The issue was caused by the removal of resolveFileGlobPatterns which resolved folder patterns into globs matching the correct file extensions. Resolve requested change in webpack-contrib#11.
Refactor LintDirtyModulesPlugin to only handle dirty file filtering. The refactor allows for easier testing without the need for mocks. Rename LintDirtyModulesPlugin to DirtyFileWatcher to make the name more descriptive of the new functionality. Refactor the tests for the simplified DirtyFileWatcher. Refactor the tests to use absolute paths because Webpack passes absolute paths in the compiler.fileTimestamps map. Refactor to only test system specific paths. The functionality of the requested change requires using real paths for some tests which causes issues with testing both Windows and Unix paths on the same machine. As the CI runs on Windows, Mac and Linux paths will be tested on all platforms. Preparation for requested change in webpack-contrib#11.
Add failing tests that ensure dirty file watching is matching patterns from the files options exactly like ESLint. Add folder fixture because handling folder paths requires checking existence of folders. Preparation for requested change in webpack-contrib#11.
Fix issue with folder patterns not working with the lintDirtyModulesOnly option. The issue was caused by the removal of resolveFileGlobPatterns which resolved folder patterns into globs matching the correct file extensions. Resolve requested change in webpack-contrib#11.
Refactor remaining callbacks to async functions to make them consistent with the rest of the taps that have been refactored.
Refactor the linter function to register hooks with the same plugin object that is used to register the run and watchRun hooks.
Refactor to parse patterns only on startup instead of parsing them to globs on each run. In watch mode Webpack does not watch it's own configuration so the plugin configuration does not change when Webpack is running. Because the file patterns and extensions do not change there is no need to process them into globs separately for each run.
Add more information about the files option to the readme.
LKummer
force-pushed
the
feat-eslint-class
branch
from
b5a40ba
to
80b82e6
Compare
|
I have performed the requested change, as well as some more refactoring and a small readme addition. It is ready for review. (sorry for the force push, I missed a commit message warning) |
|
/cc @ricardogobbosouza friendly ping |
|
Thanks @LKummer |
|
Thank you @ricardogobbosouza for merging. Thank you very much @evilebottnawi for the ping. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR contains a:
Motivation / Use-Case
The CLIEngine class has been deprecated.
This PR refactors the plugin to the new ESLint class.
Closes #10.
Breaking Changes
For more information on the new ESLint options, see the ESLint documentation.
When using deprecated options ESLint will error and suggest how to change the options to the new format.
Additional Info
Changes introduced in the new ESLint API that affect the plugin:
optionshas changed and is incompatible withCLIEngineoptions.optionsis validated to not contain extra properties.isPathIgnored()is now asynchronous.resolveFileGlobPatterns()has been removed.format()method.The commits form a list of everything that I have refactored with more detailed information.
I have also updated the peer dependency and the readme.
Please mention if you find anything I have missed.