-
-
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
Support having plugins as dependencies in shareable config #3458
Comments
Thanks for the issue! We get a lot of issues, so this message is automatically posted to each one to help you check that you've included all of the information we need to help you. Reporting a bug? Please be sure to include:
Requesting a new rule? Please be sure to include:
Requesting a feature? Please be sure to include:
Including this information in your issue helps us to triage it and get you a response as quickly as possible. Thanks! |
We already discussed this in #2518 with the conclusion that a |
Ugh, that's such a leaky abstraction. I guess I won't use plugins then... The user shouldn't have to care what plugins I use for the rules. This is like requiring to manual install of Lodash when you want ESLint. A shareable config is a node module and should act like it. |
We use This issue should be also solved when using npm version 3 which installs all subdependencies in the top-level |
Let the shareable config provide the plugin as an object: module.exports = {
plugins: [
require('eslint-plugin-no-use-extend-native')
],
env: {
node: true
},
rules: {
'comma-dangle': [2, 'never'],
'no-cond-assign': 2
};
That's an implementation detail and not always guaranteed. Nobody should ever depend on that. npm@3 promises flatter dependency tree, not flat. If there are conflicts, there will be nesting. |
The option to require the plugin in the config itself would allow users to use my shareable config without needing to manually install two other plugins. I like this proposal. |
@sindresorhus good point about npm 3, let's forget about this. I kind of like your proposal, but it has a few problems:
|
Or we can prefix plugins provided by shareable configs with the name of the config? |
@BYK how would you reference the rules then? That said, I think we should first decide if we want this feature in ESLint. |
Yep.
Agreed. Might be worth piggy backing on npm 3 instead of introducing this complexity. |
A few things:
While I can understand the desire to have one install that works, I don't see a path towards that without introducing a new type of shareable thing that could encapsulate this functionality. |
Then maybe introduce a universal sharing thing that can contain multiple plugins/configs/whatever. It could even in the future allow extending ESLint in some ways, with hooks, but I don't want to start that discussion. Just showing the possibilities with something like this. JSCS supports it like this: https://github.com/wealthfront/javascript/blob/f1f976e9c75a8d141ec77a5493d9d965d951d4a6/jscs/index.js I just want the user to be able to npm install one module and have the needed config and plugins without having to care about how anything works internally. That's the beauty of normal npm packages. |
I agree that the current method becomes unwieldy when you begin sharing configs which use other shared configs and/or plugins. For example, the installation instructions for my own personal config (which extends from Standard) is:
It would be much nicer UX to only need:
That said, I have no idea how that could be accomplished, and in the end it's a pain I can live with until/unless a better solution is found. |
We could extend plugins to allow the inclusion of configs, as plugins were always intended to be a dumping ground of stuff. Thoughts:
|
Could you elaborate on this? It seems like this is a philosophical rather than practical objection. From a user's perspective, an eslint config is just an npm package that they need to install and extend in their |
@feross Allowing executable objects arbitrarily in configs would complicate things for users. What I'm saying is let's not complicate it by ensuring that configs remain static regardless of their form. |
👍 would love to have this functionality!
The problem is that the plugin name is not left as is, but instead parsed and prepended with |
We don't have a good answer for this now. We'll revisit once we've finished some 2.0.0 tasks. |
Related to #3659 |
It seems as though this is the case for configs as well, unless I am mistaken. For configs at least, is it possible to change how This could at least solve the issue for configs, which do not have the issue of executable code. e.g.
// eslint-config-myteam/index.js
module.exports = {
extends: 'goodstyles'
} # myproject/.eslintrc
extends: myteam So when eslint is processing At the moment I think it blindly reads that in, then fails when it hits the nested
The question is whether people should be able to override configs by name (on purpose or otherwise) in their extending config. Overriding configs by accident would be possible with 3, so I would rule that out. 1 would not allow peer-dependency configs, so I think 2 is the best option - if someone wants to make other configs peer dependencies they can just not include them in their |
It's a slap in the face not to allow |
See this bug for why: eslint/eslint#3458
Was using But, recently I try yarn berry. Does any one know what's the workaround for yarn berry, the hoist way, or any way to let yarn berry auto install peer dependency? |
I got tired of waiting and dealing with gotchas with the module resolution patch libraries so I wrote a small library that lets you “wrap” dependencies’ rules, so you can add any plugins or configs you like as regular JavaScript dependencies: https://www.npmjs.com/package/eslint-plugin-wrapper We use it at my work with pnpm (rush) but since it’s playing by eslint’s rules without hacks or low level patches, it should with any package manager/module resolution strategy, feel free to try it out. |
…onfig (#10410) Works around eslint limitations by directly requiring base config, leaning on Node's module resolution instead. Note that this usage pattern is not explicitly supported by eslint, but has worked for the last few major versions, and has been sited as a workaround in the issue tracking this major eslint limitation (see Issue [3458](eslint/eslint#3458)), and is used in a number of large projects including `create-react-app`. Also note that this pattern sort of assumes that a given package **does not** install its own copies of the plugin packages used by the shared config package. If they do, they will end up getting the version of the plugin **they** specify, rather than the one specified by the shared config package. This follows npm conventions pretty reasonably, but does deviate from standard eslint conventions a bit. Behavior here could be confusing in cases where one version of a plugin contains a rule that an earlier version doesn't (for example). - Fortunately, since we leverage lerna's `--strict` option when hoisting dependencies, we currently have the guarantee that this problem won't occur :) Ultimately, I think the benefits here greatly outweigh any potential issues, and I think our other tooling will help prevent them anyways, so I think this is probably the right change. I just wanted to be transparent here about the trade-offs. Additionally, removing the extraneous plugin dependencies from consumers of the base config revealed some unused plugin deps that have also been removed.
Worth sharing this workaround: PS: you can see an opinionated example usage https://github.com/belgattitude/nextjs-monorepo-example/tree/main/packages/eslint-config-bases |
Is it possible with ESLint 8 now? I notice |
@SevenOutman It looks like |
Ahh, missed that one. |
This comment was marked as off-topic.
This comment was marked as off-topic.
I'm getting some issues with It's obvious that shell can't process any `virtual' dirs with a virtual zip archive resolver, but general pnp compat should work in this specific case, needs some investigation. ESLint: 8.23.0
Error: Cannot read config file: /home/yura/src/boosh/.yarn/__virtual__/eslint-config-airbnb-virtual-067a933ec7/0/cache/eslint-config-airbnb-npm-19.0.4-a73150c84a-253178689c.zip/node_modules/eslint-config-airbnb/whitespace.js
Error: Command failed: /home/yura/src/boosh/.yarn/__virtual__/eslint-config-airbnb-virtual-067a933ec7/0/cache/eslint-config-airbnb-npm-19.0.4-a73150c84a-253178689c.zip/node_modules/eslint-config-airbnb/whitespace-async.js
/bin/sh: line 1: /home/yura/src/boosh/.yarn/__virtual__/eslint-config-airbnb-virtual-067a933ec7/0/cache/eslint-config-airbnb-npm-19.0.4-a73150c84a-253178689c.zip/node_modules/eslint-config-airbnb/whitespace-async.js: No such file or catalog
Referenced from: /home/yura/src/boosh/.eslintrc.js
at checkExecSyncError (node:child_process:841:11)
at execSync (node:child_process:912:15)
at Object.<anonymous> (/home/yura/src/boosh/.yarn/__virtual__/eslint-config-airbnb-virtual-067a933ec7/0/cache/eslint-config-airbnb-npm-19.0.4-a73150c84a-253178689c.zip/node_modules/eslint-config-airbnb/whitespace.js:54:38)
at Module._compile (node:internal/modules/cjs/loader:1126:14)
at Object.Module._extensions..js (node:internal/modules/cjs/loader:1180:10)
at Object.require$$0.Module._extensions..js (/home/yura/src/boosh/.pnp.cjs:29491:33)
at Module.load (node:internal/modules/cjs/loader:1004:32)
at Function.require$$0.Module._load (/home/yura/src/boosh/.pnp.cjs:29331:14)
at Module.require (node:internal/modules/cjs/loader:1028:19) |
@yuriy-yarosh No, I give up trying yarn berry |
Hi folks! Just an update that the new ESLint configuration has now been enabled as an experiment. This new config system eliminates the special treatment of shared configs so that you can include plugins as direct dependencies. Read about it here: https://eslint.org/blog/2022/08/new-config-system-part-1/ This is the long-term solution to the problem in this issue. We know it took a while, but this was a really complex problem that we needed to think through. Because we now have this solution implemented, I'm closing this issue. Please try out the new config system and leave your feedback in the discussions. |
My shareable config uses rules from an external plugin and I would like to make it a
dependency
so the user doesn't have to manually install the plugin manually. I couldn't find any docs on this, but it doesn't seem to work, so I'll assume it's not currently supported.I assume it's because you only try to load the plugin when the config is finished merging.
Other shareable configs that depend on a plugin instructs the users to manually install the plugin too and they have it in
peerDependencies
. I find this sub-optimal though and I don't want the users to have to care what plugins my config uses internally.The whole point of shareable configs is to minimize boilerplate and overhead, so this would be a welcome improvement.
Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.
The text was updated successfully, but these errors were encountered: