-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Breaking: improve plugin resolving (refs eslint/rfcs#47) #12922
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to make changes to CLIEngine
now that we are deprecating it?
Yes.
|
It looks like tests might be failing due to the issue described here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small comment about the documentation formatting, but otherwise LGTM!
Co-Authored-By: Kai Cataldo <kai@kaicataldo.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you for the thorough tests - they really helped!
Thanks for working on this! |
This PR utterly broke an important use case for me. Can there be a fix/patch that optionally restores the original behavior? We have an organization-wide config that exists specifically so that people don't need to explicitly import all of the plugins in their local project. The command line option doesn't really help, but a flag in the local project's root config might. |
No, we cannot add the option that changes how ESLint loads configuration files, into configuration files. You can restore the original behavior by I'd recommend installing plugins into the same place as the config file that uses it. Otherwise, looks like And, it's good practice if your project configuration files include |
That's the problem. We need to primarily use the central configuration (and plugins), but still allow each project to add their own additional ones if they choose. And, if we add a plugin to the central configuration, we need that to propagate to all the projects without requiring them to change their local .eslintrc.js file. Can there be a way to specify two directories in |
You can use
No. Previously, eslint loads all plugins from cwd. The |
Thank you for being patient, @mysticatea. I'll try using the command line option, and see if it works for what I need. |
Note that if you use the Yarn workspaces, you can make a workspace package containing your shared configuration, and make everything else depend on it. This way your shared plugins are properly scoped to your conf, and individual consumers can add their own additions if needed. |
Prerequisites checklist
What is the purpose of this pull request? (put an "X" next to an item)
[X] Other, please explain: change existing core features
What changes did you make? (Give an overview)
This PR implements a part of RFC47.
d29f613 deprecatesCLIEngine::getRules()
and addsCLIEngine::getRulesForFile(filePath)
. (link)0d6afaf deprecatesmetadata.rulesMeta
and addsmetadata.getRuleMeta(ruleId, filePath)
. (link)f3cc32f deprecatesreport.usedDeprecatedRules
and addsreport.results[i].usedDeprecatedRules
. (this is not in RFC47, but is a part of RFC40. I found thatreport.usedDeprecatedRules
requires the plugin uniqueness over multiple target files, but RFC47 overlooked it. Fortunately, RFC40 mentions the move ofusedDeprecatedRules
.)Is there anything you'd like reviewers to focus on?
Nothing in particular.