-
-
Notifications
You must be signed in to change notification settings - Fork 70
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
New: Plugin Loading Improvement #47
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.
General question. Why wouldn't we just use require.resolve
for each plugin loaded in a given config file from the location of the config file? Unless --resolve-plugins-relative-to
is provided.
True, this proposal intends to just use But I thought we should warn confliction. For example, if there are the plugin |
Got it. Thanks for clarification. To be honest, I still think we are creating artificial constraints for ourselves. The whole scenario of using same plugin from multiple places/multiple versions doesn't seem like a real-world issue to me. I'm sure it will happen, but I'm also reasonably sure that it will be very rare and putting those constrains in on every design seems like an overkill to me. |
I believe to print understandable errors are worthful regardless of the frequency, especially if the raw error is too difficult to find the cause. |
This is more a general topic, but I tend to agree with @ilyavolodin. While I agree that helpful error messages should be helpful, I don't think it should be a higher priority than an intuitive user experience. |
Indeed, the shadowing that this RFC describes will not happen frequently. But if it happened, the error will appear as an "invalid option" or "rule not found" error. Please image that people opens an issue with the error. The error is too far from the cause, so I think that the validation in this RFC is worthful. |
@mysticatea Oh, absolutely, I think we are on the same page on this. We should provide helpful error messages to point users to the correct solution (although, I can't say that it's been working out well so far). So just to clarify this RFC suggests using |
Thank you for your feedback.
|
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.
Overall looks good. One question about performance.
This reverts commit 0d6afaf.
This reverts commit d29f613.
* Breaking: change relative paths with --config (refs eslint/rfcs#37) * update docs * Breaking: improve plugin resolving (refs eslint/rfcs#47) * replace getRules by getRulesForFile (refs eslint/rfcs#47) * replace rulesMeta by getRuleMeta (refs eslint/rfcs#47) * replace report.usedDeprecatedRules by report.results[].usedDeprecatedRules * Revert "replace report.usedDeprecatedRules by report.results[].usedDeprecatedRules" This reverts commit f3cc32f. * Revert "replace rulesMeta by getRuleMeta (refs eslint/rfcs#47)" This reverts commit 0d6afaf. * Revert "replace getRules by getRulesForFile (refs eslint/rfcs#47)" This reverts commit d29f613. * update docs * Update docs/user-guide/configuring.md Co-Authored-By: Kai Cataldo <kai@kaicataldo.com> * fix markdownlint error Co-authored-by: Kai Cataldo <kai@kaicataldo.com>
Summary
This RFC changes ESLint to load plugins from the directory of the config files, so it fixes "plugin not found" errors for varied environments.
Related Issues
--resolve-plugins-relative-to
flagESLint
Class ReplacingCLIEngine
#40 - New:ESLint
Class ReplacingCLIEngine