eslint-plugin-import-x icon indicating copy to clipboard operation
eslint-plugin-import-x copied to clipboard

New resolver design

Open JounQin opened this issue 1 year ago • 25 comments

I haven't remove the resolver concept in eslint-plugin-import-x<=0.2 because I found that the webpack resolver may be still useful for those webpack users, and although we can use enhanced-resolve directly, but the settings can not fit all projects, I need to figure out how to set the resolver correctly for specific projects.

For example:

  1. plain js
  2. plain typescript
  3. webpack specific syntaxes

I'm thinking the interface of import-x/resolver setting should be:

import type { ResolveOptions } from 'enhanced-resolve'

interface ResolverSettings {
  typescript?: boolean
  webpack?: boolean
  options?: ResolveOptions
}

By default, eslint-plugin-import-x should use enhanced-resolve directly to simulate native node resolve algorithm.

JounQin avatar Mar 12 '24 12:03 JounQin

Why not:

{
  resolver: "webpack",
  resolverOptions: {/* resolver-specific options */},
}

silverwind avatar Mar 21 '24 20:03 silverwind

I don't like this idea of resolver: 'webpack' | 'typescript'.

Let's do resolver: require('@eslint-import/webpack') and resolver: import('@eslint-import/webpack').

SukkaW avatar Mar 22 '24 03:03 SukkaW

Let's do resolver: require('@eslint-import/webpack') and resolver: import('@eslint-import/webpack').

.eslintrc won't work is this case? Maybe you mean resolver: '@eslint-import/webpack' which is how eslint-plugin-import(-x) work currently?

I don't quite want to support custom resolver actually, that's why I proposed webpack?: boolean option, there should only a few resolvers for usage, ideally there should be no resolver concept except TypeScript, but webpack users may want to resolve webpack config automatically (I'm thinking if there are really eslint users using it nowadays because it does not work for webpack.cofig.ts/webpack.config.mjs, etc).

image

JounQin avatar Mar 22 '24 06:03 JounQin

I don't quite want to support custom resolver actually, that's why I proposed webpack?: boolean option, there should only a few resolvers for usage, ideally there should be no resolver concept except TypeScript, but webpack users may want to resolve webpack config automatically (I'm thinking if there are really eslint users using it nowadays because it does not work for webpack.cofig.ts/webpack.config.mjs, etc).

Since we have multiple resolvers, and each one of them will have its own unique set of options. Perhaps we could approach it like this instead:

{
  webpack?: boolean | WebpackResolverOption | null
  typescript?: boolean | TypeScriptResolverOption | null
  node?: boolean | NodeResolverOption | null
}

SukkaW avatar Mar 22 '24 06:03 SukkaW

Since we have multiple resolvers, and each one of them will have its own unique set of options. Perhaps we could approach it like this instead:

That's how it's working nowadays except it requires specific npm packages installed. 🥹

But if we are going to adopt using enhanced-resolve instead, I think no node resolver should remain because it should be the default behavior. Should'd it?

For resolver options, could there be any other options except enhanced-resolve's?

I'm asking because I want to make a breaking change to drop support for previous resolvers instead of continuing.

JounQin avatar Mar 22 '24 10:03 JounQin

I don't like this idea of resolver: 'webpack' | 'typescript'.

Let's do resolver: require('@eslint-import/webpack') and resolver: import('@eslint-import/webpack').

That would work and be preferred in flat config format. Nonflat config has to support the package name as string.

silverwind avatar Mar 22 '24 10:03 silverwind

For resolver options, could there be any other options except enhanced-resolve's?

Yeah, I want to do the same. There are many resolve libraries out there trying to mimic Node.js-like resolving algorithm:

  • https://github.com/unjs/mlly
  • https://www.npmjs.com/package/import-meta-resolve

SukkaW avatar Mar 22 '24 11:03 SukkaW

I think the only modern resolution mechanisms one needs are these two (names based on tsconfig moduleResolution):

  • node-16: Node's resolution that requires file extensions. Likely should use import.meta.resolve and require.resolve, so should work without additional dependencies.
  • bundler: Similar to node, but does not require file extensions, maybe this could cover the webpack case as well.

silverwind avatar Mar 22 '24 11:03 silverwind

We will still need a library as long as we are publishing ESM/CJS dual packages and want to have a unified behavior across eslint.config.mjs and eslint.config.cjs.

SukkaW avatar Mar 22 '24 12:03 SukkaW

Is CJS support still desired? CJS packages could just remain on using eslint-plugin-import.

Non-flat configs load as CJS but if I recall correctly, import.meta.resolve is also available there.

silverwind avatar Mar 22 '24 12:03 silverwind

Non-flat configs load as CJS but if I recall correctly, import.meta.resolve is also available there.

Flat config supports both CJS and ESM, so there is no reason to make eslint-plugin-import-x a pure ESM module.

SukkaW avatar Mar 22 '24 13:03 SukkaW

Non-flat configs load as CJS but if I recall correctly, import.meta.resolve is also available there.

Flat config supports both CJS and ESM, so there is no reason to make eslint-plugin-import-x a pure ESM module.

Ah, you are right. I wasn't aware that it's possible to support CJS in flat config as I head read this, so transpilation to CJS will be required if non-flat config is to be supported.

silverwind avatar Mar 22 '24 14:03 silverwind

@JounQin Was reading this blog post: How we made Vite 4.3 faaaaster 🚀, and here I quote:

Vite 4.2 heavily depends on the resolve package to resolve the dependency's package.json, when we looked into the source code of resolve, there was much useless logic while resolving package.json. Vite 4.3 abandons resolve and follows the simpler resolve logic: directly checks whether package.json exists in the nested parents' directories.

IMHO should we build our own resolving algorithm as well?

SukkaW avatar Apr 13 '24 16:04 SukkaW

Regarding typescript, it would be great if this could somehow piggyback off of typescript's own module resolution.

https://github.com/import-js/eslint-import-resolver-typescript mostly matches typescript, but I don't think it supports the project service, so you still have to tell it about all the relevant projects manually (in dependency order).

typescript-eslint does support the project service through EXPERIMENTAL_useProjectService and they intend to stabilize it as projectService for the next major release: https://github.com/typescript-eslint/typescript-eslint/pull/9084

ehoogeveen-medweb avatar May 19 '24 18:05 ehoogeveen-medweb

eslint-import-resolver-vite is used to resolve path alias that defined in vite.config.*: https://github.com/pzmosquito/eslint-import-resolver-vite/issues/12

n0099 avatar Jun 05 '24 01:06 n0099

import-js/eslint-import-resolver-typescript mostly matches typescript, but I don't think it supports the project service, so you still have to tell it about all the relevant projects manually (in dependency order).

Here's a request for that: https://github.com/import-js/eslint-import-resolver-typescript/issues/282

SamuelT-Beslogic avatar Jun 05 '24 18:06 SamuelT-Beslogic

So is there any official way currently to support projectService? eslint-import-resolver-typescript doesn't support it at the moment and I don't think so it will be implemented in short. Any alternative way that can make it work?

xsjcTony avatar Aug 16 '24 00:08 xsjcTony

So is there any official way currently to support projectService?

eslint-import-resolver-typescript performs isolated file parsing and deliberately disables project and projectService. Unless typescript-eslint provides a document or a guide, it is impossible for a third-party library to use projectService.

SukkaW avatar Aug 16 '24 10:08 SukkaW

For anyone who wants to know, if you explicitly provide project to the resolver configuration, then everything will still work, since it's not using projectService at all. I didn't look into the code but I'm assuming it's trying to get the previous project setting in the TSESlint configuration.

xsjcTony avatar Aug 17 '24 15:08 xsjcTony

For anyone who wants to know, if you explicitly provide project to the resolver configuration, then everything will still work, since it's not using projectService at all. I didn't look into the code but I'm assuming it's trying to get the previous project setting in the TSESlint configuration.

If you do so, the typescript-eslint will become 1.5x slower, see https://github.com/typescript-eslint/typescript-eslint/issues/8424

That's why project and projectService are deliberately disabled (as I mentioned) in eslint-plugin-import and eslint-plugin-import-x, see:

  • https://github.com/import-js/eslint-plugin-import/pull/2963
  • https://github.com/un-ts/eslint-plugin-import-x/pull/93

SukkaW avatar Aug 17 '24 19:08 SukkaW

Yeah seems so, but currently I can't really find a way to make things work without specifying the project option in eslint-import-resolver-typescript, since the import/order rule cannot recognise the paths settings in tsconfig.json.

I seems have some idea how the resolver works, but I'm not too sure. Anyway as a result, previously when I'm using the project option for typescript-eslint v7, it works all good, but once I switched to projectService, it breaks.

I tried my best to read through those issues, but this is just a bit hard for someone have no insights to understand well😥.

At least for now I have a way to make it work, so I'm satisfied with it. I guess if one day typescript-eslint provides the guide to take advantage of projectService for third-party libs, things should be straight forward and easy to understand for everyone.

xsjcTony avatar Aug 18 '24 14:08 xsjcTony

eslint-import-resolver-typescript performs isolated file parsing and deliberately disables project and projectService. Unless typescript-eslint provides a document or a guide, it is impossible for a third-party library to use projectService.

Does this help at all? https://github.com/typescript-eslint/typescript-eslint/discussions/8030#discussioncomment-10675969

Has there been communications directly between eslint-plugin-import-x and typescript-eslint devs ?

SamuelT-Beslogic avatar Sep 19 '24 14:09 SamuelT-Beslogic