New: Load plugins relative to the configs that reference them

[not-an-aardvark]

Rendered version of this RFC

Summary

This change builds on #7 by updating ESLint's config-processing logic to load plugins relative to the configs that reference those plugins, allowing shareable configs to bundle plugins as dependencies. ESLint would throw an error when it encounters multiple plugins with the same name. Additionally, this change would also add support for loading plugins by filepath.

This is similar to #13, but it has smaller scope (it only addresses plugin-loading rather than overrides and config arrays), and provides a bit more detail on the options and tradeoffs related to plugin-loading. I think considering the issues independently might make it easier to discuss each issue.

Related Issues

• Support having plugins as dependencies in shareable config eslint#3458
• Ability to load plugins by path, not just name eslint#6237
• First-class support for project-specific linting rules eslint#8769
• New: Load configs and plugins relative to where they're referenced #5
• Rule: Warn when semicolons are missing eslint#9
• Update: Config File Improvements #13

[mysticatea]

Oh, I had misunderstood about #7; I had thought loading plugins relatively from the importer (e.g. shareable configs) has been accepted. 😅

[not-an-aardvark]

#7 loads shareable configs and parsers relative to the importer, but it loads plugins relative to the CWD. So shareable configs still need to use peerDependencies for plugins, but the plugins will get loaded reliably regardless of the user's package manager. (It was more of a big bugfix than an enhancement, although it should prevent a lot of confusion for users.)

This RFC allows plugins to be loaded relative to the importer, in the same way as shareable configs and parsers. That avoids the need for shareable configs to use peerDependencies for plugins, but it requires handling plugin conflicts somehow (as described in more detail within the RFC).

[ExE-Boss]

We’d need to do this in a way that is compatible with pnpm, which #9 would be by design.

/cc @zkochan

[not-an-aardvark]

@ExE-Boss I'm not familiar with pnpm specifically, but this proposal is expected to work regardless of what package manager was used to install packages, by using something like Module.createRequireFromPath to use the same module-loading logic that Node does.

[mysticatea]

No. I don't think extends is considered a "load" of a plugin because it doesn't define any rules to be identified to a unique name.

The extends: ["plugin:foo/bar"] uses the configs.bar property of eslint-plugin-foo, but it doesn't register the plugin to the plugin manager. This means it doesn't define any rules and envs.

[not-an-aardvark]

I think there are a few options:

1. Consider extends: ["plugin:foo/bar"] to be a "load" of a plugin (and raise an error if plugins: ["foo"] is used from a different config file)
2. Don't consider extends: ["plugin:foo/bar"] to be a "load" of a plugin. Instead, require plugins: ["foo"] to be used somewhere else, and use the already-loaded plugin to resolve plugin:foo/bar. (This is the option described in the "Open Questions" section.)
3. Don't consider extends: ["plugin:foo/bar"] to be a "load" of a plugin. However, still resolve extends: ["plugin:foo/bar"] from the location of the config file that contains it, even if plugins: ["foo"] appears in a different config and resolves to a different plugin.

Personally, I think (2) is the best option.

(3) is logically consistent, because extends: ["plugin:foo/bar"] doesn't introduce any rules/envs. However, I think it could be confusing in practice, because:

• Usually, a config from a plugin depends on the plugin's rules. So if the plugin config contains a self-load with plugins: ["foo"], it would create a conflict anyway.
• If the plugin config doesn't contain plugins: ["foo"], then the config from the plugin could be applied to rules from a different version of the plugin, potentially causing confusing schema errors.
• Users might be surprised that a plugin's shareable configs are from a different version of the plugin than the plugin's rules.

[mysticatea]

Hm. I prefer the 1 in those options because:

1. extends:"plugin:foo/bar" is enough clear to use the plugin foo.
2. people are used to such behavior because it's the popular usage that the recommended preset contains plugins setting.

[not-an-aardvark]

I think the advantage of (2) over (1) would be that it allows more valid configurations. However, let's discuss #14 (comment) first, since if we implement that suggestion, it would be unnecessary to use (2).

[not-an-aardvark]

Now that the discussion at #14 (comment) is resolved:

I think (2) is better because users occasionally might want to use extends: [plugin:foo/bar] for a shareable config's plugin, and if they can't use extends: [plugin:foo/bar] then it's sometimes difficult to make their config do what they want. On the other hand, if we make the change then it's easy for a user to restore the old behavior by simply adding plugins: ["foo"] to their config, and it would allow us to support extends: [plugin:foo/bar] for a shareable config's plugin.

[mysticatea]

I tried to abandon my adherence to option 1, but I failed by some technical issues.

Option 2 doesn't fit #13. Option 2 means that we cannot determine extends until it makes the final config for each file because we can write plugins setting in overrides. So we cannot build ConfigArray under the option.
In #13, I hope to make the order that it loads configs fully at first then determines the config for each file without fs. Probably I have to stick at option 1 or 3.

[not-an-aardvark]

Option 2 means that we cannot determine extends until it makes the final config for each file because we can write plugins setting in overrides.

In order to allow #10 to work, plugin names need to be globally unique across all files. It's not possible to have one version of eslint-plugin-foo loaded for *.js and another version of eslint-plugin-foo loaded for *.ts, because formatters need to obtain unique metadata for the rule foo/bar. So I think we'll be able to resolve plugin configs before determining the final config for each file anyway, even if we use Option 2.

To satisfy the constraint from #10, we could say that all plugins directives that appear in overrides are "hoisted". For example:

module.exports = {
    ...base,
    overrides: {
        ...overridesParams,
        plugins: ["foo"]
    }
};

would be equivalent to this config:

module.exports = {
    ...base,
    plugins: ["foo"],
    overrides: {
        ...overridesParams
    }
};

[mysticatea]

Thank you for pointing out. I had overlooked the impact of #13 for #10.

[not-an-aardvark]

Given this new information, do you think Option 2 from #14 (comment) is feasible? If so, I want to update this RFC so that we can have a final version before Thursday's TSC meeting.

[mysticatea]

It may be feasible, but I guess that it needs additional changes regardless of #13.

• Without Update: Config File Improvements #13, it loads config files from the leaf to the root on directories. We have to change this order with keeping the root property functionality. This might be tough because plugin's config can have root property.
• With Update: Config File Improvements #13, as same, it needs to change the order for the ancestors of CWD. And it needs a parameter that is the loaded plugin pool to load because it cannot access the config array elements which have been yielded in the past on loading. This may make a pro of Update: Config File Improvements #13, the removal of registrations, weak.

Therefore, I'm still prefer the option 1 or 3. That a loading logic for each config file doesn't depend on other configs makes the loading logic simpler much.

---

By the way, if we go with 2, how about the mix of 1 and 2? It looks to cohabit backward compatibility and increasing valid configs.

• If the relevant plugin has been loaded already, it uses the loaded plugin instance. Otherwise, it considers extends: ["plugin:foo/bar"] to be a "load" of a plugin (and raise an error if plugins: ["foo"] is used from a different config file).

[not-an-aardvark]

After thinking about this more, it would be nice if plugins: ["foo"] always had the same behavior as plugins: [require.resolve("eslint-plugin-foo")], since this would make it easier to normalize configs with absolute paths. But this seems difficult to accomplish because ESLint might not be able to determine that the name of the plugin is foo when given require.resolve("eslint-plugin-foo"). Maybe we could support some other way to provide both an absolute path and a name.

[mysticatea]

I have an idea about local plugins: https://github.com/eslint/rfcs/blob/eslintrc-improvements/designs/2019-eslintrc-improvements/minor-03-plugin-renaming.md

[not-an-aardvark]

That seems like a good idea for local plugins. However, how would it save people from the "plugin conflict" error?

edit: Never mind, I just saw this file.

[mysticatea]

I'm considering if we can check package.json.

• When it found a plugin in a config file, it reads package.json of the package which contains the config file. If the dependencies field doesn't include the plugin, it excludes the config file from conflict check.

