-
-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New resolver design #40
Comments
Why not: {
resolver: "webpack",
resolverOptions: {/* resolver-specific options */},
} |
I don't like this idea of Let's do |
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
} |
That's how it's working nowadays except it requires specific npm packages installed. 🥹 But if we are going to adopt using For resolver options, could there be any other options except I'm asking because I want to make a breaking change to drop support for previous resolvers instead of continuing. |
That would work and be preferred in flat config format. Nonflat config has to support the package name as string. |
Yeah, I want to do the same. There are many resolve libraries out there trying to mimic Node.js-like resolving algorithm: |
I think the only modern resolution mechanisms one needs are these two (names based on tsconfig
|
We will still need a library as long as we are publishing ESM/CJS dual packages and want to have a unified behavior across |
Is CJS support still desired? CJS packages could just remain on using Non-flat configs load as CJS but if I recall correctly, |
Flat config supports both CJS and ESM, so there is no reason to make |
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. |
@JounQin Was reading this blog post: How we made Vite 4.3 faaaaster 🚀, and here I quote:
IMHO should we build our own resolving algorithm as well? |
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 |
|
Here's a request for that: import-js/eslint-import-resolver-typescript#282 |
So is there any official way currently to support |
|
For anyone who wants to know, if you explicitly provide |
If you do so, the typescript-eslint will become 1.5x slower, see typescript-eslint/typescript-eslint#8424 That's why |
Yeah seems so, but currently I can't really find a way to make things work without specifying the 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 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 |
Does this help at all? typescript-eslint/typescript-eslint#8030 (reply in thread) Has there been communications directly between |
This comment was marked as outdated.
This comment was marked as outdated.
I agree with your design but have a few questions. |
But of course, I'd recommend all new resolvers implement The original proposal updated. |
Hey there, |
All resolves are happening inside the rules. So, unless ESLint supports async rules, we can only use a sync resolver for now. |
##### [v4.5.0](https://github.com/un-ts/eslint-plugin-import-x/blob/HEAD/CHANGELOG.md#450) ##### Minor Changes - [#192](un-ts/eslint-plugin-import-x#192) [`fbf639b`](un-ts/eslint-plugin-import-x@fbf639b) Thanks [@SukkaW](https://github.com/SukkaW)! - The PR implements the new resolver design proposed in un-ts/eslint-plugin-import-x#40 (comment) ##### For `eslint-plugin-import-x` users Like the ESLint flat config allows you to use js objects (e.g. import and require) as ESLint plugins, the new `eslint-plugin-import-x` resolver settings allow you to use js objects as custom resolvers through the new setting `import-x/resolver-next`: ```js // eslint.config.js import { createTsResolver } from '#custom-resolver'; const { createOxcResolver } = require('path/to/a/custom/resolver'); const resolverInstance = new ResolverFactory({}); const customResolverObject = { interfaceVersion: 3, name: 'my-custom-eslint-import-resolver', resolve(modPath, sourcePath) { const path = resolverInstance.resolve(modPath, sourcePath); if (path) { return { found: true, path }; } return { found: false, path: null } }; }; module.exports = { settings: { // multiple resolvers 'import-x/resolver-next': [ customResolverObject, createTsResolver(enhancedResolverOptions), createOxcResolver(oxcOptions), ], // single resolver: 'import-x/resolver-next': [createOxcResolver(oxcOptions)] } } ``` The new `import-x/resolver-next` no longer accepts strings as the resolver, thus will not be compatible with the ESLint legacy config (a.k.a. `.eslintrc`). Those who are still using the ESLint legacy config should stick with `import-x/resolver`. In the next major version of `eslint-plugin-import-x` (v5), we will rename the currently existing `import-x/resolver` to `import-x/resolver-legacy` (which allows the existing ESLint legacy config users to use their existing resolver settings), and `import-x/resolver-next` will become the new `import-x/resolver`. When ESLint v9 (the last ESLint version with ESLint legacy config support) reaches EOL in the future, we will remove `import-x/resolver-legacy`. We have also made a few breaking changes to the new resolver API design, so you can't use existing custom resolvers directly with `import-x/resolver-next`: ```js // When migrating to `import-x/resolver-next`, you CAN'T use legacy versions of resolvers directly: module.exports = { settings: { // THIS WON'T WORK, the resolver interface required for `import-x/resolver-next` is different. 'import-x/resolver-next': [ require('eslint-import-resolver-node'), require('eslint-import-resolver-webpack'), require('some-custom-resolver') ]; } } ``` For easier migration, the PR also introduces a compat utility `importXResolverCompat` that you can use in your `eslint.config.js`: ```js // eslint.config.js import eslintPluginImportX, { importXResolverCompat } from 'eslint-plugin-import-x'; // or const eslintPluginImportX = require('eslint-plugin-import-x'); const { importXResolverCompat } = eslintPluginImportX; module.exports = { settings: { // THIS WILL WORK as you have wrapped the previous version of resolvers with the `importXResolverCompat` 'import-x/resolver-next': [ importXResolverCompat(require('eslint-import-resolver-node'), nodeResolveOptions), importXResolverCompat(require('eslint-import-resolver-webpack'), webpackResolveOptions), importXResolverCompat(require('some-custom-resolver'), { option1: true, option2: '' }) ]; } } ``` ##### For custom import resolver developers This is the new API design of the resolver interface: ```ts export interface NewResolver { interfaceVersion: 3; name?: string; // This will be included in the debug log resolve: (modulePath: string, sourceFile: string) => ResolvedResult; } // The `ResultNotFound` (returned when not resolved) is the same, no changes export interface ResultNotFound { found: false; path?: undefined; } // The `ResultFound` (returned resolve result) is also the same, no changes export interface ResultFound { found: true; path: string | null; } export type ResolvedResult = ResultNotFound | ResultFound; ``` You will be able to import `NewResolver` from `eslint-plugin-import-x/types`. The most notable change is that `eslint-plugin-import-x` no longer passes the third argument (`options`) to the `resolve` function. We encourage custom resolvers' authors to consume the options outside the actual `resolve` function implementation. You can export a factory function to accept the options, this factory function will then be called inside the `eslint.config.js` to get the actual resolver: ```js // custom-resolver.js exports.createCustomResolver = (options) => { // The options are consumed outside the `resolve` function. const resolverInstance = new ResolverFactory(options); return { name: 'custom-resolver', interfaceVersion: 3, resolve(mod, source) { const found = resolverInstance.resolve(mod, {}); // Of course, you still have access to the `options` variable here inside // the `resolve` function. That's the power of JavaScript Closures~ } } }; // eslint.config.js const { createCustomResolver } = require('custom-resolver') module.exports = { settings: { 'import-x/resolver-next': [ createCustomResolver(options) ]; } } ``` This allows you to create a reusable resolver instance to improve the performance. With the existing version of the resolver interface, because the options are passed to the `resolver` function, you will have to create a resolver instance every time the `resolve` function is called: ```js module.exports = { interfaceVersion: 2, resolve(mod, source) { // every time the `resolve` function is called, a new instance is created // This is very slow const resolverInstance = ResolverFactory.createResolver({}); const found = resolverInstance.resolve(mod, {}); }, }; ``` With the factory function pattern, you can create a resolver instance beforehand: ```js exports.createCustomResolver = (options) => { // `enhance-resolve` allows you to create a reusable instance: const resolverInstance = ResolverFactory.createResolver({}); const resolverInstance = enhanceResolve.create({}); // `oxc-resolver` also allows you to create a reusable instance: const resolverInstance = new ResolverFactory({}); return { name: "custom-resolver", interfaceVersion: 3, resolve(mod, source) { // the same re-usable instance is shared across `resolve` invocations. // more performant const found = resolverInstance.resolve(mod, {}); }, }; }; ``` ##### Patch Changes - [#184](un-ts/eslint-plugin-import-x#184) [`bc4de89`](un-ts/eslint-plugin-import-x@bc4de89) Thanks [@marcalexiei](https://github.com/marcalexiei)! - fix(no-cycle): improves the type declaration of the rule `no-cycle`’s `maxDepth` option - [#184](un-ts/eslint-plugin-import-x#184) [`bc4de89`](un-ts/eslint-plugin-import-x@bc4de89) Thanks [@marcalexiei](https://github.com/marcalexiei)! - fix(first): improves the type declaration of the rule `first`'s option - [#184](un-ts/eslint-plugin-import-x#184) [`bc4de89`](un-ts/eslint-plugin-import-x@bc4de89) Thanks [@marcalexiei](https://github.com/marcalexiei)! - fix(no-unused-modules): improves the type declaration of the rule `no-unused-modules`’s `missingExports` option - [#184](un-ts/eslint-plugin-import-x#184) [`bc4de89`](un-ts/eslint-plugin-import-x@bc4de89) Thanks [@marcalexiei](https://github.com/marcalexiei)! - fix(no-deprecated): improve error message when no description is available
I am closing the issue now. The popular eslint import resolvers adoption process is tracked here: #192 (comment). |
I haven't remove the
resolver
concept ineslint-plugin-import-x<=0.2
because I found that thewebpack
resolver may be still useful for those webpack users, and although we can useenhanced-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:
I'm thinking the interface of
import-x/resolver
setting should be:By default,
eslint-plugin-import-x
should useenhanced-resolve
directly to simulate nativenode
resolve algorithm.The text was updated successfully, but these errors were encountered: