feat: support passing parser instance for flat configs

[scytacki]

This issue happens when the typescript recommended configuration is used in a workspace of a monorepo and the Yarn PnP install mode is used.

An minimal reproduction is here:
https://github.com/concord-consortium/test-eslint-import-x/tree/monorepo-typescript

The same eslint configuration and set of dependencies works when not in a monorepo that can be tested here:
https://github.com/concord-consortium/test-eslint-import-x/tree/typescript

The reported error is:

EslintPluginImportResolveError: typescript with invalid interface loaded as resolver
Occurred while linting /Users/scytacki/Development/test-eslint/demo/test.ts:1
Rule: "import-x/namespace"
    at requireResolver (file:///Users/scytacki/Development/test-eslint/.yarn/__virtual__/eslint-plugin-import-x-virtual-54f4e9d264/3/.yarn/berry/cache/eslint-plugin-import-x-npm-4.15.1-14cf13eac8-10c0.zip/node_modules/eslint-plugin-import-x/lib/utils/legacy-resolver-settings.js:100:21)
    at normalizeConfigResolvers (file:///Users/scytacki/Development/test-eslint/.yarn/__virtual__/eslint-plugin-import-x-virtual-54f4e9d264/3/.yarn/berry/cache/eslint-plugin-import-x-npm-4.15.1-14cf13eac8-10c0.zip/node_modules/eslint-plugin-import-x/lib/utils/legacy-resolver-settings.js:50:38)
    at fullResolve (file:///Users/scytacki/Development/test-eslint/.yarn/__virtual__/eslint-plugin-import-x-virtual-54f4e9d264/3/.yarn/berry/cache/eslint-plugin-import-x-npm-4.15.1-14cf13eac8-10c0.zip/node_modules/eslint-plugin-import-x/lib/utils/resolve.js:181:59)
    at relative (file:///Users/scytacki/Development/test-eslint/.yarn/__virtual__/eslint-plugin-import-x-virtual-54f4e9d264/3/.yarn/berry/cache/eslint-plugin-import-x-npm-4.15.1-14cf13eac8-10c0.zip/node_modules/eslint-plugin-import-x/lib/utils/resolve.js:207:12)
    at remotePath (file:///Users/scytacki/Development/test-eslint/.yarn/__virtual__/eslint-plugin-import-x-virtual-54f4e9d264/3/.yarn/berry/cache/eslint-plugin-import-x-npm-4.15.1-14cf13eac8-10c0.zip/node_modules/eslint-plugin-import-x/lib/utils/export-map.js:157:20)
    at captureDependency (file:///Users/scytacki/Development/test-eslint/.yarn/__virtual__/eslint-plugin-import-x-virtual-54f4e9d264/3/.yarn/berry/cache/eslint-plugin-import-x-npm-4.15.1-14cf13eac8-10c0.zip/node_modules/eslint-plugin-import-x/lib/utils/export-map.js:258:23)
    at captureDependencyWithSpecifiers (file:///Users/scytacki/Development/test-eslint/.yarn/__virtual__/eslint-plugin-import-x-virtual-54f4e9d264/3/.yarn/berry/cache/eslint-plugin-import-x-npm-4.15.1-14cf13eac8-10c0.zip/node_modules/eslint-plugin-import-x/lib/utils/export-map.js:252:13)
    at ExportMap.parse (file:///Users/scytacki/Development/test-eslint/.yarn/__virtual__/eslint-plugin-import-x-virtual-54f4e9d264/3/.yarn/berry/cache/eslint-plugin-import-x-npm-4.15.1-14cf13eac8-10c0.zip/node_modules/eslint-plugin-import-x/lib/utils/export-map.js:304:17)
    at ExportMap.for (file:///Users/scytacki/Development/test-eslint/.yarn/__virtual__/eslint-plugin-import-x-virtual-54f4e9d264/3/.yarn/berry/cache/eslint-plugin-import-x-npm-4.15.1-14cf13eac8-10c0.zip/node_modules/eslint-plugin-import-x/lib/utils/export-map.js:75:31)
    at ExportMap.get (file:///Users/scytacki/Development/test-eslint/.yarn/__virtual__/eslint-plugin-import-x-virtual-54f4e9d264/3/.yarn/berry/cache/eslint-plugin-import-x-npm-4.15.1-14cf13eac8-10c0.zip/node_modules/eslint-plugin-import-x/lib/utils/export-map.js:92:29)

[JounQin]

That's because you didn't install https://github.com/import-js/eslint-import-resolver-typescript at all, so typescript is loaded as resolver unexpectedly.

[scytacki]

@JounQin I believe it is installed here: https://github.com/concord-consortium/test-eslint-import-x/blob/monorepo-typescript/demo/package.json#L14

I would have expected that to be sufficient, it seems wrong if it is required to be installed at the root of the monorepo.

[scytacki]

I added some debug logging to eslint-plugin-import-x legacy-resovler-setting:requireResolver and I can see it is correctly finding the typescript resolver for the source files in the workspace. It fails when it is trying to find the typescript resolver for the nanoid file:
/Users/scytacki/.yarn/berry/cache/nanoid-npm-5.1.5-79b61a9d37-10c0.zip/node_modules/nanoid/index.browser.js

When run not in the monorepo the typescript resolver is found correctly for this nanoid file.

[JounQin]

Try https://github.com/import-js/eslint-import-resolver-typescript?tab=readme-ov-file#eslintconfigjs import-x/resolver-next instead. The default resolver is still in v2 interface, yarn PnP can't handle it quite well, and this is unfixable by us.

[scytacki]

Yes that worked, but I also had to add:

packageExtensions:
  "eslint-plugin-import-x@*":
    dependencies:
      "@typescript-eslint/parser": "*"

To my .yarnrc.yml

Otherwise I got errors like:

/Users/scytacki/Development/test-eslint/demo/eslint.config.mjs
  1:16  error    Parse errors in imported module '@eslint/js': eslint-plugin-import-x tried to access @typescript-eslint/parser, but it isn't declared in its dependencies; this makes the require call ambiguous and unsound.

Required package: @typescript-eslint/parser
Required by: eslint-plugin-import-x@virtual:cc2df7460319db9ae816182f06b63991081f8d138e41483e8bde891b0c595d3853782db1c2a5ffac08b59357133bb2a23ff1cde2f09f7a1ecc1dbf47096127c1#npm:4.15.1 (via /Users/scytacki/Development/test-eslint/.yarn/__virtual__/eslint-plugin-import-x-virtual-54f4e9d264/3/.yarn/berry/cache/eslint-plugin-import-x-npm-4.15.1-14cf13eac8-10c0.zip/node_modules/eslint-plugin-import-x/lib/)

Require stack:
- /Users/scytacki/Development/test-eslint/.yarn/__virtual__/eslint-plugin-import-x-virtual-54f4e9d264/3/.yarn/berry/cache/eslint-plugin-import-x-npm-4.15.1-14cf13eac8-10c0.zip/node_modules/eslint-plugin-import-x/lib/require.js (undefined:undefined) 

I still find it odd that requireResolver(name, sourceFile) works fine for the same arguments in a non-monorepo but fails in a monorepo. I suspect the PnP is patching node:module:createRequire in a way that doesn't work the same in a monorepo. I'm going to try different versions of node to see if that makes a difference.

[JounQin]

Install those dependencies at the root level may do the trick. I don't know, and that's how yarn PnP works. You can also unplug related packages, but we can't do quite much on our side.

Or maybe, just don't use PnP. 😅

[scytacki]

Yes, installing eslint-import-resolver-typescript and @typescript-eslint/parser at the root of the monorepo does fix the problem.

So from what I can tell in a monorepo workspace:
createRequire("/Users/scytacki/.yarn/berry/cache/nanoid-npm-5.1.5-79b61a9d37-10c0.zip/node_modules/nanoid/index.browser.js").resolve("eslint-import-resolver-typescript")
requires "eslint-import-resolver-typescript" to be a dependency at the root.

Without this dependency at the root createRequire(..).resolve(..) throws:

Error: nanoid tried to access eslint-import-resolver-typescript, but it isn't declared in its dependencies; this makes the require call ambiguous and unsound.

Required package: eslint-import-resolver-typescript
Required by: nanoid@npm:5.1.5 (via /Users/scytacki/.yarn/berry/cache/nanoid-npm-5.1.5-79b61a9d37-10c0.zip/node_modules/nanoid/)

Require stack:
- /Users/scytacki/.yarn/berry/cache/nanoid-npm-5.1.5-79b61a9d37-10c0.zip/node_modules/nanoid/index.browser.js
    at require$$0.Module._resolveFilename (/Users/scytacki/Development/test-eslint/.pnp.cjs:8815:13)
    at Function.resolve (node:internal/modules/helpers:190:19)
    at tryRequire (file:///Users/scytacki/Development/test-eslint/.yarn/__virtual__/eslint-plugin-import-x-virtual-6b2952cb29/3/.yarn/berry/cache/eslint-plugin-import-x-patch-6a8c531a2b-10c0.zip/node_modules/eslint-plugin-import-x/lib/utils/legacy-resolver-settings.js:130:68)

I do think there is something eslint-plugin-import-x can do here. Why is it using the source file that it is parsing as the context/filename of createRequire? It makes sense that PnP would fail to load the typescript resolver from a dependency. It seems just a lucky coincidence that it has been working in its current form.

I'd think it'd be better if the local eslint config file was used as the context/filename of createRequire. This way it is similar to what would happen if the user explicitly added an import to the eslint config file. But perhaps you've been down that road already and the way it is currently implemented is the best you can do?

[JounQin]

If you have a good way to support yarn PnP, feel free to PR!

[arcanis]

I believe the right fix is to load the plugins relative to the configuration file path rather than the source files.

Eslint used to have the same issues which led to community-maintained workarounds (the most famous being Microsoft's: https://www.npmjs.com/package/@rushstack/eslint-patch, see the "modern-module-resolution" section for details).

[JounQin]

The parsers are loaded in ESLint rules, I'm not sure how to load the parsers relative to the configuration file path rather than the source files.

Ideally we should support passing parser instance for flat configs, but for eslintrc configs, there seems to be no workaround.

[scytacki]

I did find a work around which is to use the eslint context.filename as the filename argument to createRequire(...). Since context.filename is the file being linted, not the file being parsed (which might be a module in a different package loaded by the linted file). This approach isn't as good as passing the config file path, but since most users are only going to be linting files in their own project it should usually result in the correct behavior. I'll try to put a PR with this change soon.

As far as I can tell there isn't a way get the path of the eslint config file being used for the current file being linted. Note that this config file might change depending on the location of the file being linted due to this feature: https://eslint.org/docs/latest/use/configure/configuration-files#experimental-configuration-file-resolution
So I think to support the string based dynamic imports best, eslint would need to start passing the current config file path in the context.

However as @JounQin wrote the best approach would be stop using string based dynamic imports and instead pass the parser instance directly, similar to what is done in the 'import-x/resolver-next': [ createTypeScriptImportResolver(...) ] case.