Declaring shareable configs and plugins in package.json is unreliable

[not-an-aardvark]

As of ESLint v4.19.1, all shareable configs and plugins are loaded from the location of the eslint package. This behavior is clearly documented, and we advise users that if they want their config/plugin packages to be loaded, they should ensure that the packages are placed where eslint can find them.

However, there is a problem with this approach: package managers provide no way for users to ensure that the packages will be placed in a valid way. There are a few common practices that people use to load their configs/plugins, but all of them are actually relying on an implementation detail of a package manager rather than a well-defined behavior.

For example, in order to use a config that depends on "eslint-config-foo" and "eslint-plugin-bar", we recommend putting this in a package.json file:

{
  "devDependencies": {
    "eslint": "^4.19.2",
    "eslint-config-foo": "1.0.0",
    "eslint-plugin-bar": "1.0.0"
  }
}

This package.json file imposes a requirement on a package manager: the eslint, eslint-config-foo, and eslint-config-bar packages must be accessible from the root package. However, it does not impose any requirement that the packages are accessible from each other, which is actually what the user needs.

It happens to be the case that with the most common package management strategy, the packages are accessible from each other anyway, because the packages are laid out like this:

(my project root)/
├── .eslintrc.json
└── node_modules/
    ├── eslint/
    └── eslint-config-foo/
    └── eslint-plugin-bar/

But I want to stress that this is an implementation detail of package managers, and not a required behavior. As a result, some package management strategies arrange packages in a way that breaks ESLint. For example, lerna would sometimes arrange packages like this:

(monorepo root)/
├── node_modules/
|   ├── eslint/
|   └── eslint-plugin-bar/
└── packages/
    ├── eslint-config-foo/
    |   └── index.js
    └── (my project root)/
        ├── .eslintrc.json
        ├── package.json
        └── node_modules/
            └── eslint-config-foo (symlink to (monorepo root)/packages/eslint-config-foo)

In this example, lerna is fulfilling all of the requirements in package.json: eslint, eslint-config-foo, and eslint-plugin-bar are all reachable from (my project root)/. However, ESLint fails to run because eslint-config-foo is not reachable from eslint.

It should be noted that adding a peerDependency of eslint in a config/plugin doesn't help in this case. If eslint-config-foo has a peerDependency of eslint, this imposes a requirement that the same version of eslint is reachable from both the project root and from eslint-config-foo. However, there is no requrement that eslint-config-foo is reachable from eslint. In the package layout above, all peerDependencies would be satisfied, but eslint would still be broken.

Similarly, we recommend that shareable configs include all of their plugins as peerDependencies, but this is also unreliable, because it only imposes a requirement that the plugin is reachable from the config and the project root. It doesn't impose a requirement that the plugin is reachable from the eslint package.

---

To summarize the problem, config file loading is generally fragile when using a package manager, because eslint has an unusual requirement about package layout and the user has no way to ensure that the requirement is satisfied. As a result, a lot of people end up getting confused about why eslint can't find a package, and while there are solutions that appear to work in most cases, the solutions break down when the user has an uncommon-but-valid package management workflow.

I think the current plugin/config-loading system was designed when npm (specifically npm<=2) was the only prevalent package manager that people used for Node packages. As a result, the current system encourages a pattern with peerDependencies that happened to work well when using npm2, but actually relied on implementation details rather than a well-defined behavior. Now that it's common for people to use other package management strategies (such as lerna and yarn workspaces), I think it's important that we upgrade to be compliant with how packages are supposed to work, so that users can declare their dependencies robustly.

When discussing this before, we've described it as a distinction between "local" and "global" installations, and we encourage users to install all packages "locally". However, I think this oversimplifies the issue because it assumes that everything will work fine if packages are installed locally. In fact, all of the packages in the examples above have packages installed "locally", but they still can cause ESLint to break.

After going back and forth a few times, this problem has convinced me that we should fix the "global versus local" issue by resolving all config and plugin names from the location of the config file where they appear. This would make ESLint's behavior consistent with how modules generally work in Node when calling require -- the package that contains the module name is in charge of providing it as a dependency. This behavior also has other benefits of a well-designed module system (e.g. it's possible to have nested dependencies, so modules are encapsulated and there isn't a global namespace of module names).

---

