[Deps] let user know eslint-import-resolver-node needs to be in root of node_modules (important when using install-strategy=linked)

[nikolawork]

Using install-strategy=linked only puts the packages from dependencies and devDependencies in the root of node_modules. This means that eslint-import-resolver-node is not in the root of node_modules which leads to the following error:

Resolve error: unable to load resolver "node"

This is fixed by adding eslint-import-resolver-node to your package.json. We signal this to the user by adding eslint-import-resolver-node to peerDependencies.

install-strategy=linked is stable since npm@11.18.0: https://docs.npmjs.com/cli/v11/using-npm/config#install-strategy

npm even recommends using it when developing. From the above link:

We recommend that package authors use --install-strategy=linked during development to catch undeclared ("phantom") dependencies before publishing: the isolated layout only exposes a package's declared dependencies, so an import of a package that was never added to package.json can fail instead of resolving by accident and shipping broken. See Catching undeclared ("phantom") dependencies.

There is a closed (but unresolved) issue for this: #828

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.58%. Comparing base (bf2d2aa) to head (eb02bd8).
⚠️ Report is 13 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3266   +/-   ##
=======================================
  Coverage   79.58%   79.58%           
=======================================
  Files          98       98           
  Lines        4527     4527           
  Branches     1560     1560           
=======================================
  Hits         3603     3603           
  Misses        924      924           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[nikolawork]

This fix can also be added to the fix in #2283 and then we can close this PR.

[manzoorwanijk]

I see the dependency is already being declared correctly.

eslint-plugin-import/package.json

Line 125 in bf2d2aa

"eslint-import-resolver-node": "^0.4.0",

[nikolawork]

I see the dependency is already being declared correctly.

The problem is the way the dependency is being resolved here:

eslint-plugin-import/utils/resolve.js

Lines 101 to 103 in bf2d2aa

	const resolver = tryRequire(`eslint-import-resolver-${name}`, sourceFile)
	|| tryRequire(name, sourceFile)
	|| tryRequire(path.resolve(getBaseDir(sourceFile), name));

The only way this works is if eslint-import-resolver-node is in the root of node_modules. Which it isn't when using install-strategy=linked if the dependency is not declared in the end user's package.json (which you should definitely know haha).

[manzoorwanijk]

The problem is the way the dependency is being resolved here:

Yes, that is what I was going to write. That require logic may need to be updated to ensure it works in isolated dependencies layout.

[ljharb]

This is intentional, and is why the "linked" strategy (and yarn's PNP, and similar approaches) is fundamentally flawed.

Adding it to peer deps explicitly increases maintenance burden and could be a breaking change.

[manzoorwanijk]

I have created a dedicated bug report for it - #3268

Also created a PR to fix it - #3269

[manzoorwanijk]

But the CI is broken because of npm v12.

[ljharb]

the npm 12 thing is fixed, if you rebase.

[ljharb]

note that this is a breaking change, because it will fail if someone has a newer version installed at a higher level.

[manzoorwanijk]

the npm 12 thing is fixed, if you rebase.

Your commit seems to have used another failure - https://github.com/import-js/eslint-plugin-import/actions/runs/29535690274/job/87764990067

[nikolawork]

I have created a dedicated bug report for it - #3268

Also created a PR to fix it - #3269

Thanks for the more thorough fix, @manzoorwanijk. If that PR is merged, we can close my PR.