-
-
Notifications
You must be signed in to change notification settings - Fork 68
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
feat: rewrite, speed up by using rspack-resolver
under the hood
#368
Conversation
🦋 Changeset detectedLatest commit: 2c797a9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎ This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
size-limit report 📦
|
eslint-import-resolver-oxc under
the hood
f64377f
to
aec170e
Compare
I'll investigate soon oxc-project/oxc-resolver#416 |
IMHO the default resolver should have behaviors aligned with un-ts/eslint-plugin-import-x#208 (i.e. no Also, we can investigate to only implement the v3 interface (un-ts/eslint-plugin-import-x#192) |
Considering typescript adoption by the community, I think the default resolver should count I was thinking dropping un-ts/eslint-plugin-import-x#24 (comment) un-ts/eslint-plugin-import-x#24 (comment)
And considering the fast speed of |
For the default resolver? opt-in, not opt-out. Let's do things one step at a time. At least the behavior should be aligned with
It is impossible IMHO. Many import resolver like the Vite resolver still has a very wide range of adoption.
That's a way huge step forward. Let's do things one at a time, otherwise we will lose adoptors since everyone will be scared of the breaking changes and will simply not upgrade or even go back to |
After thinking, maybe depending on So may plan would be:
cc @SukkaW |
eslint-import-resolver-oxc under
the hoodoxc-resolver
under the hood
Works for me, let's do this! |
@Boshen Hi, after adding /Users/runner/work/eslint-import-resolver-typescript/eslint-import-resolver-typescript/tests/withPathsAndNestedBaseUrl/other/bar.ts
Error: 2:8 error Unable to resolve path to module 'foo' import/no-unresolved
✖ 1 problem (1 error, 0 warnings)
/Users/runner/work/eslint-import-resolver-typescript/eslint-import-resolver-typescript/tests/multipleTsconfigs/packages/module-b/index.ts
Error: 8:8 error Unable to resolve path to module 'folder/tsImportee' import/no-unresolved
Error: 9:8 error Unable to resolve path to module 'folder/tsxImportee' import/no-unresolved
Error: 10:8 error Unable to resolve path to module 'folder/subfolder/tsImportee' import/no-unresolved
Error: 11:8 error Unable to resolve path to module 'folder/subfolder/tsxImportee' import/no-unresolved
✖ 4 problems (4 errors, 0 warnings)
/Users/runner/work/eslint-import-resolver-typescript/eslint-import-resolver-typescript/tests/dotProject/packages/module-b/index.ts
Error: 8:8 error Unable to resolve path to module 'folder/tsImportee' import/no-unresolved
Error: 9:8 error Unable to resolve path to module 'folder/tsxImportee' import/no-unresolved
Error: 10:8 error Unable to resolve path to module 'folder/subfolder/tsImportee' import/no-unresolved
Error: 11:8 error Unable to resolve path to module 'folder/subfolder/tsxImportee' import/no-unresolved
✖ 4 problems (4 errors, 0 warnings) Do I need to create a separate issue for you? |
Updated and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/@nolyfill/[email protected] |
4f54f95
to
c9f4e30
Compare
no s390x support :/ |
Oh, unfortunately, we may need to provide a fallback mode. |
It should be very easy to provide s390x support through NAPI-RS IIUC? |
I'm working merging back upstream, let's check later. See also unrs/rspack-resolver#15 and unrs/rspack-resolver#18 ![]() Performance is even better! |
Also, we could provide a WASM build for any uncommon target. |
There is indeed @eaibmz Can you provide more error details? |
is this enough:
|
@eaibmz Would you like to help trying out: # yarn 1
yarn add https://pkg.csb.dev/import-js/eslint-import-resolver-typescript/commit/6a368d69/eslint-import-resolver-typescript
# yarn 2, 3
yarn add eslint-import-resolver-typescript@https://pkg.csb.dev/import-js/eslint-import-resolver-typescript/commit/6a368d69/eslint-import-resolver-typescript/_pkg.tgz
# npm
npm i https://pkg.csb.dev/import-js/eslint-import-resolver-typescript/commit/6a368d69/eslint-import-resolver-typescript |
hmm, didn't help, same error
|
| datasource | package | from | to | | ---------- | --------------------------------- | ----- | ----- | | npm | eslint-import-resolver-typescript | 3.9.1 | 4.2.1 | ## [v4.2.1](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#421) ##### Patch Changes - [#394](import-js/eslint-import-resolver-typescript#394) [`9f11f6b`](import-js/eslint-import-resolver-typescript@9f11f6b) Thanks [@JounQin](https://github.com/JounQin)! - fix: don't set empty `configFile` when no `tsconfig` found - [#394](import-js/eslint-import-resolver-typescript#394) [`9f11f6b`](import-js/eslint-import-resolver-typescript@9f11f6b) Thanks [@JounQin](https://github.com/JounQin)! - chore: bump `rspack-resolver` to v1.2.0 ## [v4.2.0](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#420) ##### Minor Changes - [#391](import-js/eslint-import-resolver-typescript#391) [`c8121e5`](import-js/eslint-import-resolver-typescript@c8121e5) Thanks [@JounQin](https://github.com/JounQin)! - feat: make `is-bun-module` as optional peer dependency Technically this is a BREAKING CHANGE, but considering we just raise out v4 recently and this only affects `bun` users, `bun --bun eslint` even works without this dependency, so I'd consider this as a minor change. So for `bun` users, there are three options: 1. install `is-bun-module` dependency manually and use `bun: true` option 2. run `eslint` with `bun --bun eslint` w/o `bun: true` option 3. enable `run#bun` in [`bunfig.toml`](https://bun.sh/docs/runtime/bunfig#run-bun-auto-alias-node-to-bun) w/o `bun: true` option ## [v4.1.1](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#411) ##### Patch Changes - [#389](import-js/eslint-import-resolver-typescript#389) [`1b97d8a`](import-js/eslint-import-resolver-typescript@1b97d8a) Thanks [@JounQin](https://github.com/JounQin)! - fix: should prefer `module.isBuiltin` when `process.versions.bun` available ## [v4.1.0](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#410) ##### Minor Changes - [#387](import-js/eslint-import-resolver-typescript#387) [`ef5cd10`](import-js/eslint-import-resolver-typescript@ef5cd10) Thanks [@JounQin](https://github.com/JounQin)! - feat: add a new `bun?: boolean` option for `bun` users - close [#386](import-js/eslint-import-resolver-typescript#386) `process.versions.bun` is unavailable even with `bun eslint` due to its own design, but checking `bun` modules for non-bun users is incorrect behavior and just wasting time, so a new option is added for such case, you can still run with `bun --bun eslint` without this option enabled ## [v4.0.0](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#400) ##### Major Changes - [#368](import-js/eslint-import-resolver-typescript#368) [`2fd7c2e`](import-js/eslint-import-resolver-typescript@2fd7c2e) Thanks [@JounQin](https://github.com/JounQin)! - feat!: rewrite, speed up by using [`rspack-resolver`](https://github.com/unrs/rspack-resolver) which supports `references` natively under the hood BREAKING CHANGES: - drop Node 14 support, Node `^16.17.0 || >=18.6` is now required - `alwaysTryTypes` is enabled by default, you can set it as `false` to opt-out - array type of `project` is discouraged but still supported, single `project` with `references` are encouraged for better performance, you can enable `noWarnOnMultipleProjects` option to supress the warning message - root `tsconfig.json` or `jsconfig.json` will be used automatically if no `project` provided
@eaibmz Can you open a new issue for this? And help us to debug what error exactly thrown with. https://app.unpkg.com/[email protected]/files/index.js For example: |
| datasource | package | from | to | | ---------- | --------------------------------- | ----- | ----- | | npm | eslint-import-resolver-typescript | 3.9.1 | 4.2.2 | ## [v4.2.2](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#422) ##### Patch Changes - [#397](import-js/eslint-import-resolver-typescript#397) [`14a7688`](import-js/eslint-import-resolver-typescript@14a7688) Thanks [@JounQin](https://github.com/JounQin)! - chore: bump `rspack-resolver` for better P'n'P support Now `rspack-resolver` resolves `pnpapi` natively. ## [v4.2.1](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#421) ##### Patch Changes - [#394](import-js/eslint-import-resolver-typescript#394) [`9f11f6b`](import-js/eslint-import-resolver-typescript@9f11f6b) Thanks [@JounQin](https://github.com/JounQin)! - fix: don't set empty `configFile` when no `tsconfig` found - [#394](import-js/eslint-import-resolver-typescript#394) [`9f11f6b`](import-js/eslint-import-resolver-typescript@9f11f6b) Thanks [@JounQin](https://github.com/JounQin)! - chore: bump `rspack-resolver` to v1.2.0 ## [v4.2.0](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#420) ##### Minor Changes - [#391](import-js/eslint-import-resolver-typescript#391) [`c8121e5`](import-js/eslint-import-resolver-typescript@c8121e5) Thanks [@JounQin](https://github.com/JounQin)! - feat: make `is-bun-module` as optional peer dependency Technically this is a BREAKING CHANGE, but considering we just raise out v4 recently and this only affects `bun` users, `bun --bun eslint` even works without this dependency, so I'd consider this as a minor change. So for `bun` users, there are three options: 1. install `is-bun-module` dependency manually and use `bun: true` option 2. run `eslint` with `bun --bun eslint` w/o `bun: true` option 3. enable `run#bun` in [`bunfig.toml`](https://bun.sh/docs/runtime/bunfig#run-bun-auto-alias-node-to-bun) w/o `bun: true` option ## [v4.1.1](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#411) ##### Patch Changes - [#389](import-js/eslint-import-resolver-typescript#389) [`1b97d8a`](import-js/eslint-import-resolver-typescript@1b97d8a) Thanks [@JounQin](https://github.com/JounQin)! - fix: should prefer `module.isBuiltin` when `process.versions.bun` available ## [v4.1.0](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#410) ##### Minor Changes - [#387](import-js/eslint-import-resolver-typescript#387) [`ef5cd10`](import-js/eslint-import-resolver-typescript@ef5cd10) Thanks [@JounQin](https://github.com/JounQin)! - feat: add a new `bun?: boolean` option for `bun` users - close [#386](import-js/eslint-import-resolver-typescript#386) `process.versions.bun` is unavailable even with `bun eslint` due to its own design, but checking `bun` modules for non-bun users is incorrect behavior and just wasting time, so a new option is added for such case, you can still run with `bun --bun eslint` without this option enabled ## [v4.0.0](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#400) ##### Major Changes - [#368](import-js/eslint-import-resolver-typescript#368) [`2fd7c2e`](import-js/eslint-import-resolver-typescript@2fd7c2e) Thanks [@JounQin](https://github.com/JounQin)! - feat!: rewrite, speed up by using [`rspack-resolver`](https://github.com/unrs/rspack-resolver) which supports `references` natively under the hood BREAKING CHANGES: - drop Node 14 support, Node `^16.17.0 || >=18.6` is now required - `alwaysTryTypes` is enabled by default, you can set it as `false` to opt-out - array type of `project` is discouraged but still supported, single `project` with `references` are encouraged for better performance, you can enable `noWarnOnMultipleProjects` option to supress the warning message - root `tsconfig.json` or `jsconfig.json` will be used automatically if no `project` provided
| datasource | package | from | to | | ---------- | --------------------------------- | ----- | ----- | | npm | eslint-import-resolver-typescript | 3.9.1 | 4.2.2 | ## [v4.2.2](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#422) ##### Patch Changes - [#397](import-js/eslint-import-resolver-typescript#397) [`14a7688`](import-js/eslint-import-resolver-typescript@14a7688) Thanks [@JounQin](https://github.com/JounQin)! - chore: bump `rspack-resolver` for better P'n'P support Now `rspack-resolver` resolves `pnpapi` natively. ## [v4.2.1](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#421) ##### Patch Changes - [#394](import-js/eslint-import-resolver-typescript#394) [`9f11f6b`](import-js/eslint-import-resolver-typescript@9f11f6b) Thanks [@JounQin](https://github.com/JounQin)! - fix: don't set empty `configFile` when no `tsconfig` found - [#394](import-js/eslint-import-resolver-typescript#394) [`9f11f6b`](import-js/eslint-import-resolver-typescript@9f11f6b) Thanks [@JounQin](https://github.com/JounQin)! - chore: bump `rspack-resolver` to v1.2.0 ## [v4.2.0](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#420) ##### Minor Changes - [#391](import-js/eslint-import-resolver-typescript#391) [`c8121e5`](import-js/eslint-import-resolver-typescript@c8121e5) Thanks [@JounQin](https://github.com/JounQin)! - feat: make `is-bun-module` as optional peer dependency Technically this is a BREAKING CHANGE, but considering we just raise out v4 recently and this only affects `bun` users, `bun --bun eslint` even works without this dependency, so I'd consider this as a minor change. So for `bun` users, there are three options: 1. install `is-bun-module` dependency manually and use `bun: true` option 2. run `eslint` with `bun --bun eslint` w/o `bun: true` option 3. enable `run#bun` in [`bunfig.toml`](https://bun.sh/docs/runtime/bunfig#run-bun-auto-alias-node-to-bun) w/o `bun: true` option ## [v4.1.1](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#411) ##### Patch Changes - [#389](import-js/eslint-import-resolver-typescript#389) [`1b97d8a`](import-js/eslint-import-resolver-typescript@1b97d8a) Thanks [@JounQin](https://github.com/JounQin)! - fix: should prefer `module.isBuiltin` when `process.versions.bun` available ## [v4.1.0](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#410) ##### Minor Changes - [#387](import-js/eslint-import-resolver-typescript#387) [`ef5cd10`](import-js/eslint-import-resolver-typescript@ef5cd10) Thanks [@JounQin](https://github.com/JounQin)! - feat: add a new `bun?: boolean` option for `bun` users - close [#386](import-js/eslint-import-resolver-typescript#386) `process.versions.bun` is unavailable even with `bun eslint` due to its own design, but checking `bun` modules for non-bun users is incorrect behavior and just wasting time, so a new option is added for such case, you can still run with `bun --bun eslint` without this option enabled ## [v4.0.0](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#400) ##### Major Changes - [#368](import-js/eslint-import-resolver-typescript#368) [`2fd7c2e`](import-js/eslint-import-resolver-typescript@2fd7c2e) Thanks [@JounQin](https://github.com/JounQin)! - feat!: rewrite, speed up by using [`rspack-resolver`](https://github.com/unrs/rspack-resolver) which supports `references` natively under the hood BREAKING CHANGES: - drop Node 14 support, Node `^16.17.0 || >=18.6` is now required - `alwaysTryTypes` is enabled by default, you can set it as `false` to opt-out - array type of `project` is discouraged but still supported, single `project` with `references` are encouraged for better performance, you can enable `noWarnOnMultipleProjects` option to supress the warning message - root `tsconfig.json` or `jsconfig.json` will be used automatically if no `project` provided
| datasource | package | from | to | | ---------- | --------------------------------- | ----- | ----- | | npm | eslint-import-resolver-typescript | 3.9.1 | 4.2.2 | ## [v4.2.2](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#422) ##### Patch Changes - [#397](import-js/eslint-import-resolver-typescript#397) [`14a7688`](import-js/eslint-import-resolver-typescript@14a7688) Thanks [@JounQin](https://github.com/JounQin)! - chore: bump `rspack-resolver` for better P'n'P support Now `rspack-resolver` resolves `pnpapi` natively. ## [v4.2.1](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#421) ##### Patch Changes - [#394](import-js/eslint-import-resolver-typescript#394) [`9f11f6b`](import-js/eslint-import-resolver-typescript@9f11f6b) Thanks [@JounQin](https://github.com/JounQin)! - fix: don't set empty `configFile` when no `tsconfig` found - [#394](import-js/eslint-import-resolver-typescript#394) [`9f11f6b`](import-js/eslint-import-resolver-typescript@9f11f6b) Thanks [@JounQin](https://github.com/JounQin)! - chore: bump `rspack-resolver` to v1.2.0 ## [v4.2.0](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#420) ##### Minor Changes - [#391](import-js/eslint-import-resolver-typescript#391) [`c8121e5`](import-js/eslint-import-resolver-typescript@c8121e5) Thanks [@JounQin](https://github.com/JounQin)! - feat: make `is-bun-module` as optional peer dependency Technically this is a BREAKING CHANGE, but considering we just raise out v4 recently and this only affects `bun` users, `bun --bun eslint` even works without this dependency, so I'd consider this as a minor change. So for `bun` users, there are three options: 1. install `is-bun-module` dependency manually and use `bun: true` option 2. run `eslint` with `bun --bun eslint` w/o `bun: true` option 3. enable `run#bun` in [`bunfig.toml`](https://bun.sh/docs/runtime/bunfig#run-bun-auto-alias-node-to-bun) w/o `bun: true` option ## [v4.1.1](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#411) ##### Patch Changes - [#389](import-js/eslint-import-resolver-typescript#389) [`1b97d8a`](import-js/eslint-import-resolver-typescript@1b97d8a) Thanks [@JounQin](https://github.com/JounQin)! - fix: should prefer `module.isBuiltin` when `process.versions.bun` available ## [v4.1.0](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#410) ##### Minor Changes - [#387](import-js/eslint-import-resolver-typescript#387) [`ef5cd10`](import-js/eslint-import-resolver-typescript@ef5cd10) Thanks [@JounQin](https://github.com/JounQin)! - feat: add a new `bun?: boolean` option for `bun` users - close [#386](import-js/eslint-import-resolver-typescript#386) `process.versions.bun` is unavailable even with `bun eslint` due to its own design, but checking `bun` modules for non-bun users is incorrect behavior and just wasting time, so a new option is added for such case, you can still run with `bun --bun eslint` without this option enabled ## [v4.0.0](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#400) ##### Major Changes - [#368](import-js/eslint-import-resolver-typescript#368) [`2fd7c2e`](import-js/eslint-import-resolver-typescript@2fd7c2e) Thanks [@JounQin](https://github.com/JounQin)! - feat!: rewrite, speed up by using [`rspack-resolver`](https://github.com/unrs/rspack-resolver) which supports `references` natively under the hood BREAKING CHANGES: - drop Node 14 support, Node `^16.17.0 || >=18.6` is now required - `alwaysTryTypes` is enabled by default, you can set it as `false` to opt-out - array type of `project` is discouraged but still supported, single `project` with `references` are encouraged for better performance, you can enable `noWarnOnMultipleProjects` option to supress the warning message - root `tsconfig.json` or `jsconfig.json` will be used automatically if no `project` provided
Highlights:
rspack-resolver
understand the hood,I think we can make it as default resolver forcc @SukkaWeslint-plugin-import-x
different default options with originalpreferseslint-import-resolver-oxc
whichtypes
overjs
, what means@types/*
is supported,alwaysTryTypes
option is also preservedI'm thinking enable
alwaysTryTypes
by default or just remove this optionproject
option is deprecated,tsconfig
option fromoxc-resolver
is the replacement, but it's still supported temporarily by auto mapping totsconfig.configFile
project
isdeprecated
, use a singletsconfig.json
withreferences
instead, but it's still supported temporarily by only using the firstproject
Important
Dropping array type of
project
needs more discussionsclose #94
close #165
close #176
close #179
close #208
close #262
close #282
close #335
close #349
There is only one regression:
withPathsAndNestedBaseUrl
, it seemsoxc-resolver
does not usebaseUrl
as default alias. See also #68https://github.com/import-js/eslint-import-resolver-typescript/tree/feat/rework/tests/withPathsAndNestedBaseUrl
Dir
tsconfig.configFile
is not supported, is this intented?cc @Boshen
Can you help to export
OxcResolverOptions
andResolverFactory
for resuing?And also I think
removeQuerystring
function should be included ineslint-import-resolver-oxc
cc @9romise