I think the one major blocker for this change is that ESLint can't currently handle loading multiple plugins with the same name, because users need to be able to uniquely refer to a plugin when they configure its rules. With the current system, this can't occur because plugins need to be reachable from eslint in node_modules, which effectively guarantees a namespace without conflicts. (Unfortunately, this method of getting unique plugin names has significant downsides, e.g. it's not possible to load a plugin from a path because it could conflict with a name from node_modules.) This issue has also blocked #3458 and #6237.

To resolve it, I think we should investigate a solution involving plugin namespaces, (along the lines of #3458 (comment)), where users can more precisely define a rule in a config if necessary, in order to avoid naming conflicts. In my opinion, this would be a much better solution than dumping plugins into a global namespace and making the user avoid conflicts at installation time.

[arcanis]

We've started to get reports warning of incompatibilities with ESLint: facebook/create-react-app#5117

Is there a not-perfect-but-good-enough-in-practice solution we can recommend to our users? From the top of my head, these are the possible solutions, and the reason they don't currently work:

• We could recommend them to require the plugins themselves. This doesn't work because ESLint requires the plugins to be plugin names.
• We could recommend them to use require.resolve to give ESLint an explicit path to the module. This doesn't work because ESLint automatically prepends eslint-plugin at the beginning of the absolute path, making it invalid.

Could you consider changing the second point so that /foo/bar will become /foo/eslint-plugin-bar instead of eslint-plugin-/foo/bar? And /foo/eslint-plugin-bar would stay unchanged? I'm open to make the PR myself if I get the greenlight. It won't solve the duplicate package problem, but at least give a workaround until #10643 gets implemented 🙂

cc @ilyavolodin @gaearon

[not-an-aardvark]

TSC Summary: ESLint's plugin- and config-loading mechanism currently depends on implementation details of package managers like npm and yarn, rather than intended behavior. (In short, eslint needs to be able to load plugins like eslint-plugin-react relative to its own location, even though eslint never specifies eslint-plugin-react in its own dependencies.) This causes ESLint to unexpectedly break when using package management tools like Lerna or Yarn Plug N' Play, which is creating problems for downstream tools.

It appears that any solution to this problem would require shareable configs to be able to load their plugins from their own dependencies (or from a config-provided absolute path). As discussed in #3458 and #10643, this would introduce a problem if two loaded plugins have the same name, because the user wouldn't be able to independently configure the rules from the two plugins.

It seems like we have several ways we could proceed:

1. Load plugins from the dependencies of the config that specifies them, and introduce a mechanism for users to disambiguate plugins with the same name (e.g. as proposed in Proposal for loading plugins relative to the configs that depend on them #10643). Note that Proposal for loading plugins relative to the configs that depend on them #10643 is semver-major.
2. Load plugins from the dependencies of the config that specifies them, and throw a fatal error if two loaded plugins have the same name. (This would also be semver-major, although it would require significantly less implementation effort than (1).)
3. Continue loading plugins from the location of the eslint package, but allow configs to specify absolute paths to plugins. Throw a fatal error if two loaded plugins have the same name. (This would not be semver-major, although it would introduce the same potential user confusion (2).
4. Don't change anything, and decide that we don't support using Lerna or Yarn Plug N' Play. I would not recommend this option; ostensibly a linter shouldn't care what package management strategy is being used.

Solution (3) seems to be the only option that fixes the issue without a breaking change. If we adopt solution (3), I think we should commit to also adopting solution (1) or (2) in the next major release, otherwise the situation will end up very messy. In past discussions, the idea of throwing a fatal error when encountering duplicate plugin names (i.e. implementing (2) or (3)) has been very contentious.

I think solution (1) would be the best solution in the long term, but based on past discussions I suspect this opinion might also be contentious.

TSC Question: How should we address this issue?

[not-an-aardvark]

This issue was discussed in today's TSC meeting. The resolution was that we plan to solve this issue with option (1) from #10125 (comment) in a major release. (The solution would probably be #10643, although it's still pending an implementation.) We decided not to add a stopgap solution like option (3) in the meantime, because it might have the potential to create confusion after (1) is implemented, and it might interfere with the solution for (1) if the proposal changes.

[sibelius]

I think we need a guide to how to setup eslint when using yarn workspaces, lerna and monorepos structures

we created this monorepo boilerplate https://github.com/entria/entria-fullstack to experiment with monorepos and be easy to reproduce some issues we are having

[evocateur]

Just run it from the root? There's nothing special about a monorepo as far as eslint is concerned.

…

On Oct 31, 2018, at 05:19, Sibelius Seraphini ***@***.***> wrote: I think we need a guide to how to setup eslint when using yarn workspaces, lerna and monorepos structures we created this monorepo boilerplate https://github.com/entria/entria-fullstack to experiment with monorepos and be easy to reproduce some issues we are having — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[sibelius]

what if we want to have different eslint configs per package?

imagine that I have server, web and app

web should have eslint-plugin-react
app should have eslint-plugin-react-native

and so on

the problem is that having a .eslintrc.js in each package, the eslint won't find eslint plugins because they will be hoisted to root

[ljharb]

I don’t see why they wouldn’t be found; require looks recursively upwards

[evocateur]

Configure from the root as well, using overrides.

…

On Oct 31, 2018, at 09:25, Sibelius Seraphini ***@***.***> wrote: what if we want to have different eslint configs per package? imagine that I have server, web and app web should have eslint-plugin-react app should have eslint-plugin-react-native and so on the problem is that having a .eslintrc.js in each package, the eslint won't find eslint plugins because they will be hoisted to root — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[sibelius]

moving to root works great, tks

the problem is when trying to use together with jest-runner-lint (https://github.com/jest-community/jest-runner-eslint)

as it makes you have one config for each package

[ljharb]

@sibelius you can configure all of those at the root, too, using jest's "projects" feature.

[not-an-aardvark]

Hi everyone, can we move the discussion about how to set ESLint up to the Gitter channel or a new issue? It doesn't really seem related to this issue.

[medikoo]

@not-an-aardvark solution as described at #10643 looks great.

The only usability concern I see (referring to an example from #10643), is that in scenario when both eslint-config-foo and eslint-config-baz depend on same version of eslint-plugin-react,
to tweak some rule from eslint-plugin-react, we will be expected configure our tweaks twice, separately for foo::react and bar::baz::react.

Wouldn't it be better to still accept no prefixes form (so react) in such case, and throw only if given config tweak seems not compatible with used plugin version?

[nzakas]

The ESLint team has just created an RFC process for complicated changes that require designs. This issue was marked as "needs design" and so falls into this new RFC process. You can read more about the RFC process here:

https://github.com/eslint/rfcs/

Thanks!

[nzakas]

There is an RFC related to this issue: eslint/rfcs#5

[not-an-aardvark]

eslint/rfcs#7, which addresses this issue, has been accepted.

[not-an-aardvark]

I've created a PR to fix this issue in #11388. The PR probably won't be merged for awhile (since it's going to have to wait for our next major release), but if this issue has been causing problems for anyone's integrations/package managers/etc., it might be a good idea to double-check that the implementation in the PR fixes those problems.