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.
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:
eslint -v
)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
peerDependency is the way to go.
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.
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.
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.
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:
plugins: [ { 'eslint-plugin-foo' : require('eslint-plugin-foo')} ]
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 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.
@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.
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:
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.
We could extend plugins to allow the inclusion of configs, as plugins were always intended to be a dumping ground of stuff. Thoughts:
extends
? eslint-plugin-foo.configs.whatever
? Something else?eslint.plugin-foo/configs/whatever
.eslint-config-*
convention for configurations, so it ends up blurring what is a configuration and what is not.@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 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.
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.
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 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.
eslint-config-myteam
myteam
config on another shareable config eslint-config-goodstyles
with a few modifications.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:
eslint
was run, then in the specific module if the config is missing form thereThe 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.
This issue isn't really about extending configs, it's about bundling configs with plugins.
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?
Semantics. To me "configs with plugins" means "config packages contain dependencies on plugin packages".
@nzakas should I copy the "configs with configs" thing to a separate issue?
@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 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.
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
npm install -D eslint-config-acme-corp eslint-config-dep-a eslint-config-dep-b eslint-plugin-dep-c ...
.. uff)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 bundleThe 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 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.
Thank you for the honest response!
Is there a target release date for 2.0.0?
Targeting January.
FYI: Shareable configs can include some plugins in those dependencies
field.
For example, my config is depending on eslint-plugin-mysticatea
, eslint-plugin-node
, and eslint-plugin-react
. But user (me!) does not need to install the plugins manually (e.g. package.json).
Because npm does flatten dependencies, so user's project can require()
the plugins.
For npm 2.x, we can use peerDependencies
.
@mysticatea that only works when the user hasnt also installed the plugins (maybe a different version). The risk is still there if you ever do that in the future.
Also keep in mind that npm 3 only flattens dependencies when there's no conflict amongst dependency versions for all dependents. If there is a conflict, the npm 2 approach is used.
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.
:+1: to this. ask anyone on npm team; if you need a package, you need to specify it as a dependency, flattened node_modules
or no, or you're just asking for trouble
if anyone wants to mess around with this, here's a quick-and-dirty monkeypatch to avoid having to npm install
_n_ packages to get one config and the plugins it uses.
Sorry to ping everyone, but has there been any progress on this?
I want to make a shareable config that extends xo and then adds a few custom in-house rules. Is the situation still that this is not possible?
You can, but you need to treat xo as a peerDependency
. Take a look at semistandard for an example of another config doing something similar.
@callumlocke our current priority is making autofix better. When there are updates for this issue, we will comment.
Since this was possible in JSCS and it is needed for good amount of plugin writers (including me) i think we can treat it as priority.
If no one minds i'd like to talk about this on the next TSC meeting
Per TSC meeting (21-Jul-16), it would be good to get an overview of the JSCS plugin use case plus a description of how JSCS currently solves this problem. Then, we need a proposal for how to overcome the problems discussed earlier in this thread (primarily, what happens when two different versions of a plugin are required by different configs used in the same project?).
Oh, there is bunch of issues then that, will try to document a proposal and tackle them when I finish with couple other issues.
Will put a JSCS label here, not sure if it needed or not though, so feel free to remove it if you feel it is not appropriate here
Just to add another use case example for this. We recently released Create React App—a tool that creates React apps with no build configuration. Internally we use ESLint but we are intentionally keeping it non-configurable. (There is an escape hatch for people who want configuration: they can “eject” and then all config files get copied into their projects.)
This project also follows the “single dev dependency” approach because many people (especially those getting started) are tired of installing dependencies. I know it sounds funny but it’s true. That create-react-app
gained 4k stars in 4 days speaks to the desire for zero configuration and having few dependencies. Of course this doesn’t work for everyone. But our initial users seem fairly happy with this setup.
The problem for us is that while command line linting works fine (since we’re able to specify a custom hidden config), editor integrations are not as smooth.
Having to do this in the generated project is already a tiny bit frustrating:
{
"name": "my-app",
"version": "0.0.1",
"private": true,
"devDependencies": {
"react-scripts": "0.1.0"
},
"dependencies": {
"react": "^15.2.1",
"react-dom": "^15.2.1"
},
"scripts": {
"start": "react-scripts start",
"build": "react-scripts build",
"eject": "react-scripts eject"
},
"eslintConfig": {
"extends": "./node_modules/react-scripts/config/eslint.js" // meh!
}
}
The only reason eslintConfig
is needed here is because editors don’t “know” about our custom config, and we have to make it discoverable.
But of course even that doesn’t solve the problem completely because of this issue. If the user is on npm 2, they’re out of luck completely because plugins are not getting discovered. Even if the user is on npm 3, it working feels like a fragile coincidence.
I think the bigger theme here is that as a community, we solved many problems of customizing configuration (and ESLint has been terrific at it), but maybe it’s time for the pendulum to swing back, and for us to start solving the problem of sane zeroconf defaults via opinionated packages.
On a related note, I posted a proposal for a configurator
field in package.json
. It is intended for zero-conf tools like standard
and create-react-app
, and I think a convention like that could solve this issue. Please let me know what you think: https://github.com/facebookincubator/create-react-app/issues/215
cross-posted as per: https://github.com/eslint/eslint/pull/6865#issuecomment-238923390
eslintrc at odds with npm link (and other node resolution friendly usages)
B
: the extraction of generic build tool code (the invoker of eslint)A
: the application which which configures eslint + depends on the generic build tool code moduleA
depends on B
B
is a descendent of A
on the file systemB
"resolves" A's configuration it will walk up the file system and eventually find it~/projects/A/
~/projects/A/node_modules/B
A
depends on B
B
is in an ancestor directories node_module of A
on the file systemB
"resolves" A's configuration it will walk up the file system and not find A, rather it may find something else~/projects/A/
~/projects/node_modules/B
A
depends on B
B
npm link'd to A
on the file systemB
"resolves" A's configuration it will walk up the file system and not find A, rather it may find something else~/projects/A/
~/projects/node_modules/B -> ~/src/B
In node references to other code, and modules is always related to the including modules position on disk. Not relative to the invoker, or cwd
. But fs/system calls are relative to cwd().
// foo/bar/baz.js
require('some-module'); // is a resolve relative to the current file
For those interested, I found a workaround to have your plugins as dependencies in your shareable config.
See https://github.com/Cellule/eslint-myrules/commit/83b3ac20a3b202f952726f57a5d5f680a83d64a9
Essentially this monkey patches eslint plugin resolution by resolving the plugin yourself therefore using your module's resolve paths.
This works with npm link
as well.
It heavily depends on the current eslint file structure (works at least between versions 2.0.0 and 3.3.0) thus can be broken at any time.
Until this is officially supported I hope this might help someone else who like me wanted to alleviate the burden of settings up and maintaining eslint plugin versions.
Just pinging in that I too was bit by this.
I think community needs shareable configs to behave as usual npm packages—encapsulate deps with itself
Thanks everyone for the feedback. We're aware of everyone's desire for this feature and plan on addressing it once we've finished up JSCS compatibility work. As such, I'm going to lock this issue because we understand the request and the use cases, and at this point we're just getting a bunch of "me too" comments.
I know this is a popular issue, so I'm going to try to summarize where we are and why this is hard. The team is aware that this is a popular request, but unfortunately, popularity does not make the work any simpler or create the extra time needed to investigate how to solve this issue.
ESLint supported plugins first, and sometime later, shareable configs were created. At that time, we encouraged people to use peer dependencies through npm in order to define the relationships between shareable configs and plugins. This worked well because npm would install peer dependencies for you and it solved the deduping problem if, for example, a shareable config relied on a different version of a peer dependency than the one you already had installed.
Unfortunately, npm decided they would stop installing peer dependencies by default, instead relying on developers to manually install those dependencies. That was really the beginning of the request for shareable configs to be able to include plugins because developers didn't want to tell users of their shareable configs to manually install a bunch of other stuff. That's fair, but unfortunately, the relationship between shareable configs and plugins was designed, from the start, to be one of peer dependencies. With the npm changes, that relationship was no longer optimal.
There are a lot of comments that say something along the lines of, "well, I think shareable configs should just act like any other npm package in the way it defines its dependencies." Yes, it would be great if that were possible, however, shareable configs don't work the same way that other npm modules do (that's a feature, not a bug).
One of our early assumptions was that there could only be one instance of a plugin loaded per run. This is important because of the way we refer to plugin assets like rules. For instance, suppose you had eslint-config-myplugin
, your config looks like this:
plugins:
- myplugin
rules:
myplugin/rule: "error"
First, we load the plugin based on myplugin
in the plugins
array, and then we reference a rule within the plugin as myplugin/rule
. In order to use that convention, we must know with 100% certainty what myplugin
refers to, and it must refer to the same thing across all configs in the project in order to ensure each config may override settings that other configs set. That means each config cannot search up the directory structure for plugins like npm package lookup does. We keep an internal cache of the plugins and use that for lookup.
All that said, the reason this issue hasn't moved forward is because of one very serious edge case: what if two different versions of the same plugin are required by two different configs in a project? For instance, suppose:
eslint-config-foo
that depends on [email protected]
because it wants "foo" rules.[email protected]
directly.In this case, we have several problems:
eslint-plugin-foo
version to load, given that the config file will just say foo
?[email protected]
) and eslint-config-foo
references a rule that doesn't exist until version 2.0.0, ESLint will throw an error because the rule is missing.At this point, you'll likely arrive at a number of different solutions:
This is deceptively difficult to do. First, we don't have a good way of knowing that we're attempting to load a different version of the plugin rather than just the same one we saw before. Second, it might be beyond the end user's capabilities to fix this problem because they may be using several shareable configs, each of which depends on a different version of the plugin. Penalizing the end user for including configs with contradictory plugin package versions seems like the wrong choice.
First, that would mean figuring out a way to load two different versions of the same package, which Node.js isn't designed to do by default, so we'd have to go crawling around the filesystem to do this. Crawling around the filesystem is made much more difficult due to npm node_modules
flattening, so we couldn't really know the correct places to look, which would mean a lot of searching around for packages. We'd also need to come up with some way to ensure the end users knows which version of plugin they are configuring at any given time, which likely means we'd need a different naming scheme for configuring plugins that were included via shareable config vs. installed by the user. That's also difficult because shareable configs are treated like any other config inside of ESLint, so that would also be a lot of work.
Of course, when what you created isn't working the way you wanted, it's logical to stop and consider if you want to create a new type of thing that can work the way you want. The big problem here is that we have a big ecosystem of already-existing shareable configs and plugins, and would we really want to impose the burden of creating something new to fulfill that need?
I hope that helps explain why this issue hasn't moved much. Despite the desire of many people to figure out how to make this work, at the end of the day, there are enough complexities that it will take a significant amount of time from someone (on the team or off) to really dig in and try to solve this. As the ESLint team is a group of unpaid volunteers, no one has had enough time or interest to dig into this problem.
If you are interested in pitching in to solve this problem, we are open to hearing design proposals. The only constraint is that any design must address the hard problem of having two different parts of the project include different versions of the same plugin. All of the proposals to this point, both in this thread and through various pull requests, have not addressed it and so cannot be considered.
I'm unlocking this thread to allow for designs to be posted but if I see the thread fill up with +1s and other useless comments, I'll lock it again.
What we need is someone or a group of someones who are willing to dig and do the research and prototyping necessary to solve this problem. If that's you, please let us know.
@nzakas
Figure out a way to load two different versions of the same package
First, that would mean figuring out a way to load two different versions of the same package, which Node.js isn't designed to do by default, so we'd have to go crawling around the filesystem to do this.
This issue could potentially be addressed by allowing configs to include code, like this:
module.exports = {
plugins: {
foo: require('eslint-plugin-foo')
},
rules: {
'foo/rule': 'error'
}
}
This way, each config can use the exact plugin version that they list in their package.json
dependencies. No need to crawl around the filesystem.
We'd also need to come up with some way to ensure the end users knows which version of plugin they are configuring at any given time.
We could solve this by saying: you can only configure plugin rules if you require that plugin in your config. That way it's clear what version of the plugin you're configuring.
If you re-use a plugin rule that was already defined in an earlier config, then the later rule takes precedence (like how it works today). Note: Both the rule's setting and the rule code from the later config/plugin will take precedence.
Optional convenience feature: We could consider relaxing the rule about depending on the plugin, if the config is just disabling rules from that plugin.
One downside of this approach is that it only works with JS configs, not JSON or YAML.
@feross Essentially you're specifying a path, right? I don't see why that would have to be restricted to javascript. Babel does something similar where you can specify presets & plugin paths.
// how babel reads it
{
"presets": [
"/my_project/node_modules/babel-preset-es2015/lib/index.js",
"/my_project/node_modules/babel-preset-react/lib/index.js"
],
"plugins": [
"/my_project/node_modules/babel-plugin-transform-flow-strip-types/lib/index.js"
]
}
// can get compiled into a babel-readable config from any other language, example js:
presets: [
require.resolve('babel-preset-es2015'),
require.resolve('babel-preset-react')
],
plugins: [
require.resolve('babel-plugin-transform-flow-strip-types')
]
@feross unfortunately, I don't think we can accept a solution that only works in a JS file with passing around objects. There are plenty of folks who don't use JS-based configurations, and we need to be able to support them, as well. We'd like to keep configs as data-only.
@stoikerty that's an interesting idea - we could do an intermediate step that allows you to pass in package by filepath. We'd have to do something to allow you to name that package (since we wouldn't have the package name. Something like what @feross mentioned:
plugins:
foo: "/some/location/to/index.js"
rules:
foo/rule: "error"
There is still a concern in that case that someone might include eslint-config-foo
in a different config, and that would end up colliding with the foo
defined here. Maybe we'd need some special naming convention to differentiate between foo
meaning eslint-config-foo
vs. foo
meaning a custom name chosen for a filepath.
There is still a concern in that case that someone might include eslint-config-foo in a different config
This might be ugly but could we consider an absolute filepath a valid plugin “identifier”?
const foo = require.resolve('eslint-plugin-foo');
module.exports = {
plugins: [foo],
rules: {
[`${foo}/rule`]: 'error'
}
}
We would not support relative filepaths which excludes any non-JS configs from supporting this feature, but presumably it's fine to make a subset of configs (JS-based ones) more powerful?
There is still a concern in that case that someone might include eslint-config-foo in a different config, and that would end up colliding with the foo defined here.
When eslint is parsing a config-file, is it able to distinguish between configs and their associated plugins?
Mapping it out, if you have the following:
⦿ pluginConfigA ➤ pluginConfigB ➤ userConfig ◉
where:
[email protected]
[email protected]
and includes pluginConfigA[email protected]
and includes pluginConfigBWhat is the most reasonable way to handle this scenario? Is it possible for each config to reference and use its own foo
?
One other requirement that I wanted to mention is that the new way should be backwards compatible. We can't just break hundreds of plugins that are already published.
I think the main issue here is that the current processing of config files doesn't allow contextual identifiers. In other words, eslint-plugin-import
has to mean the exact same thing regardless of the config file that it appears in. This is very limiting, because if we want sharable configs to be usable independently from each other, they have to be able to have their own dependencies, and potentially their own versions of eslint-plugin-import
.
To fix this, rules and plugins referenced in a config file should be resolved relative to the config file itself. In other words, config files will become more like modules rather than using a global plugin namespace.
Proposal:
require()
relative to that config file./
as a delimiter. This allows a user to override settings specified in a particular config file.For example, here's eslint-config-foo
, which depends on [email protected]
. eslint-config-foo
can specify eslint-plugin-bar
as a dependency.
// ./node_modules/eslint-config-foo/index.js
module.exports = {
plugins: ['bar'], // resolved as if require() was used from this file, yielding [email protected]
rules: {
'bar/no-horses': 'error',
// As before, eslint-plugin-foo can have local references to rules as well.
'tball': './lib/rules/sports-jokes/tball.js'
}
};
And here's [email protected]
:
// ./node_modules/eslint-config-foo/node_modules/eslint-plugin-bar/index.js
module.exports = {
rules: {
'no-horses': require('./lib/rules/bar-jokes/no-horses.js');
}
};
Meanwhile, the user would like to use eslint-config-foo
, but they would also like to use [email protected]
separately. They can do this by installing eslint-config-foo
and eslint-plugin-bar
as dependencies (or devDependencies), and arranging their config file like this:
// ./.eslintrc.js
module.exports = {
// `foo` is resolved by using require('eslint-config-foo') from this file
extends: ['foo'],
// `bar` is resolved by using require('eslint-plugin-bar') from this file.
// This will yield [email protected], since the user has that installed as a dependency.
plugins: ['bar'],
rules: {
// Again, references to `bar` are resolved by using `require` from this file.
// So this will turn on the `tender` rule from [email protected]:
'bar/tender': 'error',
// As before, `foo` is resolved by using require('eslint-config-foo') from this file.
// This yields eslint-config-foo, which allows the user to configure rules from that config.
'foo/tball': 'off',
// Finally, a slash character can be used to chain references.
// In this case, `foo` is resolved by using require('eslint-config-foo') from this file.
// Then `foo/bar` is resolved by using require('eslint-plugin-bar') from eslint-config-foo.
// This allows the user to toggle the rules from [email protected] that are used by foo:
'foo/bar/no-horses': 'warn'
}
};
@feross unfortunately, I don't think we can accept a solution that only works in a JS file with passing around objects. There are plenty of folks who don't use JS-based configurations, and we need to be able to support them, as well. We'd like to keep configs as data-only.
Can we offer this as a JS-only feature to at least get started? I understand the concern around restricting configs to only data but if we had the actual JS and module system, it may open other doors. Existing JSON and YAML configurations are already compatible with this, just more limited and in the future if we decide to drop support for these, it should be trivial to write a tool (or may be even include it in eslint
) that converts these to JS modules.
@BYK I think it's worth figuring out a good solution first, rather than implementing if halfway and figuring out the rest later. If we implement it halfway, it will make it much harder to change the system later.
@not-an-aardvark
It is not clear to me from the team responses so far whether making JS configs more powerful is considered implementing it “halfway”, and why.
This is a hard problem if you treat configs purely as data, precisely because data presumes constant plugin identities whereas the issue is about potentially conflicting plugins.
Other tools, such as Babel, have already successfully solved the exact same problem by making presets code rather than data. I don’t quite understand why ESLint can’t follow the same path, even while allowing backward compatibility with “less powerful” formats. I also don’t quite see why having multiple formats to specify configs is useful, but this is probably due to my ignorance about the topic.
@gaearon My objection was mostly in response to offering as a JS-only feature "to at least get started". If we determine that JS-only is the solution we want to use, then that's fine, but I don't think we should rush to a temporary solution with plans to change it later, because that will cause backwards-compatibility issues.
I think the solution in https://github.com/eslint/eslint/issues/3458#issuecomment-253968468 would be better anyway -- it would allow locally-resolved dependencies without needing to explicitly call require
.
Can we offer this as a JS-only feature to at least get started?
I thought I'd addressed this pretty clearly earlier in the thread, but I'm going to say it once again: no. Continuing to discuss strategies that only work in JS configs is not helping us solve the problem. I'm not going to go into a long explanation of why we have different config file formats, but the fact is that we do and we have a large ecosystem using all different types of formats. Our promise has always been that the formats are interchangeable, and I don't see that changing anytime soon. So let's focus on the problem at hand.
@not-an-aardvark That's an interesting approach, and I really appreciate you taking the time to explain it. Some points that popped into my head while reading it:
plugins: ['bar']
and the user config does not even though it's configuring rules like bar/rule
. I think your proposal would break those configs (Airbnb is the most popular shareable config, and they are setup like this).eslint-config-a
depends on eslint-config-b
which depends on eslint-plugin-p
, do rules now become a/b/p/rule
?)I think my overall feedback is that there may be a breakdown with the naming convention once we get into more involved relationships between shareable configs and plugins. Do you have thoughts around that?
It's unclear to me if this approach is breaking. Right now, if someone is using a shareable config that relies on a plugin, they are installing both the plugin and config, presumably at the same level. However, the shareable config has plugins: ['bar'] and the user config does not even though it's configuring rules like bar/rule. I think your proposal would break those configs (Airbnb is the most popular shareable config, and they are setup like this).
There are two issues to consider here:
require('eslint-plugin-bar')
from foo
would still be able to find bar
, due to Node's node_module
resolution algorithm.foo
by simply referencing bar/rule
, rather than foo/bar/rule
. This would be a breaking change according to the current proposal. As a workaround for backwards compatibility, maybe we could handle missing references to bar
based on whether another config had a valid reference to bar
, and use that reference as a fallback.// ./node_modules/eslint-config-foo/index.js
module.exports = {
plugins: ['bar'],
rules: {
'bar/rule1': 'error'
}
};
// ./.eslintrc.js
module.exports = {
extends: ['foo'],
rules: {
// eslint-config-foo had a valid reference to `bar`, and no valid reference can be resolved here.
// As a fallback, this reference will work as an alias for 'foo/bar/rule1', resolving as the version of
// `bar` that eslint-config-foo used.
'bar/rule1': 'warn'
}
};
While this would preserve backwards-compatibility, it isn't an ideal solution, since it could cause confusing behavior if the user installs something else that depends on a different version of eslint-plugin-bar
. If we did decide to follow this pattern, we would probably want to deprecate it in favor of explicitly listing foo/bar/rule1
instead. (Alternatively, I'm open to hearing other workarounds for this.)
There's a level of increasing complexity here that I'm concerned about: what if a shareable config is dependent on another shareable config that, in turn, is dependent on a plugin? How does that affect how you configure rules? (eslint-config-a depends on eslint-config-b which depends on eslint-plugin-p, do rules now become a/b/p/rule?)
Yes, this would be resolved as a/b/p/rule
. This does lead to increasing complexity, but I'm not sure this is avoidable; if there are there are multiple different versions of eslint-plugin-p
scattered throughout node_modules
, it is necessary to explicitly specify which version of eslint-plugin-p
the user is referring to.
We _could_ allow shorthands for these references if they're unambiguous (see the backwards-compatibility workaround above), which would allow things like p/rule
rather than a/b/p/rule
. However, I don't think we should encourage this pattern, since it's liable to unexpectedly break if a different version of eslint-plugin-p
gets installed by some other dependency.
Keep in mind that plugins can also export configs that can be inherited from (extended). That means you can't always assume that one member of a slash-delimited identifier is a config vs. a plugin.
True, but I don't think this is a problem; we can simply resolve extended config files the same way as everything else. For example: 'airbnb/airbnb-base/import/no-unresolved': 'off'
I think my overall feedback is that there may be a breakdown with the naming convention once we get into more involved relationships between shareable configs and plugins. Do you have thoughts around that?
I would say that the naming convention _works_ in these relationships, but it doesn't produce an ideal experience for users that want to configure rules from inherited plugins. Admittedly, having to use a path like a/b/p/rule
in your config file is annoying.
However, if we want to allow multiple different versions of configs at the same time (which is necessary for a good resolution to this issue), while still allowing users to individually configure inherited rules, I don't think this annoyance is avoidable in the general case. Allowing multiple versions of a plugin to be run simultaneously will require the user to be more specific about which version of the plugin they want to configure.
Yes, to clarify, I have no problem with the part of the proposal to load plugins relative to shareable configs. In fact, I've never been opposed to that, my only problem has been with how we expect users to address multiple versions of the same plugin in their configs -- to me, that's the hard part of this problem. (Although loading plugins relative to their configs is a nontrival effort to implement, it's pretty straight forward.)
I would say that the naming convention works in these relationships, but it doesn't produce an ideal experience for users that want to configure rules from inherited plugins. Admittedly, having to use a path like a/b/p/rule in your config file is annoying.
Yeah, I think this is a legitimately horrible experience. The problems (off the top of my head):
namespace/rule
). We'd need a different syntax for namespaces that are config names to avoid naming collisions.If we are making the argument that shareable configs should work like any other modules, then I think the only real direction is to force shareable configs to re-export whatever it is that they want to expose to end users. That way, the end user only needs to know about the shareable config without any of its dependencies (which is ultimately what we want). So maybe a shareable config has something like:
exportedPlugins:
foo: react # Load eslint-plugin-react into identifier foo
bar: import # Load eslint-plugin-import into identifier bar
Then in end-user config, you'd do:
extends: myconfig
rules:
$myconfig/foo/rule1: "error"
$myconfig/bar/ruleA: "warn"
Where the $
indicates that this is coming from the config rather than a plugin. (Could be any special character, doesn't have to be $
.)
The problem with this approach is that it only really works one level down. If a shareable config A depends on another shareable config B, I'm not sure how you'd say to promote the rules from B so they can be accessed from A.
As I've said a few times, the devil is in the details on this effort. :)
Perhaps you could also do:
exportedPlugins:
foo: react # Load eslint-config-airbnb into identifier foo
bar: import # Load eslint-plugin-import into identifier bar
baz: import/dep # Load eslint-plugin-import's exported plugin "dep" into identifier baz
In other words, it'd be one level down, but at each level, it could be explicitly re-passed?
If we are making the argument that shareable configs should work like any other modules, then I think the only real direction is to force shareable configs to re-export whatever it is that they want to expose to end users
I'm not sure I understand this point. I thought a design goal here was that users should always be able to disable a given rule in their config. If we're okay with a rule being impossible to disable from a config since it's not explicitly exported by a plugin, then this changes some of the strategies that we might use to address the problem.
Importing slightly different rules from different versions of a single plugin seems theoretically logic but practically confusing, IMHO.
Wouldn't it be more straightforward to just fail-fast in a similar scenario, or maybe to make an arbitrary choice (plugin-wide or rule-wide) in case of version conflict?
After reading this thread, I think I actually agree with @caesarsol It seems like we are trying to preemptively fix an issue that isn't a real problem (at least, that's not something we've seen so far). While it's entirely possible to have very complex configuration with multiple configs inherited and deep nesting, it seems like it would be very rare, it also seems that the person that might go through the trouble of setting something like that up, knows what they are doing and should be able to fix it himself. Maybe we don't need to solve the issue of multiple versions of the same plugin?
fwiw, Airbnb's use case for eslint-config-airbnb
will be satisfied as long as:
eslint
@ilyavolodin I disagree. How many times have we seen edge cases pop up from multiple people over a period of time that eventually forces us to make a change? I'm pretty exhausted by this discussion, so if we're going to solve this problem, then I want to solve it in such a way that we don't need to think about it ever again. :)
@not-an-aardvark
I'm not sure I understand this point. I thought a design goal here was that users should always be able to disable a given rule in their config.
They'd still be able to disable the rule, but the reference would be through the config rather than directly to the plugin.
Okay, so I was lying awake in bed last night when I realized something: if we slightly change this discussion from "how can a shareable config bundle plugins as a dependency" to "how can a config bundle plugins as a dependency," then the problem is actually already solved because plugins can contain configs and can also specify other plugins as dependencies.
Suppose Airbnb has a config they want people to use and that config depends on eslint-plugin-react
. If the config is exposed in a plugin instead of a shareable config, you can make a plugin that looks like this:
// eslint-plugin-airbnb
const reactPlugin = require("eslint-plugin-react");
// export the es6 config
module.exports.configs = {
es6: require("./configs/es6.json")
};
// export the rules you want to expose
module.exports.rules = reactPlugin.rules;
The eslint-plugin-airbnb
package can depend directly on eslint-plugin-react
, so the dependencies are installed automatically with npm i eslint-plugin-airbnb
. Then, in your local project config, you can do this:
plugins:
- airbnb
extends:
- plugin:airbnb/es6
rules:
airbnb/jsx-uses-vars: "error"
In this case, the project config is extending the config from eslint-plugin-airbnb
and the plugin is re-exporting the jsx-uses-vars
rule from eslint-plugin-react
, so you have to address it using airbnb/
instead of react/
. IMHO, this is kind of nice because it makes the plugin dependency completely opaque. However, if you wanted a bit more transparency, you could change the plugin to prepend react/
to the beginning of each rule:
// eslint-plugin-airbnb
const reactPlugin = require("eslint-plugin-react");
// export the es6 config
module.exports.configs = {
es6: require("./configs/es6.json")
};
// export the rules you want to expose
module.exports.rules = {};
Object.keys(reactPlugin.rules).forEach(ruleName => {
module.exports.rules["react/" + ruleName] = reactPlugin.rules[ruleName];
});
Then you could reference the rules as airbnb/react/jsx-uses-vars
.
It seems like this approach solves all of the hard problems we've been trying to address with a decent amount of flexibility (in that plugin authors can decide which rules to export and how they should be named):
As I've said before, shareable configs were really not designed to be used in the way we are trying to get them to be used in this thread, and I think the reason we keep banging into walls is because we're trying to force shareable configs into a paradigm they were never designed for.
Plugins, on the other hand, are made to be more composeable and looking back, it actually looked like we wanted to encourage people to use plugin configs instead of shareable configs when we first implemented it.
It seems like the best next step is for someone to actually try this out and give feedback on the experience. Any volunteers?
@nzakas This sounds like a good solution and I'd be happy to give it a try in Create React App.
If it works, this would solve some persistent problems we've had:
react-scrips
) to the project. But currently using editor integrations for ESLint requires our users to also install all the plugin dependencies we use either globally or in their project.I can try this out and give feedback on how it goes.
That's a pretty huge breaking change for all the current users of eslint-config-airbnb
- anything that actually solves this problem for me would need to work with users leaving "extends": "airbnb"
as-is.
I agree with @ljharb here. Since eslint is very particular about the naming of packages and how plugins / configs are used, it's a pretty big problem that we have to migrate all of our users to a new package and deprecate the old one to solve this.
Would it make sense to allow plugins to define a "default configuration" that would be used when the user has only a name of a plugin in extends
(the way shareable configs can be used at the moment)? It seems that this way "extends": "airbnb"
could continue to work with a new eslint-plugin-airbnb
that would specify a default configuration.
@fson i like that idea but there already exist conflicting eslint-config-foo
and "eslint-plugin-foo
with a default config" packages all over the place.
@ljharb I think it's still worth trying out first. If it proves to be working well with plugins, we could look into ways of either having ESLint handle configs as plugins, or allow ESLint to extend configs and plugins through the same mechanism. So far this is the first suggestion that we've had that seems like it might work, so I wouldn't want to dismiss it without trying it out first.
@ilyavolodin there's certainly nothing wrong with improving plugins. I just want to make it clear that if it requires people to change their eslintrc or not be able to depend on the same eslint-config-airbnb
, then it won't actually resolve the issue.
@fson we actually used to define a default plugin config, but we decided it was too big of a surprise to users to have a plugin automatically enforce a config just because you included it. I don't think we want to go back there.
@ljharb sometimes improvements require breaking changes, unfortunately. It seems like in this case, the change from "airbnb" to "plugin:airbnb/foo" is easily solved with simple find-and-replace, as would be the change from "react/foo" to "airbnb/react/foo" or something else. So you could probably write a small utility to help people convert their existing configs if it proves to be a major pain point.
We can also help the cause by formally deprecating the eslint-config-*
format and encouraging people to use plugins from now on (which it seems was my original thinking way back when).
@nzakas If you accept that there's no way to make configs solve this problem, then of course it's required - but I don't think that's been proven in any way. If eslint wants to formally deprecate configs, then obviously my hand is forced (altho that's much better than informally deprecating them). (side note: i hope "extends": "plugin:airbnb"
would be possible, so people wouldn't have to couple to a plugin's sub-config name)
I'm sure someone could write a utility to convert eslint-config-x
into eslint-plugin-y
, especially if a plugin can specify a default config (that required explicit extending ofc, not that was applied automatically), but that's a pretty big burden to impose on users.
@ljharb I understand your point of view and concerns. I do believe it's been proven we can't do this in configs, and I don't think it's worth the time to continue down that path.
It would be really helpful if you (or anyone on this thread) could try to create a plugin as I described and see if it works for your use case. What we need now is feedback on what I described so we can move forward or not.
@ljharb
I'm sure someone could write a utility to convert eslint-config-x into eslint-plugin-y, especially if a plugin can specify a default config (that required explicit extending ofc, not that was applied automatically), but that's a pretty big burden to impose on users.
prob its easy to write codemod for this
I'm in agreement with a few other people on this thread that this would be a very unsatisfying solution.
we actually used to define a default plugin config, but we decided it was too big of a surprise to users to have a plugin automatically enforce a config just because you included it. I don't think we want to go back there.
I agree that it would be confusing for plugins to automatically enforce a config. But that's because enforcing a config by default is precisely the behavior that shareable configs are designed for. If we deprecate shareable configs and tell people to use plugins instead, then we need a good replacement for that behavior.
It feels like we're getting too caught up in the edge cases here, and it's causing us to throw out the idea of usable shareable configs entirely, even when it would make things easier in the vast majority of cases. The whole point of a shareable config is to be able to publish a style guide to npm, and allow users to enforce it without having to worry about configuration details. One of the design goals of ESLint is that users should be able to create custom rules so that core doesn't need to support everyone's requests. But as it currently stands, a shareable config publisher can only use ESLint's core rules, otherwise they incur an installation burden on all their users. For example, I can't use a rule from eslint-plugin-node
in my shareable config without forcing users to install eslint-plugin-node
manually. The fact that a particular rule comes from eslint-plugin-node
, rather than core, is an implementation detail that that shareable configs are supposed to avoid.
This is enough of a problem that people use packages like standard
, which is just a wrapper around ESLint that also has a preset config. Standard is popular because it makes it easy to just follow a styleguide, without worrying about configuration. If a beginner wants to download airbnb's style guide and enforce it on their code, they shouldn't have to worry about the distinction between plugins, shareable configs, and plugin-specific configs. They should be able to run npm install eslint-config-airbnb
and put one extends:
line, and have everything just work.
While I agree that we need to account for edge cases such as multiple versions of the same plugin, our solution shouldn't complicate enforcing a styleguide in general just because a shareable config decides to use a rule from a plugin instead of a core rule. Even implementing an inelegant solution for these edge cases would be better than forcing users to worry about the internals of a shareable config whenever it includes a plugin. While my proposal here has a lot of flaws (some of which might be fixable), I still think it would be a distinct improvement over the current behavior.
Here's an updated proposal.
tl;dr:
plugin/rule
, and can optionally be preceded by a chain of dependent configs separated by slashes. For example, if the current config depends on foo
, and foo
depends on bar
, and bar
depends on plugin
, the rule can be referenced with foo/bar/plugin/rule
. (syntax up for bikeshedding)bar
does not depend on any config nonexistent
, the reference foo/bar/nonexistent/plugin/rule
is invalid. ESLint should throw an error if it encounters an invalid reference.foo/bar/plugin/rule
, it is not required that bar
explicitly depend on plugin
.bar
depends on baz
, and baz
depends on plugin
, then foo/bar/plugin/rule
is considered equivalent to foo/bar/baz/plugin/rule
. In this case, foo/bar/plugin/rule
is called an abbreviated reference, which expands to the full reference foo/bar/baz/plugin/rule
.bar
also depends on qux
, and qux
depends on a different version of plugin
, then the reference foo/bar/plugin/rule
is ambiguous. (It could refer to either foo/bar/baz/plugin/rule
or foo/bar/qux/plugin/rule
.) If ESLint encounters an ambiguous reference (defined as an abbreviated reference that could expand to more than one possible full reference), it should throw an error. It might also output a list of possible full references, so that the user can easily clarify which reference they intended to refer to.Suppose my project depends on eslint-config-airbnb
, which uses rules from eslint-plugin-import
. I could configure these rules the same way that I currently do:
yaml
rules:
import/no-unresolved: warn
extends:
- airbnb
Now I decide to also use eslint-config-node
, which depends on a different version of eslint-plugin-import
. So I install it and add it to my config file:
rules:
import/no-unresolved: warn
extends:
- airbnb
- node
At this point, ESLint will throw an error, because I'm configuring the import/no-unresolved
rule but it's unclear which version of eslint-plugin-import
I'm referring to. To fix this, I can use a more specific reference:
rules:
airbnb/import/no-unresolved: warn
extends:
- airbnb
- node
Note that I only need to change my own config file, and I don't need to change anything in eslint-config-airbnb
. The reference to import
in eslint-config-airbnb
is still unambiguous, because eslint-plugin-node
is not in eslint-plugin-import
's dependency chain.
One disadvantage to this proposal is that if it's implemented precisely as stated above, adding a plugin to a shareable config would be a breaking change, because it could cause a reference to become ambiguous. For example, suppose my config looks like this:
rules:
import/no-unresolved: warn
extends:
- airbnb
- google
Since eslint-config-airbnb
depends on eslint-plugin-import
, but eslint-config-google
does not, this reference is unambiguous, so the config is valid. However, if eslint-config-google
adds eslint-plugin-import
internally in a minor release, my config will break because the reference import/no-unresolved
will become ambiguous.
I don't see this as a major concern, because if eslint-config-google
starts depending on eslint-plugin-import
, it's likely that the config turns on new rules from eslint-plugin-import
, which would be a breaking change anyway.
Edit from a year later: I realized that this solution doesn't work well when using extends:
with a relative path. Maybe there is a way to modify it to support that.
That proposal sounds awesome and seems like it would solve my issue completely - as an added bonus, most people would just be able to trim down their package.json and wouldn't have to edit a single line of config!
@not-an-aardvark Your proposal address collisions with multiple configs/plugins of different version. It does not address the changes needed to allow configs to have hard dependency on plugins, however. How would you handle (with the same code) scenarios where plugin exposes shareable config and it's own rules and is extended by another shareable config? How would plugin with shareable config refer to it's own rules anyways?
@ilyavolodin Thanks for the feedback on the proposal. However, I don't understand some of the issues you raised.
Your proposal address collisions with multiple configs/plugins of different version. It does not address the changes needed to allow configs to have hard dependency on plugins, however.
I'm not sure I understand the distinction you're making. As far as I can tell, the proposal would allow configs to have a hard dependency on plugins (that was the main design goal). Could you clarify why you don't think it would work?
How would you handle (with the same code) scenarios where plugin exposes shareable config and it's own rules and is extended by another shareable config? How would plugin with shareable config refer to it's own rules anyways?
This would work the same way as it currently does. The plugin's shareable config, and also a config that directly extends the plugin's shareable config, would both refer to the plugin's rules as plugin-name/rule-name
.
Let me introduce an example of shareable configs which are using plugins.
Somebody don't need to install the plugins to use the config.
This config depends on 3 plugins; [eslint-plugin-eslint-comments], [eslint-plugin-mysticatea], [eslint-plugin-node].
However, we can install the config with npm install --save-dev eslint eslint-config-mysticatea
then we can use it. Users don't need to install the plugins manually.
npm@2
installs the plugins as peerDependencies
.npm@>=3
installs the plugins as dependencies
and flattens those.In both cases, it would notify INVALID_PEER
errors if it happened version conflicts of the plugins.
So version conflicts of the plugins are notified as installation errors, we can act to resolve it.
I have been using this mechanism while 1.5 years and I have never encountered any problems.
So honestly I wonder if this complex feature requires truly.
@mysticatea relying on the automatic installation of peer deps, while also relying on npm 3+'s flattening, is certainly a very clever trick, but in no way do I think that's a proper solution to this problem. Additionally, because of the way your peer deps are defined, if I already have an existing top-level dep on eslint-plugin-node
at, say, 2.x or 4.x - it will satisfy the peerDep requirement, but not satisfy the dependencies requirement - which will cause duplicates to be installed in my tree in npm 2, and the conflict will fail to hoist in npm 3 - which could either break, or silently do the wrong thing. It's fortunate that you've had no problems but this is a very brittle solution - one which is only lightly lessened if your peer+normal deps are pinned.
If a reference to a plugin rule is ambiguous, ESLint throws an error and tells the user to be more specific.
References to plugins are resolved such that the end user always has the ability to resolve an ambiguity.
I know this is a long thread, but I pointed out earlier that placing this burden on the end user is unfair and not something I support. It shouldn't be up to end users to understand a magical dependency path to get a rule to work -- that burden should be on the shareable config developer alone. Shareable configs should be black boxes to their consumers, anything more is going far beyond what is either expected or useful.
I'm still waiting to hear prototype feedback about using plugins instead. That seems to solve all of the problems on this thread without any changes. I'd like to ask again for some volunteer(s) to try out that approach and post feedback here.
I don't see any reason to keep coming up with other proposals until someone tries out the plugin approach to either prove or disprove it's viability as a solution.
@nzakas i'm still unclear on how i can try out that approach without changes in eslint, since plugins that my plugin depends on still currently need to be top-level deps.
@nzakas Thanks for the feedback.
It's unclear to me how shareable configs can be considered "black boxes". Even with their current behavior, the user has to know a lot about a config's internal dependencies and structure. For example, the user has to configure a rule based on the name of the plugin that it comes from (e.g. import/no-unresolved
). Also, with the current behavior, the user has to manually install all of a shareable config's dependencies (although I realize that this is due to changes in npm and is not by design). If configs were really black boxes, the userule wouldn't have to worry about details like this. I think making the user consider a config's dependencies only in exceptional cases (i.e. when two versions are installed) is much better than making the user consider the dependencies every time they install a shareable config. As I mentioned above, it seems right now we're optimizing for very uncommon edge cases while making the most common use-case significantly less convenient.
@nzakas The problem I have with your approach, is it sort of promotes marketing somebody else's work as your own. While in your example you export rules out as react/ruleName
, not everyone might do that since it's not enforced in any way, in that case, to the end user it will look like rules that are configured by current plugin are also written by the same person. This makes for ambiguous ownership as well as might make it harder to report bugs with the rule in appropriate repository.
This bite today, and after reading through this thread I think it would be best if eslint itself shipped any sort of upgrade tool, like babel-doctor
/brew doctor
, that could handle and changes in syntax. E.g. extends: "airbnb"
-> extends: "plugin:airbnb"
, and since this would be a one-time thing I do think it's OK to force upon end users.
One disadvantage of this is how to know what the equivalent plugin would, and for that the config needs to somehow specify that. That could probably go into the package.json, or exported as a special property.
But what to do with legacy configs? First off, the same doctor
command could warn and suggest/fix sharable configs, but unless you go with some sort of rules like @not-an-aardvark suggests. Alternatively, or perhaps complementary, is to analyze the peer dependencies for the end user and sync it with their packages.json. This would allow any config install to be reduce to npm install --save eslint-config-foo; eslint sync/doctor/whatever
, or even eslint install --config foo
.
Not sure if I like the latter though, but such an approach would "solve" the issue today from the eyes of an end user, and allow smooth upgrading when a good solution exists. (Stripping out the peer dependencies and update the user config).
I'm sure there's many things I missed, but the important point is to ship an official doctor/upgrade command when the time comes.
Shameless plug.
If you need an example I used the hints in this thread to create an eslint-plugin with bundled dependencies: eslint-plugin-react-app.
@mmazzarolo What happens if there's two conflicting copies of one of your plugin dependencies in the tree?
@ljharb I'll be honest: I don't know, I haven't played with them yet.
I have been using this lib for a while on multiple projects without any issue, but I'm almost sure that my strategy would not suit at all your use case.
I was reading through this thread again, and I noticed a point of confusion:
@nzakas i'm still unclear on how i can try out that approach without changes in eslint, since plugins that my plugin depends on still currently need to be top-level deps.
I don't think @nzakas's proposal would require any changes to ESLint itself. In that proposal, people would share their configs using plugins, and the plugins could internally call require
on other plugins and export their rules. For example, one could create eslint-plugin-airbnb
containing code like this:
const importPlugin = require('eslint-plugin-import');
module.exports.rules = {
'no-unresolved': importPlugin.rules['no-unresolved'],
// ...
};
ESLint wouldn't care that eslint-plugin-airbnb
got the rule from eslint-plugin-import
-- as far as ESLint is concerned, the rule would be airbnb/no-unresolved
. This would mean that eslint-plugin-airbnb
could have eslint-plugin-import
as a dependency, not a peerDependency.
There are still a few downsides of this proposal, (described here and here), but I wanted to clarify that it does not require any changes to eslint to work.
Thanks for clarifying.
Indeed, those downsides means that I could not ship a package that worked in this way without breaking a large number of users eventually.
+1 on this issue. Would love to have it as well.
I've attempted the "plugin exposing config and specifying other plugins as dependencies" approach highlighted above by @nzakas, following @mmazzarolo's eslint-plugin-react-app
example.
It works, but I found another problem/downside: if the config exposed by the plugin extends
other configs, the plugin needs to rewrite the configured plugin rules in _all_ of this tree of extends
to prefix them with its name.
extends
– prefix one level deep only (eg. what eslint-plugin-react-app does).Without this traversing/prefixing, the linting would show errors coming from the bundled plugins directly. Oh and I think this goes beyond rules – since we can't rely on extends
(rules would not be prefixed), then all of the configuration may need to be recursively merged together during the traversal.
My ESLint knowledge only goes so far so please let me know if I've missed something. Here is my real-world example, and (hacky) solution to this problem, relying on ESLint's CLIEngine#getConfigForFile
to compute the final config before prefixing the rules.
Edit: extra food for thought – while this approach technically works without relying on brittle transitive dependencies, I find the idea of prefixing all plugin rules with my config (company's) name quite ugly. This makes configs and inline overrides slightly harder to reuse beyond our projects, and slightly harder to search for.
Trying to put together a shared config and running into these issues as well. I tried to go down the plugin approach but agree that it feels hack-ish, especially when you consider that you have to walk the entire tree as @thibaudcolas described.
I like @not-an-aardvark's proposal, it makes sense in most cases and like he points out adding plugins in a config will likely force a breaking change anyways, so it's ok to make it always happen. Alternatively, we could disallow plugin resolution from a config altogether. If you want to override a rule provided by a plugin, you would _have_ to install the plugin at the level you're trying to override it at.
I may have missed it but I'm curious, does the proposal solve for plugins not resolving when using npm link - https://github.com/eslint/eslint/issues/6222 ?
I've just submitted a PR to https://github.com/nathanhleung/install-peerdeps that allows doing this in the postinstall
script: https://github.com/okonet/eslint-config-okonet/pull/4
Update: there is a related open issue https://github.com/nathanhleung/install-peerdeps/issues/11 but once it is resolved, it's a workaround I'm sticking with for now.
I've read pretty much the whole thread and in the end I'm a bit confused. Is using a plugin instead of a shareable config the way to go to solve this problem or is still undecided at this point?
Thanks all for your detailed explanations and reflexions on this matter.
Using a plugin in the first place, with exported configs, indeed avoids this problem.
For authors of a shared config that want to solve the peer dep UX problems without a breaking change, we remain at the impasse of "there's viable suggestions; core will refuse to implement one until a user land prototype exists; but a userland prototype can't possibly solve the problem, so core would have to implement it first".
In other words, this problem probably can never be fixed until eslint core decides to embrace the risk and ship a solution attempt.
@ljharb Alright thanks for your answer.
I'm an author of a shared config but it has not been published yet so I guess I'm free to proceed with the plugin exporting config solution. By the way does that solution allow testing your plugin using npm/yarn link as well? I haven't tried it yet, I shall try tonight.
If your shared config is public, can you kindly post a link once you got it working? Maybe other will be able to learn from it.
Until there is a better solution, the current situation is very frustrating!
@mbrevda here are configs that have been created following the "config in plugin" approach:
eslint-plugin-react-app
eslint-plugin-springload
(of which I'm the author)@thibaudcolas/eslint-plugin-cookbook
(also author)Both of these are prototypes following the approach that has been suggested above, and they demonstrate its shortcomings, which are:
react/react-in-jsx-scope
becomes springload/react/react-in-jsx-scope
.eslint-disable
comments) need to be updated.extends
other configs requires resolving the whole tree of config extends
, so that all of the references to rules can be renamed.I'm quite happy with my prototype anyway, it became much easier to use this config in projects, and it also made it easier to introduce new plugins within it. However, migrating from eslint-config-springload
to the config(s) exposed by eslint-plugin-springload
is indeed a breaking change and UX issue (rule names are different).
@mbrevda I could indeed post it here once I finish it if some more examples are desired.
@thibaudcolas Thanks for sharing those, and especially the downside listing. Your repo is nicely documented, thanks for sharing it. :+1:
@nzakas given https://github.com/eslint/eslint/issues/3458#issuecomment-333036299 (the prototype feedback you requested on using plugin configs instead, in https://github.com/eslint/eslint/issues/3458#issuecomment-268709610) - which indicates that plugin configs are not a satisfactory replacement for shared configs - are you now willing to explore a change in eslint core to try to remove the usability issues with peer deps in shared configs?
@nzakas what is there to lose by taking a crack at this in eslint core? As a newcomer to this issue I am surprised to see that it was opened in 2015 yet still no good solutions seem to exist. All of the hacks up till this point which I have tried (plugins/postinstall scripts) seem to have significant downsides.
I am sure it is more complicated than at face value but it seems like bundling dependencies is something javascript should be good at.
Just my two cents after having struggled to get the hacks to work. Thanks!
npm5 _almost_ solves this by flattening dependencies
(including those of devDependencies
of the top-level project). Unfortunately, its flattening logic is inconsistent (see npm/npm#18960). If that bug gets resolved, then you could do
// @my/eslint-config/package.json
"dependencies": {
"@my/eslint-config": "1.2.3"
}
// @my/eslint-config/index.js
extends: ['@other']
and eslint's module-loader will find @other/eslint-config
just fine.
@jamesarosen that's not "solved"; version conflicts will still have nesting, as they should.
version conflicts will still have nesting, as they should
Right! I had forgotten about the case where multiple configs might depend on different versions of the same common config.
And that suggests some added benefits of solving this in eslint's module-loader (building on @thibaudcolas's comments above):
eslint-disable foo/shared/no-foo bar/shared/no-foo
extends: ['foo', 'bar']
already determines that bar/shared
wins over foo/shared
lmao @ the "needs bikeshedding" label!
Sorry for the time between replies. As many are aware, I've been dealing with health issues and have not been able to keep up with conversations regularly.
@ljharb If I'm reading @thibaudcolas https://github.com/eslint/eslint/issues/3458#issuecomment-333036299 correctly, it seems like my suggestion actually worked fine and as expected. The change to rule names is what I explicitly mentioned and will definitely lead to some trouble when changing from another config to a plugin config. That's just the cost of admission for this approach.
@tnrich There is nothing to "take a crack at" in core. There is no accept proposal for solving this right now. I think we'd all be happy to have a solution to this problem in the core, we just haven't been able to come up with one.
For everyone: I don't think this was clear from my previous comments, but I'm not against solving this problem of configs relying on plugin rules. I am against trying to shoehorn that behavior into the current shareable config functionality because it was never intended to be used in this way. I know that's not an answer people will be happy with, but it's the truth.
Shareable configs was something I came up with after we implemented extends
in configs, as I realized using require
meant that an npm package could be used as well. At the time it seemed like an easy win, but if I had to do it over again, I would have spent more time to flesh out a full plugin proposal instead. I never anticipated shareable configs referencing plugin rules so unfortunately, there's no affordances for that nor is there an easy way to add them.
That's why this issue has been open for so long. It's not that we are putting off implementing, it's that we don't have a design to implement. Currently, this is a problem without a solution except for the plugin method I mentioned earlier (which I know isn't ideal, but at least you can get something working).
I have identified another fix for this issue which does not require writing a plugin. We wanted a centralised configuration shared across projects and managed to achieve it like so. This solution is only really suitable if you control the projects consuming the shared config.
eslint.config.js
) as a JS file like this example. Nothing special, it's just your eslint config exported.module.exports = {
extends: ['standard', 'plugin:react/recommended', 'prettier', 'prettier/react', 'prettier/standard'],
plugins: ['react', 'prettier', 'standard'],
parser: 'babel-eslint',
parserOptions: {
sourceType: 'module',
ecmaFeatures: {
jsx: true
}
},
env: {
es6: true,
node: true,
jest: true
},
rules: {
'prettier/prettier': 'error',
'react/no-unused-prop-types': 'error'
}
}
node_modules
. This is key. It means eslint searches for plugins/configs in the node_modules
of your shared config package, rather than the project consuming it:{
"name": "my-common-linting",
"version": "0.1.0",
"bin": {
"eslint": "./node_modules/eslint/bin/eslint.js"
},
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
},
"dependencies": {
"babel-eslint": "8.2.2",
"eslint": "4.19.0",
"prettier": "1.11.1",
"eslint-plugin-import": "2.9.0",
"eslint-plugin-node": "6.0.1",
"eslint-plugin-prettier": "2.6.0",
"eslint-plugin-promise": "3.7.0",
"eslint-plugin-react": "7.7.0",
"eslint-plugin-standard": "3.0.1",
"eslint-config-prettier": "2.9.0",
"eslint-config-standard": "11.0.0"
}
}
.eslintrc.js
file that looks like this:module.exports = require('my-common-config/eslint.config.js')
{
"scripts": {
"lint": "eslint ."
}
}
eslint.nodePath
to ./node_modules/my-common-linting/node_modules
. You can then commit these editor settings alongside your code. I'm not sure about other editors like Atom.One thing you lose with this solution is the ability to easily override rules at a project level. But you could conceivably solve this by making eslint.config.js
export a function where you can pass in additional config.
Its also not a solution for an uncontrolled environment where you want an easily-consumed library (because of the editor configuration required). Though I think you could just set eslint as a peerDependency and it could possibly work. Though, still, it means you break out of the normal "extends" pattern so this solution is not suitable for library authors.
None of this is ideal, but it works for my simple case of wanting to share configuration amongst internal projects and have the config and the versions of the plugins centrally controlled.
@adamscybot this seems like it’s a race condition between whether your package, or eslint itself, installs and creates its “bin” first.
@ljharb In the solution, eslint is only included in the shared package, not in the project that is being linted. Since npm only links binaries from direct dependencies (afaik?) there is no race condition.
If I’m wrong about that though you could just give is a different name.
File tree to visualize:
└─ node_modules
├─ .bin
│ └─ eslint -> ../my-common-config/node_modules/eslint/bin/eslint.js
└─ my-common-config
├─ eslint.config.js
└─ node_modules
├─ .bin
│ └─ eslint -> ../eslint/bin/eslint.js
└─ eslint
I would encourage future feedback to continue in #10643.
Is there any update / resolution for this issue?
@M3kH see https://github.com/eslint/rfcs/pull/5 for an open RFC related to this
It looks like something about this issue is still unresolved, but FYI I was able to resolve an import error by upgrading to ESLint 6. (I was previously getting the error ESLint couldn't find the plugin "eslint-plugin-import"
, related to the fact that our shared package for linting config had it as a dependency.)
@mbrowne it has, and still should have, to be a peer dependency, not a regular one. You may be benefiting from hoisting, but that’s not reliable.
@ljharb Ah, I see...
To clarify for others (assuming I understand correctly), it works as long as your shared config package is the only dependency that depends on the eslint plugins you're using. (Otherwise there could be version discrepancies, in which case the plugins will be installed in nested node_modules
folders instead of the top-level node_modules
for your project.) That's what makes it "not reliable".
Summary of problem
Let's revisit this topic of enabling ESLint to resolve a plugin package _relative to the config file that references it_ (instead of relative to the project folder, or based on a manual --resolve-plugins-relative-to
CLI option).
Example motivating scenario: Say I have a local project my-project
that gets its ESLint config and plugins from a shared package eslint-config-my-shared-defaults
. The local config should look something like this:
my-project/.eslintrc.js
module.exports = {
extends: "eslint-config-my-shared-defaults",
// resolveRelativeToConfigFile: true,
parserOptions: {
tsconfigRootDir: __dirname
},
};
The issue is that the eslint-config-my-shared-defaults
package is going to load a bunch of plugins. My project just wants to use these plugins -- but it doesn't want to configure their versions or manage them. I mean, suppose there are 5 plugins and 200 such projects in my monorepo -- it's a pain to have to specify 5 x 200 = 1,000 dependencies that are all copy+pasted duplicates! Thus eslint-config-my-shared-defaults
should be the only needed dependency, like this:
my-project/package.json
{
"name": "my-project",
"devDependencies": {
"eslint": "^6.0.0",
"eslint-config-my-shared-defaults": "~1.2.3"
}
}
Today, this produces an error because ESLint will try to load a plugin like @typescript-eslint/eslint-plugin
, but it can't find it as my-project/node_modules/@typescript-eslint/eslint-plugin
. If you're using NPM, it may get flattened into that location, but what version we'll get is random. And that path won't exist at all for people whose node_modules
folder was installed by PNPM or Rush or Yarn Plug 'n Play (or even npm link
).
What we really want is for ESLint to resolve a plugin package _relative to the config file that referenced it_. This is the modern convention used by TypeScript, TSLint, Jest, etc. It generally works great. If your shared config wants to delegate its versions via peer dependencies, it works great for that, too.
Maybe the problem isn't as hard as we think
The https://github.com/eslint/rfcs/pull/5 proposed the same idea a long time ago. But that discussion seemed to get hung up on concerns about duplicate plugin names:
Implementing this scheme as-is would cause naming ambiguity when referring to rules. For example, there could be two shareable configs which have dependencies on two different plugins that both happen to be called
react
This concern seems somewhat artificial, though. TSLint/Jest/etc don't bother to address that problem, and don't seem to encounter any trouble in practice. People don't load /that/ many plugins, and naming conflicts seem like something the plugin authors could sort out among themselves, rather than waiting for a complex disambiguation feature from ESLint.
As of ESLint 6, I found that the referencing config file is now tracked by ESLint's module resolver. Thus, for my own case, apparently we can "fix" this by changing just one line of code as follows:
try {
// filePath = ModuleResolver.resolve(request, relativeTo);
filePath = ModuleResolver.resolve(request, importerPath);
} catch (resolveError) {
This causes plugins to get resolved relative to importerPath
(the path of the referencing config file) instead of the the local project folder. (There's one other place that could be fixed as well, although that didn't seem to be needed for my setup.)
Proposed change
Thus, rather than waiting for a major overhaul of ESLint config files, what if we added a simple option resolveRelativeToConfigFile
as suggested above. This option would enable the behavior change that I outlined above. We would recommend for libraries NOT to use this option -- instead, it's meant to be used by a local project's .eslintrc.js that wants to "opt-in" to this alternate behavior. Presumably people won't "opt-in" without confirming whether it works correctly for their setup.
This isn't an elegant or perfectly correct solution, but it allows us to experiment with a different approach. I suspect it would meet the needs of most people who wanted RFC 5. It's also very little work to implement.
Does this seem like a good idea? Would such a PR be accepted by ESLint?
Here's a monkey patch:
const path = require('path');
let currentModule = module;
while (!/[\\/]eslint[\\/]lib[\\/]cli-engine[\\/]config-array-factory\.js/i.test(currentModule.filename)) {
if (!currentModule.parent) {
// This was tested with ESLint 6.1.0; other versions may not work
throw new Error('Failed to patch ESLint because the calling module was not recognized');
}
currentModule = currentModule.parent;
}
const eslintFolder = path.join(path.dirname(currentModule.filename), '../..');
const configArrayFactoryPath = path.join(eslintFolder, "lib/cli-engine/config-array-factory");
const configArrayFactoryModule = require(configArrayFactoryPath);
const moduleResolverPath = path.join(eslintFolder, "lib/shared/relative-module-resolver");
const ModuleResolver = require(moduleResolverPath);
const originalLoadPlugin = configArrayFactoryModule.ConfigArrayFactory.prototype._loadPlugin;
configArrayFactoryModule.ConfigArrayFactory.prototype._loadPlugin = function(name, importerPath, importerName) {
const originalResolve = ModuleResolver.resolve;
try {
ModuleResolver.resolve = function (moduleName, relativeToPath) {
// resolve using importerPath instead of relativeToPath
return originalResolve.call(this, moduleName, importerPath);
};
return originalLoadPlugin.apply(this, arguments);
} finally {
ModuleResolver.resolve = originalResolve;
}
}
You can paste this at the top of your .eslintrc.js file. Alternatively, I added it to my shared config package, so my .eslintrc.js file starts with the line require("eslint-config-my-shared-defaults/patch-eslint");
which is pretty concise. So far it seems to solve the problem.
If other people try this and it find that does NOT work, please share your repro so I can debug it. Otherwise, if this seems to be a good solution, I can follow up with a proper PR.
Would it be easier to patch the installed eslint
package with that one-liner change using something like patch-package?
Sure, this would be fine for a small standalone project. For a business monorepo, tampering with another package's node_modules folder is dangerous. I can imagine all sorts of things it might break. The most obvious one is that the folder may be shared with other projects in the repo, or other branches and/or repos that are built on the same lab machine.
@octogonz I'm getting the following error when trying to follow your steps:
yarn eslint src/showLoadingMask.js
yarn run v1.17.3
$ /Users/tnrich/Sites/teselagen-react-components/node_modules/.bin/eslint src/showLoadingMask.js
currentModule.filename: /Users/tnrich/Sites/teselagen-react-components/node_modules/eslint/bin/eslint.js
Cannot read config file: /Users/tnrich/Sites/teselagen-react-components/.eslintrc.js
Error: Failed to patch ESLint because the calling module was not recognized
Error: Cannot read config file: /Users/tnrich/Sites/teselagen-react-components/.eslintrc.js
Error: Failed to patch ESLint because the calling module was not recognized
at Object.<anonymous> (/Users/tnrich/Sites/teselagen-react-components/node_modules/eslint-config-teselagen/patch.js:8:11)
at Module._compile (internal/modules/cjs/loader.js:776:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:787:10)
at Module.load (internal/modules/cjs/loader.js:653:32)
at tryModuleLoad (internal/modules/cjs/loader.js:593:12)
at Function.Module._load (internal/modules/cjs/loader.js:585:3)
at Module.require (internal/modules/cjs/loader.js:690:17)
at require (internal/modules/cjs/helpers.js:25:18)
at Object.<anonymous> (/Users/tnrich/Sites/teselagen-react-components/.eslintrc.js:1:1)
at Module._compile (internal/modules/cjs/loader.js:776:30)
I added a console log to see what file it is failing for. Any ideas as to what is going wrong? Thanks!
Are you using ESLint 5? This feature requires ESLint 6+.
@octogonz I don't know if this is really the most appropriate place to have this discussion but I haven't been able to get it to work now that I'm using eslint v6. Maybe you could post an example repo where your method is working? Thanks!
Will do. BTW I'm reachable on gitter.im if you want to follow up offline.
@tnrich If you want to share a repro branch for your issue, I'd be happy to debug it. I'm almost done converting my repo which can serve as an example, but I realized that seeing it work may not give much insight into why your project is failing.
Maybe you could post an example repo where your method is working?
@tnrich Here's a real life repo that illustrates how the patch works: .eslintrc.js
Thanks @octogonz I was able to get it working. I'll give you more feedback if I run into any issues with it. Are there any caveats or gotcha's with your patch?
I'm using it for a few large react apps extending Create React App's config and some smaller node projects. Here's the link https://github.com/TeselaGen/eslint-config-teselagen
Is there any reason why we can't make a PR fixing this issue? It seems that some people figured it out and I would love to have this feature.
If other people try this and it find that does NOT work, please share your repro so I can debug it. Otherwise, if this seems to be a good solution, I can follow up with a proper PR.
I'm using the patch that @octogonz made in ~6 repos now (some primarily reactjs code and some just node code) and haven't hit any issues with it.
I'm waiting for prettier-eslint for VS Code to upgrade to eslint v6 (https://github.com/prettier/prettier-eslint/pull/236) for my vscode prettier to work with this.
I met the sasme problem when I try to make a custom eslint plugin.
@octogonz I have tested your patch and it works excellent, using ESLint 6.3.0. Could you please consider making a PR, sir?
I made it work closer to where eslint
is ignoring the module resolution, this works for all configurations, no just for plugins
.
This works with workspaces or whatever since it is just Node resolution from now on.
import * as path from 'path';
const eslintFolder = path.join(path.dirname(require.resolve('eslint')), '..');
const moduleResolverPath = path.join(
eslintFolder,
'lib/shared/relative-module-resolver'
);
const ModuleResolver = require(moduleResolverPath);
ModuleResolver.resolve = function (moduleName: string) {
return require.resolve(moduleName);
};
I spent the past few days making my own ESLint configuration, which I'd like to now share across all my projects without polluting their listed dependencies and I've just stumbled upon this issue.
I don't have a general solution to all the hypothetical issues mentioned here, but I have a few ideas I think that could be implemented pretty easily which would solve a useful subset of this issue, for starters:
Why not adding a settings.trustMeThereAreNoVersionsMismatches
setting, perhaps with a less silly name, that would allow ESLint to just import all plugins listed in extended configurations anyway, trusting the user who's telling you that there won't be any conflicts?
This isn't the ideal solution, but al least it would allow this useful use case for many people, without forcing them to either litter their listed dependencies with plugins used only in external configurations or to wrap ESLint entirely in custom CLI tools.
@octogonz I tried your monkey-patch with yarn Plug'n'Play and got different allocation errors. For example:
<--- Last few GCs --->
[54340:0x102883000] 112188 ms: Scavenge 2031.4 (2050.6) -> 2030.9 (2050.9) MB, 7.5 / 0.0 ms (average mu = 0.093, current mu = 0.014) allocation failure
[54340:0x102883000] 112199 ms: Scavenge 2031.9 (2050.9) -> 2031.5 (2051.1) MB, 6.4 / 0.0 ms (average mu = 0.093, current mu = 0.014) allocation failure
[54340:0x102883000] 112214 ms: Scavenge 2032.2 (2051.1) -> 2031.5 (2051.4) MB, 7.7 / 0.0 ms (average mu = 0.093, current mu = 0.014) allocation failure
<--- JS stacktrace --->
==== JS stack trace =========================================
0: ExitFrame [pc: 0x10092fa99]
1: StubFrame [pc: 0x10098e4cb]
Security context: 0x2502495408a1 <JSObject>
2: mapInternal(aka mapInternal) [0x250217f93511] [/Users/7rulnik/Library/Caches/Yarn/v6/npm-prettier-1.17.0-53b303676eed22cc14a9f0cec09b477b3026c008-integrity/node_modules/prettier/index.js:4204] [bytecode=0x25020f3db001 offset=38](this=0x2502fec404a9 <undefined>,0x250217f94201 <Object map = 0x25021e5d6141>)
3: map [0x...
FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory
1: 0x10007e71b node::Abort() [/Users/7rulnik/.nvm/versions/node/v12.12.0/bin/node]
2: 0x10007e89f node::OnFatalError(char const*, char const*) [/Users/7rulnik/.nvm/versions/node/v12.12.0/bin/node]
3: 0x100176137 v8::Utils::ReportOOMFailure(v8::internal::Isolate*, char const*, bool) [/Users/7rulnik/.nvm/versions/node/v12.12.0/bin/node]
4: 0x1001760d3 v8::internal::V8::FatalProcessOutOfMemory(v8::internal::Isolate*, char const*, bool) [/Users/7rulnik/.nvm/versions/node/v12.12.0/bin/node]
5: 0x1002fa185 v8::internal::Heap::FatalProcessOutOfMemory(char const*) [/Users/7rulnik/.nvm/versions/node/v12.12.0/bin/node]
6: 0x1002fb854 v8::internal::Heap::RecomputeLimits(v8::internal::GarbageCollector) [/Users/7rulnik/.nvm/versions/node/v12.12.0/bin/node]
7: 0x1002f8727 v8::internal::Heap::PerformGarbageCollection(v8::internal::GarbageCollector, v8::GCCallbackFlags) [/Users/7rulnik/.nvm/versions/node/v12.12.0/bin/node]
8: 0x1002f670d v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, v8::internal::GarbageCollectionReason, v8::GCCallbackFlags) [/Users/7rulnik/.nvm/versions/node/v12.12.0/bin/node]
9: 0x100301e24 v8::internal::Heap::AllocateRawWithLightRetry(int, v8::internal::AllocationType, v8::internal::AllocationAlignment) [/Users/7rulnik/.nvm/versions/node/v12.12.0/bin/node]
10: 0x100301e9f v8::internal::Heap::AllocateRawWithRetryOrFail(int, v8::internal::AllocationType, v8::internal::AllocationAlignment) [/Users/7rulnik/.nvm/versions/node/v12.12.0/bin/node]
11: 0x1002cd879 v8::internal::Factory::NewFixedArrayWithFiller(v8::internal::RootIndex, int, v8::internal::Object, v8::internal::AllocationType) [/Users/7rulnik/.nvm/versions/node/v12.12.0/bin/node]
12: 0x1004dbbf8 v8::internal::BaseNameDictionary<v8::internal::NameDictionary, v8::internal::NameDictionaryShape>::New(v8::internal::Isolate*, int, v8::internal::AllocationType, v8::internal::MinimumCapacity) [/Users/7rulnik/.nvm/versions/node/v12.12.0/bin/node]
13: 0x1004abc07 v8::internal::JSObject::MigrateToMap(v8::internal::Isolate*, v8::internal::Handle<v8::internal::JSObject>, v8::internal::Handle<v8::internal::Map>, int) [/Users/7rulnik/.nvm/versions/node/v12.12.0/bin/node]
14: 0x1004c3ec0 v8::internal::LookupIterator::Delete() [/Users/7rulnik/.nvm/versions/node/v12.12.0/bin/node]
15: 0x1004a30bd v8::internal::JSReceiver::DeleteProperty(v8::internal::LookupIterator*, v8::internal::LanguageMode) [/Users/7rulnik/.nvm/versions/node/v12.12.0/bin/node]
16: 0x10060710e v8::internal::Runtime::DeleteObjectProperty(v8::internal::Isolate*, v8::internal::Handle<v8::internal::JSReceiver>, v8::internal::Handle<v8::internal::Object>, v8::internal::LanguageMode) [/Users/7rulnik/.nvm/versions/node/v12.12.0/bin/node]
17: 0x10060d5a7 v8::internal::Runtime_DeleteProperty(int, unsigned long*, v8::internal::Isolate*) [/Users/7rulnik/.nvm/versions/node/v12.12.0/bin/node]
18: 0x10092fa99 Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_NoBuiltinExit [/Users/7rulnik/.nvm/versions/node/v12.12.0/bin/node]
19: 0x10098e4cb Builtins_DeletePropertyStrictHandler [/Users/7rulnik/.nvm/versions/node/v12.12.0/bin/node]
error Command failed with signal "SIGABRT".
@7rulnik if you are able to share a repro branch, I can take a look at it. Not sure why you're getting a C++ call stack, though. (Also, make sure you are using an LTS version of NodeJS.)
@yordis great code
Here's a PR that implements the resolveRelativeToConfigFile
setting proposed above:
@yordis does your solution work with external ESLint invokers, such as the VS Code Add-in? (That's why I had modeled my solution as a monkey patch.)
@octogonz that is a good question, I am not sure.
I do see my VS Code showing me the errors, but it didn't fix it automatically.
What was the mistake I made? It seems you knew about it.
We already discussed this in #2518 with the conclusion that a
peerDependency is the way to go.
Then I will never be able to use my own eslint config with create-react-app because ultimately my own eslint plugin versions will be overridden by react-scripts.
What speaks against setting the require-root to the location of the config file that is requesting the plugin instead of the CWD
?
imo babel does this right by allowing a require
(or any other function you want to resolve with) to resolve the pluging closest to itself.
This works fine in npm3/yarn workspace installs with multiple mismatching version.
I would also prefer code over these string DSLs:
extends: [
'plugin:import/typescript',
],
@mjangir not without ejecting, yes - that’s part of the design of CRA.
@igl It’s been stalled for a little while now, but this RFC might interest you. Please feel free to jump in the discussion!
This would be of huge benefit to mono repositories, where nested packages (like React's CRA) have their own extends
properties, allowing unit/style/lint tests to be run from the root for the entire project, which is much faster and cleaner than executing each package's own test setups. Prettier and Jest already follow this philosophy, allowing per-package configuration. The only workaround I have found is to hoist all dependencies using Lerna.
https://github.com/yarnpkg/berry/issues/8 with Yarn 2 Plug'n'play it becomes even more crucial.
@sindresorhus is there anything with which I can help to push this forward? Any advices how to start resolving configs in Babel-style?
@sindresorhus I've implemented a suboptimal but working solution for Yarn 2 with PNP https://github.com/yarnpkg/berry/issues/8#issuecomment-594274272
It would be 10 times easier if instead of resolve-plugins-relative-to
CLI/Node API option this parameter can be set in config...
We will look to solve this problem through this RFC: https://github.com/eslint/rfcs/pull/9
The RFC is still in development but has been accepted and the basic approach has already been prototyped and verified.
@nzakas I appreciate your open source work and I am thrilled to hear that this issue is being addressed!
@nzakas eslint/rfcs#9 looks very promising, thank you very much for sharing it. Do you have any ETA when it can be released?
The RFC needs to be finalized first, then I can start developing the prototype into the final solution. Our priority right now is getting 7.0.0 released, so this won’t happen until afterwards.
@nzakas understood, thank you. As a short term solution can you consider adding resolvePluginsRelativeTo
to the config? It will allow to make configuration self-contained (incl all subsequent configs/plugins) and will unblock shareable configs with PNP...
I’m not in favor of making any more changes to the current config system. It’s already quite complicated and fragile, and I don’t see the benefit of further complicating it in the short-term when a long-term solution is coming.
@nzakas although I do understand motivation not to change, the problem is that the timeline is uncertain and there is no good solution for the issue for Yarn 2, which is almost released. So basically anything that can unblock will be good.
If config var is too complicated even ENV var will be OK (in addition to CLI arg).
Please take a look at the ugly workaround https://github.com/yarnpkg/berry/issues/8#issuecomment-594274272 — it should not be that bad )))
@nzakas I've made another ugly workaround, maybe someone will find it helpful.
I've created a simple patch script in my shared config that brutally replaces resolvePluginsRelativeTo
in ESLint code. It works from console, works from WebStorm, and probably will work from VSCode too: https://github.com/ringcentral/ringcentral-javascript/blob/master/eslint-config-ringcentral-typescript/src/bin/rc-eslint-patch.js
#!/usr/bin/env node
/* eslint-disable @typescript-eslint/no-var-requires */
const fs = require('fs');
const patchPaths = [
require.resolve('eslint/lib/cli-engine/cascading-config-array-factory'),
require.resolve('eslint/lib/cli-engine/config-array-factory')
];
const pkg = require('../../package.json');
const token = 'resolvePluginsRelativeTo = cwd';
const addon = token.replace('cwd', `require.resolve('${pkg.name}') || cwd`);
patchPaths.forEach(path => fs.writeFileSync(path, fs.readFileSync(path).toString().replace(token, addon)));
Then in the app users need to add {"scripts": {"postinstall": "rc-eslint-patch"}}
.
And then unplug eslint to allow script to make changes:
$ yarn unplug eslint
Looks like at the moment it's the only option since there are no other escape latches.
Thanks for the workaround in the meantime @kirill-konshin! 🙌
I've got a question regarding 7.0.0 release notes: https://eslint.org/blog/2020/02/whats-coming-in-eslint-7.0.0#plugins-loaded-from-config-file-directory
In v7.0.0, plugins will be loaded relative to the configs that reference them. You can read more in the RFC.
This is a confusing statement. Seems that it means just the root config, not the ones that it extends.
Are there any plans to fix this issue?
If you are working in a monorepo, your natural instinct is to bundle all your plugins, extends, and rules into a single dependency to declutter your dependencies, and share this amongst all your packages. But it seems this is not possible.
I would assume that most people would come to eslint thinking that the config would behave the same as babel where you can do require.resolve('plugin')
or require('plugin')
.
It might be worth adding a note to the docs to make it clear that it is not possible to pass filenames to extends like in babel. There is an error message that detects filenames, but you will get some other wierd errors if you pass a module.
WebStorm + CLI
ESLint > Extra ESLint options
= --resolve-plugins-relative-to=<full-path-to-shareable-config>
EDIT: WebStorm not working. See issue.
Create a wrapper script or add to npm scripts: eslint --resolve-plugins-relative-to=<full-path-to-shareable-config>
If you are working in a monorepo, your natural instinct is to bundle all your plugins, extends, and rules into a single dependency to declutter your dependencies, and share this amongst all your packages. But it seems this is not possible.
@vjpr I solved it using the monkey patch described in this comment. It patches ESLint's module resolution to work the way you would expect.
I later opened PR https://github.com/eslint/eslint/pull/12460 to propose integrating this feature into ESLint, however that PR got hamstrung by the RFC process.
I gave up because the monkey patch works perfectly for our needs:
If people show interest in merging the PR, I would consider revisting it. But the general impression was that the ESLint maintainers prefer to completely revamp the module loader architecture versus accepting a minimal (and opt-in) workaround for the existing architecture. The concern was that it might be difficult to support.
@octogonz The workaround I posted works is currently working for me. Need to patch IntelliJ though because its missing one line config option here.
The workaround I posted works is currently working for me. Need to patch IntelliJ though because its missing one line config option here.
I did consider --resolve-plugins-relative-to
but for our case it had a couple issues:
For me this now got even worse with eslint v7.
We are using spire to run eslint in our projects. How it works is simple: in a project I only install spire
and a spire-config
. The config depends on eslint
, eslint-configs and eslint-plugins. The configs we use have a hierarchy, for example typescript-node-config extends nodejs-config which extends base-config
. Each of the configs might use other plugins which they peerDepend on. The spire-config
in the end begins everything together by fulfilling these peerdeps.
When using eslint v7 the plugins are not found anymore, because yarn is not hoisting them anymore to the top. Not sure why though.
Imho instead of resolving the plugins from the entry-config it should be resolved from the config file that mentions/uses them.
So for example /project/.eslintrc.js
extends /other/config.js
which uses a plugin, then the plugin should be resolved from /other/node_modules
. Maybe if people have the need for the current way there could be a fallback to /project/node_modules
if not found in the other location.
I have a use case similar to @danez (not with spire, but a custom build infrastructure) where we wrap eslint in a CLI tool which configures a base configuration bundled in the same NPM package as the CLI. The monkey-patch we had working with eslint v6 now has to be updated after the breaks in v7.
As others have briefly mentioned above, I believe this situation gets worse with package managers which don't hoist like yarn, such as pnpm. So I hope https://github.com/eslint/rfcs/pull/9 does support isolating plugin dependencies to the configs where they are enabled, and that change is rolled out soon before further breaks to ESLint config resolution.
Here's the moneky-patch for v7:
#!/usr/bin/env node
/* eslint-disable @typescript-eslint/no-var-requires */
const fs = require('fs');
const patchPaths = [
// require.resolve('eslint/lib/cli-engine/cascading-config-array-factory'),
require.resolve('eslint/lib/cli-engine/config-array-factory')
];
const pkg = require('../../package.json');
const token = 'internalSlotsMap.set(this, {';
const addon = `resolvePluginsRelativeTo = require.resolve('${pkg.name}');\n ${token}`;
patchPaths.forEach(path => fs.writeFileSync(path, fs.readFileSync(path).toString().replace(token, addon)));
We call it as bin script of our shared config.
Here is our version of the monkey-patch for eslint v7 (adapted from @octogonz https://github.com/eslint/eslint/issues/3458#issuecomment-516716165 above):
/**
* Definition from eslint/lib/cli-engine/config-array-factory
*/
interface ConfigArrayFactoryLoadingContext {
/** The path to the current configuration. */
filePath: string;
/** The base path to resolve relative paths in `overrides[].files`, `overrides[].excludedFiles`, and `ignorePatterns`. */
matchBasePath: string;
/** The name of the current configuration. */
name: string;
/** The base path to resolve plugins. */
pluginBasePath: string;
/** The type of the current configuration. This is `"config"` in normal. This is `"ignore"` if it came from `.eslintignore`. This is `"implicit-processor"` if it came from legacy file-extension processors. */
type: "config" | "ignore" | "implicit-processor";
}
export function dangerouslyPatchESLint() {
const ModuleResolver = require("eslint/lib/shared/relative-module-resolver");
const { ConfigArrayFactory } = require("eslint/lib/cli-engine/config-array-factory");
const originalLoadPlugin = ConfigArrayFactory.prototype._loadPlugin;
ConfigArrayFactory.prototype._loadPlugin = function (_name: string, ctx: ConfigArrayFactoryLoadingContext) {
const originalResolve = ModuleResolver.resolve;
try {
// Resolve using current config filePath instead of `relativeToPath`
ModuleResolver.resolve = function (moduleName: string, _relativeToPath: string) {
return originalResolve.call(this, moduleName, ctx.filePath);
};
return originalLoadPlugin.apply(this, arguments);
} finally {
ModuleResolver.resolve = originalResolve;
}
};
}
Update: Since people found it useful, we've released our patch as a standalone NPM package: @rushstack/eslint-patch
I updated the implementation to work with both ESLint 6.x and 7.x. We'll maintain it until whenever ESLint can provide an official solution. Thanks all!
An update on this from ESLint team: The official way we will be supporting plugins as dependencies in shareable configs is through the new simple config system (eslint/rfcs#9). You can follow the implementation progress here:
https://github.com/eslint/eslint/issues/13481
I was under the impression, that this problem was finally fixed in v7. According to the announcement:
ESLint will now resolve plugins relative to the entry configuration file. This means that shared configuration files that are located outside the project can now be colocated with the plugins they require.
Is it just me misunderstanding or is the announcement misleading?
I think the announcement was not misleading intentionally, but definitely confusing. I also initially read it and thought "great news", and then saw that the related issues were still open.
@octogonz , thank you for your @rushstack/eslint-patch!
Unfortunately, it didn't solve the problem with my very simple configuration:
require("@rushstack/eslint-patch/modern-module-resolution");
Still, react-scripts start
fails with "Failed to load config "react-app" to extend from"
Therefore I decided to rollback to yarn 1.22 for now.
I'll appreciate any assistance in solving the issue.
@milichev Please open a Rush Stack issue and include some repro steps. That's where we are providing support for @rushstack/eslint-patch
. Thanks!
@milichev you may also want to take a look at this thread here https://github.com/facebook/create-react-app/issues/9446. Your setup is very similar to the one I'm using _(except for the eslint-patch — I don't use that, I just follow this thread for other reasons)_ and with a little workaround everything works for me now.
It might be worth noting that npm v7 will automatically install peerDependencies, so everything declared as a peer dependency by a shared ESLint config will get installed.
Most helpful comment
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.