ESLint

[arcanis]

What package is covered by this investigations?

ESLint

Describe the goal of the investigation

I'd like ESLint to support PnP painlessly. It currently works, but requires a specific logic on our side, which is slightly suboptimal. This investigation aims to figure out how to improve this state.

Investigation report

ESLint currently loads its plugins using a raw require.resolve - meaning that from PnP's point of view, eslint is the package that needs to list the plugins in its dependencies (instead of the package that contains the ESLint configuration file).

This problem is compounded by the fact that ESLint bugs out on absolute paths, meaning that the ESLint configuration files cannot call require.resolve themselves in order to explicitly tell ESLint what's the actual location of the plugins they use.

Because of the PnP fallback mechanism, this problem doesn't occur in most cases (ie when the ESLint configuration file is your own). In this case even is ESLint doesn't list the plugin in its dependency PnP will still check which dependencies are listed at the top-level of your application.

This problem only happens when the ESLint configuration is part of a package you depend on. For example react-scripts, created by create-react-app. In this instance, the plugins aren't listed in the application package.json, meaning that the fallback cannot kick in and do its job.

In order to fix this, a temporary solution has been put into place: react-scripts and gatsby (which are two of the most popular packages using this pattern) are used as secondary fallbacks if the top-level isn't enough. This is but a bandaid, since it causes other problems (other packages can forget to list their own dependencies if react-scripts or gatsby happen to depend on them), but should do the trick for now.

The real fix is eslint/eslint#11388, which is expected to land in ESLint 6.

[not-an-aardvark]

👋 I had a few questions what Plug n' Play will look like with ESLint after eslint/eslint#11388 is merged, and whether the ESLint team might be able to make it better, and it seems like this would be a good place to ask.

With eslint/eslint#11388, ESLint will load plugin relative to its "CWD" (note that the someone invoking ESLint can set the "CWD" that it uses to something other than process.cwd()). This should allow packages like react-scripts to use it reliably, by setting its "CWD" to the location of their own package. (We might add a specific flag for this to avoid other inconvenient side-effects of changing the CWD that ESLint uses.)

• How do editor integrations typically work with PnP? My understanding is that when Plug n' Play is in use, there is no node_modules folder, and Node scripts either need to be invoked with yarn or with something like -r .pnp.js. So if an editor integration needs to load an ESLint plugin to lint a user's project, but it isn't aware of PnP, does it just fail?

• If a user has a create-react-app project and is using an ESLint editor integration, the integration typically won't know that it's supposed to use a different CWD. As a result, it would probably load ESLint plugins relative to process.cwd(), which is typically in the user's project. Would this result in missing-plugin errors?

  (This seems like it would be a broader issue with editor integrations not having access to tools/config files when using things like create-react-app, although it would be good to know in advance if this is going to cause a problem so that we can work around it if needed.)

[arcanis]

👋 !

How do editor integrations typically work with PnP? My understanding is that when Plug n' Play is in use, there is no node_modules folder, and Node scripts either need to be invoked with yarn or with something like -r .pnp.js. So if an editor integration needs to load an ESLint plugin to lint a user's project, but it isn't aware of PnP, does it just fail?

If the editors invoke ESLint through its CLI interface they'll just have to invoke yarn eslint instead of ./node_modules/.bin/eslint and it will just work, regardless of whether the project uses PnP or not. I could imagine a plugin generating a compatibility wrapper in node_modules/.bin each time a package with binary scripts is added to the dependency tree, but ideally it shouldn't be needed.

If the editors do more exotic things like running an internal instance of ESLint that require() the plugins shipped with the project it might cause issues indeed, since they won't know where to look for them (plus they won't be able to open them without further processing, since they are not unpacked). Do you think that will be a problem? On the top of my head I would think this pattern shouldn't occur often, since the plugins might not be compatible with the internal ESLint version anyway - so calling the local binary installed with the project would seem the safest path.

If a user has a create-react-app project and is using an ESLint editor integration, the integration typically won't know that it's supposed to use a different CWD. As a result, it would probably load ESLint plugins relative to process.cwd(), which is typically in the user's project. Would this result in missing-plugin errors?

Based on what I understand from eslint/eslint#11388 it seems like react-scripts would tell ESLint that its lint rules should be resolved from its own cwd ("and other packages relative to where they're specified in a config"), is that correct? If so it looks like it should work fine.

[not-an-aardvark]

If the editors invoke ESLint through its CLI interface they'll just have to invoke yarn eslint instead of ./node_modules/.bin/eslint and it will just work, regardless of whether the project uses PnP or not.

Thanks, that makes sense.

When using something like create-react-app, it seems like this could be a problem because yarn eslint wouldn't find anything (since react-scripts installs eslint, but the end user does not). However, I think some editor integrations bundle their own version of ESLint to get around this issue.

Based on what I understand from eslint/eslint#11388 it seems like react-scripts would tell ESLint that its lint rules should be resolved from its own cwd ("and other packages relative to where they're specified in a config"), is that correct? If so it looks like it should work fine.