This means, the following cases will be ignored in conflict check naturally:

• shareable configs which use peerDependencies for plugins
• plugin configs which loads itself
• .eslintrc files which use plugins that shareable configs imported (as expecting the package manager hoists the plugins)

[not-an-aardvark]

Hmm, checking package.json doesn't seem appealing to me because then we really would be duplicating the behavior of a package manager. I think there are lots of other cases (e.g. optionalDependencies, local packages within other packages, etc.) that we would need to deal with -- it seems like it would work in common cases but it could break due to unrelated features of package managers.

I think the cases are worth considering separately:

• To simulate shareable configs that have peerDependencies (where multiple configs can use the same version of the same plugin):

  I think any solution here needs to be opt-in for shareable configs. Otherwise, a shareable config could start depending on a different version of the plugin and accidentally break its users.

  But if shareable configs are opting in anyway, then maybe they could just add eslint-plugin-foo as a peerDependency, remove plugins: ["foo"] from their config, and require the user to supply plugins: ["foo"]. Then the user would be supplying the plugins: ["foo"] directive directly.

plugin configs which loads itself

I think we can just keep the existing special case here -- it adds a small amount of complexity, but doesn't really cause major problems.

.eslintrc files which use plugins that shareable configs imported (as expecting the package manager hoists the plugins)

I think this would already be adequately solved by using option (2) from #14 (comment).

[mysticatea]

I don't want to touch the details of a package manager, too. But, I thought touching the minimum spec of package.json (rather than a package manager) to relax conflict errors may be nice because the confliction matter causes by the change of package.json.

I want to keep the current behavior that we can use extends:"plugin:foo/bar" without plugins field.

[not-an-aardvark]

I'm thinking that I should withdraw this RFC from consideration for v6.0.0 because it doesn't seem to be ready yet.

By the way, what would you all think about adding a new keyword such as ownedPlugins to address the peerDependencies use case? The only difference would be:

• A plugin specified with plugins would be loaded from the user's project root
• A plugin specified with ownedPlugins would be loaded from the config file that contains it.

Then we would have a clear way to address the existing peerDependencies use case. We would recommend that shareable configs with peerDependencies use plugins, and we would recommend that shareable configs with dependencies use ownedPlugins.

The motivation is that this would avoid breaking existing use cases where a user depends on two shareable configs that depend on the same version of a plugin. (As-is, this RFC would not provide any recourse for users in that case.) We would probably encourage "styleguide" shareable configs to use ownedPlugins, and we would encourage other shareable configs to use plugins.

[mysticatea]

Sounds fine to me (if it's in ESLint 5.x, it's excellent).

... Or how about https://github.com/eslint/rfcs/blob/eslintrc-improvements/designs/2019-eslintrc-improvements/minor-03-plugin-renaming.md and require.resolve() function (or just require() function)? It's also a small minor change and shareable config owner can control plugin's locations.

[not-an-aardvark]

I'm not sure plugin renaming is a good solution to the plugin-conflict problem because most of the time, I think the user would want to merge configs from the same plugin rather than configuring the two plugins separately. (With #5, it was possible to have multiple reports from the same rule, which would be annoying.)

However, plugin renaming could be independently useful to support loading plugins by path.

(Note: If we introduce ownedPlugins, I think we would still want to throw an error if there is a conflict between ownedPlugins and ownedPlugins, or ownedPlugins and plugins.)

[mysticatea]

• If a plugin is a module name, it loads the plugin relative to CWD as is currently.
• If a plugin is a file path renamed (e.g. require.resolve("eslint-plugin-foo")), it considers that the config file which includes it does control the plugin version fully. Therefore, if another file has the same plugin ID regardless of renamed or not, it throws "plugin conflict" error.

In other words, I think that my renamed plugins are complete same as your ownedPlugins.

[not-an-aardvark]

Good point. I agree that renamed plugins can handle the same cases as ownedPlugins.

As a minor note, I think it could be better UX to implement plugin renaming as a separate property:

module.exports = {
    plugins: [
        "foo", // loaded from cwd
        "bar" // loaded from cwd
    ],
    localPlugins: {
        baz: "./packages/eslint-plugin", // loaded relative to config file as eslint-plugin-baz
        quux: require.resolve("eslint-plugin-quux") // absolute path, result is that plugin is loaded relative to shareable config
    }
}

Otherwise, it could be confusing that different plugins in plugins are loaded from different places, and it seems like it wouldn't be possible to use both kinds of plugins simultaneously.

---

In comparison to the current peerDependencies solution, I think the main downside of having shareable configs do this is that if eslint-config-foo introduces eslint-plugin-bar as a localPlugin, a user would be unable to use eslint-plugin-foo at the same time as any other shareable config that also uses eslint-plugin-bar. (I think this would necessarily be true for any robust solution that allows configs to specify plugins as dependencies. I'm just pointing it out explicitly since it might make the feature less appealing to shareable config authors.) However, it still seems worthwhile to allow plugins to be loaded by path anyway, and shareable configs can decide whether or not to use the feature.

[mysticatea]

Indeed, different property such as localPlugins sounds good UX.

[mysticatea]

In comparison to the current peerDependencies solution, I think the main downside of having shareable configs do this is that if eslint-config-foo introduces eslint-plugin-bar as a localPlugin, a user would be unable to use eslint-plugin-foo at the same time as any other shareable config that also uses eslint-plugin-bar.

I think this workaround utility is valid.

[not-an-aardvark]

This is a summary of the discussion on this RFC so far, and an overview of solutions that have been considered.

(ccing @nzakas since this is relevant to my technical concern with #9)

The design problem we're trying to solve is that it seems difficult to satisfy all of the following constraints simultaneously:

1. Shareable configs can control what versions of plugins they use (i.e. specify them as dependencies). This is the feature request from Support having plugins as dependencies in shareable config eslint#3458.
2. Shareable configs can't inadvertently break end users by upgrading the plugins that they depend on. Similarly, plugins can't inadvertently break end users with a patch release. (This is the "robustness constraint" from this overview.)
3. When two plugins don't conflict (e.g. when two shareable configs both depend on the same plugin, but the versions happen to be compatible), users can load the same plugin through multiple shareable configs, possibly with cooperation from one or more of the shareable configs.

In the current version of ESLint, we satisfy Constraint 2 and Constraint 3 but we've repeatedly punted on Constraint 1. (This is the "Don't change anything" solution.)

With earlier versions of #13, as well as the current version of #9 (where the same plugin can be loaded from multiple places, provided that the plugin has the same object identity), we satisfy Constraint 1 and we sometimes satisfy Constraint 3 depending on the package manager, but we don't satisfy Constraint 2. (This is the "Allow npm deduping to resolve plugin conflicts" solution, sometimes referred to as "naive conflict handling" in other discussions.)

As currently written, this RFC (as well as an earlier version of #9) satisfies Constraint 1 and Constraint 2 but does not satisfy Constraint 3. (This is the "Strictly check for plugin conflicts" solution.)

Possible solutions

Allowing shareable config authors to choose (the "local plugins" solution)

We could let shareable configs decide whether they want to "own" their plugins or "share" them with other shareable configs. An "owned" plugin would be a dependency of a shareable config, and loading the plugin from any other config would cause an error (to ensure that the "owner" has the freedom to upgrade the pluging arbitrarily without breaking any other configs). On the other hand, a "shared" plugin would be a peerDependency with the current behavior, and it could be used by multiple shareable configs since only the end user would have the freedom to upgrade it.

In practice, this could look something like the example config from #14 (comment). We would simply allow plugins to be loaded by filepath, and a shareable config would represent that it "owns" a plugin by using that plugin's absolute path.

This solution seems appealing because it addresses all three constraints above. It also requires no breaking changes, and being able to load a plugin by path would be very useful in its own right (it would address the big ergonomics issue of allowing people to easily use project-specific rules that aren't published as an npm plugin).

As a downside of this solution, shareable configs would be imposing significant restrictions on users if they chose to "own" a plugin, since users would be unable to use any other shareable configs that depend on the same plugin without using additional utilities like this. While these restrictions are necessary to satisfy constraint (2) above, it's unclear whether the feature would still be appealing as a solution to eslint/eslint#3458 with these restrictions present.

It's also not clear how this general idea would work if we decide to implement #9. (In principle it seems like it could be possible, but it would probably require substantial changes since #9 currently prevents ESLint from knowing what a plugin is.)

(Note: This solution has also been called "plugin renaming" in previous discussions.)

Dropping a constraint

We could drop one of the three constraints. Below, I've described what dropping each constraint would look like, followed by my personal opinion on how much of a problem it would be to drop that constraint.

Dropping Constraint 1 (the "Don't change anything" solution)

Dropping Constraint 1 would mean that the end user always has the ability to control which versions of plugins get used. This would avoid the possibility of having a shareable config break a user by upgrading a plugin (since shareable configs can't control the versions of plugins), and it would also allow the end user to mix-and-match shareable configs in cases where no plugin conflicts occur.

In practice, we would implement this by explicitly deciding not to address eslint/eslint#3458, and otherwise leaving the behavior as-is. (It's not quite clear how this would fit into #9 -- perhaps ruleDefinitions would only be allowed in the "top level" of a config rather than in nested configs.)

Personally, I actually wouldn't mind doing this. If we implement #9, my main concern would be that this might make the new config format less consistent/intuitive. If we don't implement #9, my main concern would be that this would make it harder to support loading plugins by path in the future.

Dropping Constraint 2 (the "Allow npm deduping to resolve plugin conflicts" solution)

Dropping Constraint 2 would mean that shareable configs can control which versions of plugins they use. Users would also be able to use multiple shareable configs that depend on the same plugin, provided that the package manager deduplicates packages. (However, users' builds could unexpectedly break if the shareable configs decided to update their plugins.)

In practice, we would probably implement this by allowing two plugins to be loaded only if they have the same object identity (i.e. they are located in the same place in the filesystem).

Personally, I'm strongly opposed to doing this because it breaks the "robustness constraint" from this overview. This would give the illusion that shareable configs have control over plugin versions, when in fact they wouldn't be able to safely update their plugins without the risk of breaking users. Effectively, we would be telling shareable config authors to just hope nothing goes wrong, and I don't think stable tools should do things like that.

Dropping Constraint 3 (the "Strictly check for plugin conflicts" solution)

Dropping Constraint 3 would mean that shareable configs can specify plugins as dependencies, but ESLint would enforce that no plugin can be loaded by more than one shareable config at a time. This would ensure that shareable configs can control which versions of plugins they use, and can also upgrade those plugins freely without the risk of breaking the user's config. However, this would prohibit some currently-valid configurations where an end user depends on multiple shareable configs, both of which depend on the same version of a plugin. (If we gave shareable configs the ability to upgrade their plugins, then they might start conflicting with each other later on, which is not possible with the current behavior because only the end user can update plugins.)

The implementation of this solution is described in this RFC (link to current version). This could also be implemented as part of #9, but it seems like the usability/conflict problems would be more serious. (This RFC has a special case for plugin-provided configs which allows some additional setups that wouldn't break robustness. It seems like this special case wouldn't be possible to replicate with #9 because ESLint wouldn't have any notion of whether something is a plugin config.)

Personally, I'm not strongly opposed to this but I'm concerned about breaking existing setups with no recourse.

---

While the discussion is still ongoing, if we decide to drop a constraint then my current order of preference would be to drop Constraint 1, then Constraint 3, then Constraint 2.

[ExE-Boss]

Well, pnpm mostly ensures that Constraint 3 is always met, at least as long as a strict dependency deduplication strategy is used.

/cc @zkochan

[btmills]

Thank you all for the discussion in this thread and particularly @not-an-aardvark for putting the RFC together! Now that we've decided on a direction to take configs going forward in #9, the TSC decided today to close the other config-related RFCs so we can focus on the new format. You can follow along with that implementation work at eslint/eslint#13481.