eslint icon indicating copy to clipboard operation
eslint copied to clipboard

Support having plugins as dependencies in shareable config

Open sindresorhus opened this issue 10 years ago • 205 comments

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.

module.js:338
    throw err;
          ^
Error: Cannot find module 'eslint-plugin-no-use-extend-native'
    at Function.Module._resolveFilename (module.js:336:15)
    at Function.Module._load (module.js:278:25)
    at Module.require (module.js:365:17)
    at require (module.js:384:17)
    at /usr/local/lib/node_modules/eslint/lib/cli-engine.js:106:26
    at Array.forEach (native)
    at loadPlugins (/usr/local/lib/node_modules/eslint/lib/cli-engine.js:97:21)
    at processText (/usr/local/lib/node_modules/eslint/lib/cli-engine.js:182:5)
    at processFile (/usr/local/lib/node_modules/eslint/lib/cli-engine.js:224:12)
    at /usr/local/lib/node_modules/eslint/lib/cli-engine.js:391:26

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.

sindresorhus avatar Aug 20 '15 07:08 sindresorhus

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:

  1. The version of ESLint you are using (run eslint -v)
  2. The source code that caused the problem
  3. The configuration you're using (for the rule or your entire config file)
  4. The actual ESLint output complete with line numbers

Requesting a new rule? Please be sure to include:

  1. The use case for the rule - what is it trying to prevent or flag?
  2. Whether the rule is trying to prevent an error or is purely stylistic
  3. Why you believe this rule is generic enough to be included

Requesting a feature? Please be sure to include:

  1. The problem you want to solve (don't mention the solution)
  2. Your take on the correct solution to problem

Including this information in your issue helps us to triage it and get you a response as quickly as possible.

Thanks!

eslintbot avatar Aug 20 '15 07:08 eslintbot

We already discussed this in #2518 with the conclusion that a peerDependency is the way to go.

lo1tuma avatar Aug 20 '15 07:08 lo1tuma

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.

sindresorhus avatar Aug 20 '15 07:08 sindresorhus

A shareable config is a node module and should act like it.

We use require to load shareable configs or plugins, what would make it act more like a node module than that?

This issue should be also solved when using npm version 3 which installs all subdependencies in the top-level node_modules folder.

lo1tuma avatar Aug 20 '15 07:08 lo1tuma

We use require to load shareable configs or plugins, what would make it act more like a node module than that?

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
};

This issue should be also solved when using npm version 3 which installs all sub-dependencies in the top-level node_modules folder.

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.

sindresorhus avatar Aug 20 '15 07:08 sindresorhus

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.

feross avatar Aug 20 '15 08:08 feross

@sindresorhus good point about npm 3, let's forget about this.

I kind of like your proposal, but it has a few problems:

  1. Eslint needs to know the name of the plugin. This could be solved easily by providing an object with the name e.g plugins: [ { 'eslint-plugin-foo' : require('eslint-plugin-foo')} ]
  2. Eslint caches plugins once they are loaded. This could be a problem if you use different shareable configs in different .eslintrc files where the shareable configs require the same plugin, but in a different version. Possible solution would be to avoid caching in this case.

lo1tuma avatar Aug 20 '15 08:08 lo1tuma

Possible solution would be to avoid caching in this case.

Or we can prefix plugins provided by shareable configs with the name of the config?

BYK avatar Aug 20 '15 09:08 BYK

@BYK how would you reference the rules then? configname/pluginname/rulename? But I guess we would have the same problem if we avoid caching. We can't determine to which version of the plugin the rule belongs to.

That said, I think we should first decide if we want this feature in ESLint.

lo1tuma avatar Aug 20 '15 09:08 lo1tuma

@BYK how would you reference the rules then? configname/pluginname/rulename?

Yep.

That said, I think we should first decide if we want this feature in ESLint.

Agreed. Might be worth piggy backing on npm 3 instead of introducing this complexity.

BYK avatar Aug 20 '15 10:08 BYK

A few things:

  1. npm 3 doesn't solve this problem, the relationship between a config and a plugin remains a peer relationship. The fact that npm 3 flattens dependencies doesn't fundamentally change that relationship, is just an implementation quirk that allows dependencies to be treated as peers in certain situations. That's not a solution, is a gamble.
  2. Configs should not contain executable code, that's very far outside of the responsibilities of configs.
  3. To keep this in context: we are talking about a one-time setup cost rather than ongoing pain.

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.

nzakas avatar Aug 20 '15 16:08 nzakas

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.

sindresorhus avatar Aug 21 '15 05:08 sindresorhus

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:

npm install --save-dev eslint-plugin-standard eslint-config-standard eslint-config-ianvs 

It would be much nicer UX to only need:

npm install --save-dev eslint-config-ianvs 

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.

IanVS avatar Aug 21 '15 16:08 IanVS

We could extend plugins to allow the inclusion of configs, as plugins were always intended to be a dumping ground of stuff. Thoughts:

  1. How do we specify them in extends? eslint-plugin-foo.configs.whatever? Something else?
  2. We could, in theory, just expose the file system so you could do eslint.plugin-foo/configs/whatever.
  3. This is a bit lousy because we lose the nice eslint-config-* convention for configurations, so it ends up blurring what is a configuration and what is not.
  4. What if a config wants to depend on a plugin that it doesn't own? What does that look like?
  5. What is the expected behavior when a plugin is install directly and the same plugin is installed indirectly via another plugin?

nzakas avatar Aug 24 '15 18:08 nzakas

@nzakas

Configs should not contain executable code, that's very far outside of the responsibilities of configs.

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 .eslintrc. They don't care if there's executable code in there or not. Why complicate things for users?

feross avatar Aug 30 '15 20:08 feross

@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.

nzakas avatar Aug 31 '15 21:08 nzakas

Let the shareable config provide the plugin as an object

:+1: would love to have this functionality!

We use require to load shareable configs or plugins, what would make it act more like a node module than that?

The problem is that the plugin name is not left as is, but instead parsed and prepended with eslint-plugin-. If ESLint didn't do this one could have solved the problem by adding plugins by their full paths, e.g. plugins: [path.join(__dirname, 'node_modules', 'eslint-plugin-babel')], not fancy but it would probably work.

joakimbeng avatar Sep 01 '15 05:09 joakimbeng

We don't have a good answer for this now. We'll revisit once we've finished some 2.0.0 tasks.

nzakas avatar Sep 01 '15 23:09 nzakas

Related to #3659

ilyavolodin avatar Sep 16 '15 00:09 ilyavolodin

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 extends are loaded so that nested extends are processed in the module context that they come from?

This could at least solve the issue for configs, which do not have the issue of executable code.

e.g.

  • say I am making a shareable config for my team's projects, eslint-config-myteam
  • I want to base myteam config on another shareable config eslint-config-goodstyles with a few modifications.
  • in one of my team's projects ("myproject"), I npm install eslint eslint-config-myteam and create .eslintrc that extends: myteam
// eslint-config-myteam/index.js
module.exports = {
  extends: 'goodstyles'
}
# myproject/.eslintrc
extends: myteam

So when eslint is processing myproject/.eslintrc and finds extends: myteam it will locate node_modules/eslint-config-myteam.

At the moment I think it blindly reads that in, then fails when it hits the nested extends: goodstyles because that is not available at the top level. Could it instead keep track of which module it found the myteam config in, and if it finds an extends in there, search in that module for the config it extends. There are a few options for how to search:

  1. look only in the specific module that the extending config is from
  2. look first in the specific module that the extending config is from, then look at the higher level if it is not found there
  3. look first in the module where eslint was run, then in the specific module if the config is missing form there

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 package.json, but there is the option to include them and make life easier for consumers of their shared config.

davidmason avatar Oct 20 '15 02:10 davidmason

This issue isn't really about extending configs, it's about bundling configs with plugins.

nzakas avatar Oct 20 '15 16:10 nzakas

Or is it about bundling plugins with configs? From the first post in this issue by @sindresorhus:

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.

It is currently painful to extend a config which uses rules from a plugin. Or am I missing a larger picture?

IanVS avatar Oct 20 '15 16:10 IanVS

Semantics. To me "configs with plugins" means "config packages contain dependencies on plugin packages".

nzakas avatar Oct 20 '15 22:10 nzakas

@nzakas should I copy the "configs with configs" thing to a separate issue?

davidmason avatar Oct 21 '15 00:10 davidmason

@davidmason no need. That's a level of complexity that I don't want to build. If you want to extend an existing config in your own, just require it, make whatever modifications you want, then export it. It will work.

nzakas avatar Oct 21 '15 01:10 nzakas

@nzakas makes sense, I'll do that. I can check the eslint code to make sure I merge my changes in the same way so there are no surprises.

davidmason avatar Oct 21 '15 05:10 davidmason

Any news on this topic? I do understand that, from some point of view, peerDependencies might be the "theoretical right" spot to declare it.

But if we take a step back and think about the user, a more flexible / babeljs@6-like solution would be wunderful! Custom bundles of eslint rules/plugins are currently cumbersome to both

  • install it in the first place (npm install -D eslint-config-acme-corp eslint-config-dep-a eslint-config-dep-b eslint-plugin-dep-c ... .. uff)
  • keeping track of updates
    • automated checks (like http://greenkeeper.io) will notify me about updates available for dependencies I (the regular user) don't care about
    • what if eslint-config-acme-corp drops one dependency? or replaces it with a whole new one? it becomes a instant pain for all users of the bundle

The issues above are the things I, as the creator of the bundle, like to avoid for all my users. And this is currently, AFAIK, not possible. So from a user point of view a good solution to create, share and update "eslint presets" would totally be a benefit. Or to put it differently: Those are the core requirements of a good "eslint presets" feature, they are currently missing and so is the whole feature.

A personal short side-note: The require.resolve-hack used by groupon and proposed to airbnb is nothing eslint should encourage you to! Also it only works for extends and parser - plugins does not allow this "trick".

michaelcontento avatar Dec 02 '15 07:12 michaelcontento

@michaelcontento no updates. This is quite a bit more difficult than Babel plugins because you can directly reference plugins in config files. If a shareable config depends on a plugin foo, and a user has manually installed that plugin as well, then when the end user references foo, what is the expected behavior? Which instance of the plugin should it refer to?

We will take another look after 2.0.0, by this is the main problem that needs resolving.

nzakas avatar Dec 02 '15 17:12 nzakas

Thank you for the honest response!

Is there a target release date for 2.0.0?

michaelcontento avatar Dec 02 '15 19:12 michaelcontento

Targeting January.

nzakas avatar Dec 02 '15 23:12 nzakas