Resolution relative to cwd/__placeholder__.js is flaky, breaks when deps nested below top level

[isaacs]

Tell us about your environment

• ESLint Version: Both 6.x and 7.x (that is the part of the problem!)
• Node Version: 15.0.0 (not relevant)
• npm Version: 7.0.2 (extremely relevant)

What parser (default, @babel/eslint-parser, @typescript-eslint/parser, etc.) are you using?

Not sure. This is in the yargs project, which uses standard, via standardx.

Please show your full configuration:

Configuration

The config used directly in yargs with eslint 7.x is this:

{
  "extends": "./node_modules/gts/",
  "overrides": [
    {
      "files": ["**/*.ts", "**/*.tsx"],
      "rules": {
        "@typescript-eslint/no-var-requires": "off",
        "@typescript-eslint/no-unused-vars": "off",
        "@typescript-eslint/no-explicit-any": "off"
      }
    }
  ]
}

The one used by standard, at node_modules/standard/eslint.json is

{
  "extends": ["standard", "standard-jsx"]
}

What did you do? Please include the actual source code causing the issue, as well as the command that you used to run ESLint.

As of npm v7, peerDependencies are installed by default, and npm makes every effort (short of venturing into NP-hard territory) to ensure that every dependency's peerDependencies will be met according to their stated constraints. This can involve nesting dependencies in a peerDependency set deeper in the tree.

The --legacy-peer-deps flag allows npm 7 to ignore all peerDependencies, as npm versions 4-6 did.

When installing the yargs project using npm v7.0.2, the install works properly, and npm ls --all does not report any conflicting peer dependencies. Hooray!

npm ls output when installed with default settings in npm v7.0.2

yargs@16.1.0 /Users/isaacs/dev/js/yargs
├─┬ gts@3.0.1
│ ├─┬ @typescript-eslint/eslint-plugin@4.4.1
│ │ ├─┬ @typescript-eslint/experimental-utils@4.4.1
│ │ │ └── eslint@7.11.0 deduped
│ │ └── eslint@7.11.0 deduped
│ ├─┬ @typescript-eslint/parser@4.4.1
│ │ └── eslint@7.11.0 deduped
│ ├─┬ eslint-config-prettier@6.13.0
│ │ └── eslint@7.11.0 deduped
│ ├─┬ eslint-plugin-node@11.1.0
│ │ ├─┬ eslint-plugin-es@3.0.1
│ │ │ └── eslint@7.11.0 deduped
│ │ └── eslint@7.11.0 deduped
│ ├─┬ eslint-plugin-prettier@3.1.4
│ │ └── eslint@7.11.0 deduped
│ └── eslint@7.11.0
└─┬ standardx@5.0.0
  └─┬ standard@14.3.4
    ├─┬ eslint-config-standard-jsx@8.1.0
    │ └── eslint@6.8.0 deduped
    ├─┬ eslint-config-standard@14.1.1
    │ └── eslint@6.8.0 deduped
    ├─┬ eslint-plugin-import@2.18.2
    │ └── eslint@6.8.0 deduped
    ├─┬ eslint-plugin-node@10.0.0
    │ ├─┬ eslint-plugin-es@2.0.0
    │ │ └── eslint@6.8.0 deduped
    │ └── eslint@6.8.0 deduped
    ├─┬ eslint-plugin-react@7.14.3
    │ └── eslint@6.8.0 deduped
    ├─┬ eslint-plugin-standard@4.0.1
    │ └── eslint@7.11.0 deduped
    └── eslint@6.8.0

Hooray! no problems found! Every package has its peerDependencies constraints honored.

By comparison, when installed with the --legacy-peer-deps flag, we get this resulting npm ls output:

npm ls output when installed with legacy peer deps mode

yargs@16.1.0 /Users/isaacs/dev/js/yargs
├─┬ gts@3.0.1
│ ├─┬ @typescript-eslint/eslint-plugin@4.4.1
│ │ ├─┬ @typescript-eslint/experimental-utils@4.4.1
│ │ │ └── eslint@7.11.0 deduped
│ │ └── eslint@7.11.0 deduped
│ ├─┬ @typescript-eslint/parser@4.4.1
│ │ └── eslint@7.11.0 deduped
│ ├─┬ eslint-config-prettier@6.13.0
│ │ └── eslint@7.11.0 deduped
│ ├─┬ eslint-plugin-node@11.1.0
│ │ ├─┬ eslint-plugin-es@3.0.1
│ │ │ └── eslint@7.11.0 deduped
│ │ └── eslint@7.11.0 deduped
│ ├─┬ eslint-plugin-prettier@3.1.4
│ │ └── eslint@7.11.0 deduped
│ └── eslint@7.11.0
└─┬ standardx@5.0.0
  └─┬ standard@14.3.4
    ├─┬ eslint-config-standard-jsx@8.1.0
    │ └── eslint@7.11.0 deduped
    ├─┬ eslint-config-standard@14.1.1
    │ └── eslint@7.11.0 deduped
    ├─┬ eslint-plugin-import@2.18.2
    │ └── eslint@7.11.0 deduped invalid
    ├─┬ eslint-plugin-node@10.0.0
    │ ├─┬ eslint-plugin-es@2.0.0
    │ │ └── eslint@6.8.0 deduped
    │ └── eslint@6.8.0 deduped
    ├─┬ eslint-plugin-react@7.14.3
    │ └── eslint@7.11.0 deduped invalid
    ├─┬ eslint-plugin-standard@4.0.1
    │ └── eslint@7.11.0 deduped
    └── eslint@6.8.0

npm ERR! code ELSPROBLEMS
npm ERR! invalid: eslint@7.11.0 /Users/isaacs/dev/js/yargs/node_modules/eslint

Note that eslint@7.11.0 is being provided as a resolution for eslint-plugin-react and eslint-plugin-import, despite those packages having a peerDependencies constraint requiring eslint 6.x.

where this gets interesting...

Yargs has dependencies on both gts (which uses eslint 7) and standardx (which uses eslint 6). In order to satisfy the dependency constraints, standard needs to have a nested copy of eslint in node_modules/standard/node_modules/eslint. This is not new. What is new is that all the dependencies of standard which have peerDependencies on eslint are now also nested under node_modules/standard/node_modules/..., so that they will get the version of eslint that they claim to work with.

Standard then loads eslint with a config (referenced above) that includes "extends": ["standard", "standard-jsx"]. Eslint looks for a package named eslint-config-standard, but does so by attempting to resolve it relative to cwd + '/__placeholder__.js'.

Since eslint-config-standard has been nested under node_modules/standard/node_modules/..., however, it cannot be found relative to the root project directory.

The interesting thing is that it actually does work (albeit, likely somewhat by accident) when the peerDependencies on eslint 6 are not honored, likely because the config shape just hasn't changed too much between 6 and 7. Of course, in general, for a package manager, this is not even slightly safe to assume, so we don't assume it any more in npm 😅.

A better approach, when loading from an implicit package name without any explicit file path, might be for eslint to attempt to load the module by doing require.resolve() (to get it relative to the eslint package itself), or relative to the module that first require()'d eslint (if require.main is not eslint), and only then from process.cwd().

This does all work fine, of course, if eslint is loaded as a bin script rather than a library used by some other bin script, as gts, standard, standardx, and many others do.

What did you expect to happen?

Expect eslint to find the eslint-config-standard module in the package tree, as it is visible to eslint and the code calling eslint, even though it is not visible to a require() from the cwd.

What actually happened? Please include the actual, raw output from ESLint.

$ npm run check:js

> yargs@16.1.0 check:js
> standardx '**/*.mjs' && standardx '**/*.cjs' && standardx './*.mjs' && standardx './*.cjs'

standardx: Unexpected linter output:

Error: Failed to load config "standard" to extend from.
Referenced from: BaseConfig
    at configMissingError (/Users/isaacs/dev/js/yargs/node_modules/standard/node_modules/eslint/lib/cli-engine/config-array-factory.js:265:9)
    at ConfigArrayFactory._loadExtendedShareableConfig (/Users/isaacs/dev/js/yargs/node_modules/standard/node_modules/eslint/lib/cli-engine/config-array-factory.js:826:23)
    at ConfigArrayFactory._loadExtends (/Users/isaacs/dev/js/yargs/node_modules/standard/node_modules/eslint/lib/cli-engine/config-array-factory.js:731:25)
    at ConfigArrayFactory._normalizeObjectConfigDataBody (/Users/isaacs/dev/js/yargs/node_modules/standard/node_modules/eslint/lib/cli-engine/config-array-factory.js:660:25)
    at _normalizeObjectConfigDataBody.next (<anonymous>)
    at ConfigArrayFactory._normalizeObjectConfigData (/Users/isaacs/dev/js/yargs/node_modules/standard/node_modules/eslint/lib/cli-engine/config-array-factory.js:596:20)
    at _normalizeObjectConfigData.next (<anonymous>)
    at createConfigArray (/Users/isaacs/dev/js/yargs/node_modules/standard/node_modules/eslint/lib/cli-engine/config-array-factory.js:340:25)
    at ConfigArrayFactory.create (/Users/isaacs/dev/js/yargs/node_modules/standard/node_modules/eslint/lib/cli-engine/config-array-factory.js:395:16)
    at createBaseConfigArray (/Users/isaacs/dev/js/yargs/node_modules/standard/node_modules/eslint/lib/cli-engine/cascading-config-array-factory.js:86:48)