At the moment, only the user/tool that actually invokes ESLint can tell it where to find rules. (This avoids the possibility of having multiple parties give ESLint conflicting information.) Whoever invokes ESLint is required to install plugins (packages containing rules) themselves. So it seems like editor integrations would have trouble with this case, since they try to invoke ESLint but wouldn't have access to the info from react-scripts that describes how ESLint should be invoked. (This is also a problem for some other CLI flags, and isn't specific to package-loading. I suppose the general solution is to put more information in config files so that editor integrations don't need to deal with that sort of thing.)

Thanks, I think that answers my questions -- I might propose some separate changes to ESLint based on this info.

[mbeaudru]

If the editors invoke ESLint through its CLI interface they'll just have to invoke yarn eslint instead of ./node_modules/.bin/eslint and it will just work, regardless of whether the project uses PnP or not.

Following this advice, I configured vscode-eslint to use yarn as package manager to use eslint. I'm using a fresh CRA install.

./vscode/settings.json

{
  "eslint.packageManager": "yarn"
}

Unfortunately, it doesn't seems to be enough for vscode-eslint to work properly:

[Info  - 17:21:43] ESLint server stopped.
[Info  - 17:21:43] ESLint server running in node v10.2.0
[Info  - 17:21:43] ESLint server is running.
[Info  - 17:21:45] 
Failed to load the ESLint library for the document /home/mbeaudru/projects/my-awesome-project/src/App.js

To use ESLint please install eslint by running yarn add eslint in the workspace folder find-this-song
or globally using 'yarn global add eslint'. You need to reopen the workspace after installing eslint.

Alternatively you can disable ESLint for the workspace folder find-this-song by executing the 'Disable ESLint' command.

Is there something I'm missing to make it work ? I also tried to unplug eslint and added the eslint bin in NODE_PATH but it didn't worked as well.

[zgeoff]

@mbeaudru - i think if you dig into vscode-eslint plugin's code, setting the packageManager property is only useful in telling it where to find a globally installed version of ESLint.

I believe this issue to be the problem - VSCode's language server implementation isn't PnP aware, which their ESLint plugin uses. There might be a simple workaround, but I'm unsure - I don't actively use VSCode.

[zgeoff]

@not-an-aardvark - I have a scenario where I need the CLI flag --resolve-plugins-relative-to to get PnP working inside a monorepo, which is a non-issue besides that I can't pass it to various wrappers ex. prettier-eslint-cli. Is this configurable, or do I need to write my own wrapper of the ESLint Node.js API & should open an feature request/PR on who's repository?

@arcanis - for context, I've got a project that contains a workspace named @meatwallace/eslint-config, and I want to have an .eslintrc in the root of the project that extends it. My root package.json depends on "@meatwallace/eslint-config": "workspace:*" - is this the correct approach in v2?

[not-an-aardvark]

I'm not sure I understand the question, but it is configurable using the --resolve-plugins-relative-to flag or the corresponding resolvePluginsRelativeTo Node.js API option.

[kirill-konshin]

I have the issue when extending config written like this https://github.com/ringcentral/ringcentral-javascript/blob/master/eslint-config-ringcentral-typescript/src/index.js ESLint tries to resolve from CWD and fails, obviously.

@not-an-aardvark:

At the moment, only the user/tool that actually invokes ESLint can tell it where to find rules. (This avoids the possibility of having multiple parties give ESLint conflicting information.) Whoever invokes ESLint is required to install plugins (packages containing rules) themselves.

This defeats the benefits of installing third party configs and NPM basic principles. I tried to specify all dependencies of the entire eslint config/plugin tree, it's quite nasty. Moreover, if any config down the line will change I will have to update my "fix".

Without PnP everything is just lifted to node_modules so plugin/config authors were able to list whatever they wanted, everything came out flat.

I haven't found a way to rewrite something like extends: 'plugin:jsx-a11y/recommended' into require.resolve call because recommended is just a property in https://github.com/evcohen/eslint-plugin-jsx-a11y/blob/master/src/index.js#L43, and extends only accept strings.

I also tried to explicitly list plugins in eslint-config-ringcentral-typescript like so: plugins: [require.resolve('eslint-plugin-import'), ...] but no luck either. I guess that's more of a question to ESLint, how to let it know where the plugin resides.

[jacogr]

Been lurking, but thought I may as well comment as well, which is hopefully not jut a me-too...

Have also played with the yarn 2 conversion, and the last hurdle is the eslint configs. (In my specific case there is a base package with all the rules and each project in the GH org uses exactly the same config) It certainly would not make sense to have each and every repo install all the eslint deps in my case, it becomes a rabbit hole and then also needs to be maintained down the line in multiple places instead of having each and every repo standard.

There are some extends that I managed to convert via resolver, e.g. https://github.com/polkadot-js/dev/blob/83892ce57440d56fb7ac14c06549dc9399c84c2d/packages/dev/config/eslint.js#L15

(here the resolver import is just a small wrapper around require.resolve that take take string, string[] or [string, options] and just maps it - it helped in the case of babel as well keeping it kinda neat)

With some hacking and starting to extract rules form the packages extended (and including them manually, from source), can get to the point where the extends part actually more-or-less work (or at least doesn't complain anymore), although obviously this is not quite what the "extends" intended.

However, when it gets to the plugin definitions, it expects a string[] and there is no resolver that can be used there.

TL;DR For shared config, eslint indeed has "some" issues as indicated in the documentation :)