If you think this is a bug in `standardx`, open an issue: https://github.com/standard/standardx/issues

Are you willing to submit a pull request to fix this bug?

Sure, but I probably won't get to it as quickly as someone more familiar with the project.

[isaacs]

Note: another (imo, perfectly reasonable) way to approach this would be to say that eslint is really not intended to be bundled as a library, or require that standard and friends find some other way to define the configs that they extend. (For example, by using require.resolve('eslint-config-standard') themselves to get the full actual path to the config definitions.)

Then, if the eslint bin is not the main module in node, then stubbornly refuse to resolve extends values by tacking on 'eslint-config-' and looking for a package by that name in the tree.

[isaacs]

Oh! Or, maybe, the resolution should be relative to the eslint.json file that's doing the extension? That would seem to make the most sense, since these things will most likely have dependencies or peerDependencies on the eslint-config-* packages that they're extending.

When I hard-code the full file paths to the extended config files, it runs into the same issue with the react plugin, failing to find the (nested) eslint-plugin-react module.

Attempting to shove in an absolute file path there results in:

Error: Plugins array cannot includes file paths.
    at /Users/isaacs/dev/js/yargs/node_modules/standard/node_modules/eslint/lib/cli-engine/config-array-factory.js:847:23
    at Array.reduce (<anonymous>)
    at ConfigArrayFactory._loadPlugins (/Users/isaacs/dev/js/yargs/node_modules/standard/node_modules/eslint/lib/cli-engine/config-array-factory.js:845:22)
    at ConfigArrayFactory._normalizeObjectConfigDataBody (/Users/isaacs/dev/js/yargs/node_modules/standard/node_modules/eslint/lib/cli-engine/config-array-factory.js:667:32)
    at _normalizeObjectConfigDataBody.next (<anonymous>)
    at ConfigArrayFactory._normalizeObjectConfigData (/Users/isaacs/dev/js/yargs/node_modules/standard/node_modules/eslint/lib/cli-engine/config-array-factory.js:596:20)
    at _normalizeObjectConfigData.next (<anonymous>)
    at ConfigArrayFactory._normalizeObjectConfigDataBody (/Users/isaacs/dev/js/yargs/node_modules/standard/node_modules/eslint/lib/cli-engine/config-array-factory.js:660:25)
    at _normalizeObjectConfigDataBody.next (<anonymous>)
    at ConfigArrayFactory._normalizeObjectConfigData (/Users/isaacs/dev/js/yargs/node_modules/standard/node_modules/eslint/lib/cli-engine/config-array-factory.js:596:20)

[btmills]

Thanks @isaacs for the incredibly thorough deep dive! @nzakas is working on a simplification of our config system that will solve this, assuming I understand the issue correctly. In summary, configs and plugins will require()/import their dependencies themselves. If you're curious, check out the RFC and implementation progress.

Originally, all ESLint configs were JSON-serializable and we didn't have JS configs, so as we added support for plugins and shareable configs, we ended up with complex loading and merging strategies inside of ESLint rather than relying on require.resolve() in the extended configs. Check out the bottom of the RFC for links to other module resolution issues like this one and feature requests we can't support while ESLint is responsible for loading. The new system removes the JSON-serializability constraint, so plugins and configs will be regular modules. By relying on Node's built-in module resolution, we should get all those bug fixes and features for "free". eslint/rfcs#5 seems to be most similar to your last suggestion, though we're focusing on the larger config simplification RFC instead.

[nzakas]

Thanks @isaacs and @btmills for all of the details. Indeed, we are in a period where the current config system is frozen and won't be making any more changes to it to ensure we'll have some level of backwards compatibility when the new config system is ready. This is exactly the type of problem we are looking to avoid with the new config system, so the good news is that a solution is coming and it won't be nearly as ugly as if we had to fix the original config system.

[isaacs]

Oh, Hey, @nzakas, there's a name I haven't seen in a while, hope you're doing well :)

Sounds good. In the meantime, I've just suggested yargs use --legacy-peer-deps and (continue to) ignore the warnings from npm ls.

[nzakas]

@isaacs indeed, it has been a minute! I’m hanging in, I hope you’re doing well, too.

[nzakas]

Closing as we are working on the new config system.