Skip to content
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

chore: replace resolver-oxc with new resolver-typescript #386

Closed
wants to merge 2 commits into from

Conversation

JounQin
Copy link

@JounQin JounQin commented Mar 26, 2025

Hi, eslint-plugin-import-x and eslint-import-resolver-typescript owner here.

The brand new eslint-import-resolver-typescript is now powered by unrs-resolver which is fork of oxc-resolver and rspack-resolver with P'n'P support and more targets, and it has competitive performance compared with eslint-import-resolver-oxc.

Before with resolver-oxc:

 socket-cli on  main [$!] ❯ TIMING=1 npmR check:lint

> [email protected] check:lint
> eslint --report-unused-disable-directives .

Rule                                | Time (ms) | Relative
:-----------------------------------|----------:|--------:
import-x/namespace                  |  1131.479 |    62.1%
@typescript-eslint/return-await     |   288.136 |    15.8%
import-x/order                      |    98.431 |     5.4%
import-x/extensions                 |    30.366 |     1.7%
n/no-extraneous-import              |    19.631 |     1.1%
n/no-unsupported-features/es-syntax |    18.692 |     1.0%
n/no-extraneous-require             |    17.510 |     1.0%
import-x/no-duplicates              |     9.148 |     0.5%
no-misleading-character-class       |     8.638 |     0.5%
import-x/default                    |     8.380 |     0.5%
is 📦 v0.14.67 via  v23.7.0 took 15s

After with resolver-typescript:

 socket-cli on  main [+] ❯ TIMING=1 npmR check:lint

> [email protected] check:lint
> eslint --report-unused-disable-directives .

Rule                                | Time (ms) | Relative
:-----------------------------------|----------:|--------:
import-x/namespace                  |  1058.428 |    65.0%
@typescript-eslint/return-await     |   225.935 |    13.9%
import-x/order                      |    71.322 |     4.4%
import-x/extensions                 |    27.978 |     1.7%
n/no-extraneous-require             |    15.662 |     1.0%
n/no-extraneous-import              |    15.299 |     0.9%
n/no-unpublished-import             |    12.542 |     0.8%
n/no-unsupported-features/es-syntax |    11.428 |     0.7%
n/no-unpublished-require            |     9.040 |     0.6%
no-misleading-character-class       |     8.839 |     0.5%
is 📦 v0.14.67 via  v23.7.0 took 10s

Copy link

socket-security bot commented Mar 26, 2025

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/[email protected] Transitive: environment, eval, network, shell +28 51.9 MB

🚮 Removed packages: npm/[email protected]

View full report↗︎

@socket-security-staging
Copy link

socket-security-staging bot commented Mar 26, 2025

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/[email protected] Transitive: environment, eval, filesystem, network, shell +28 11.6 MB jounqin

🚮 Removed packages: npm/[email protected]

View full report↗︎

@socket-security-staging
Copy link

socket-security-staging bot commented Mar 26, 2025

🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎

To accept the risk, merge this PR and you will not be notified again.

Alert Package NoteSourceCI
Unpopular package npm/[email protected] ⚠︎

View full report↗︎

Next steps

What are unpopular packages?

This package is not very popular.

Unpopular packages may have less maintenance and contain other problems.

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/[email protected] or ignore all packages with @SocketSecurity ignore-all

@JounQin
Copy link
Author

JounQin commented Mar 26, 2025

Actually, rules like n/no-extraneous-require/n/no-extraneous-import can just be remove in favor of no-extraneous-dependencies alreay covering them.

@jdalton jdalton added the dependencies Pull requests that update a dependency file label Mar 26, 2025
Copy link
Collaborator

@jdalton jdalton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @JounQin. Holding for more thorough review.

@JounQin JounQin force-pushed the chore/import_resolver branch from ab70f30 to 21f254c Compare March 26, 2025 12:09
@JounQin
Copy link
Author

JounQin commented Mar 26, 2025

@jdalton By the way, I have troubles when running npm run bs locally

 socket-cli on  chore/import_resolver ❯ npmR bs

> [email protected] bs
> npm run build:dist; npm exec socket --


> [email protected] build:dist
> del-cli 'dist' && rollup -c .config/rollup.dist.config.mjs


/Users/JounQin/Workspaces/GitHub/socket-cli/src/cli.ts, /Users/JounQin/Workspaces/GitHub/socket-cli/src/constants.ts, /Users/JounQin/Workspaces/GitHub/socket-cli/src/shadow/npm/bin.ts, /Users/JounQin/Workspaces/GitHub/socket-cli/src/shadow/npm/inject.ts → dist/module-sync...
created dist/module-sync in 5.3s

/Users/JounQin/Workspaces/GitHub/socket-cli/src/cli.ts, /Users/JounQin/Workspaces/GitHub/socket-cli/src/constants.ts, /Users/JounQin/Workspaces/GitHub/socket-cli/src/shadow/npm/bin.ts, /Users/JounQin/Workspaces/GitHub/socket-cli/src/shadow/npm/inject.ts → dist/require...
created dist/require in 4.9s
node:internal/modules/cjs/loader:1397
  throw err;
  ^

{
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/Users/JounQin/Workspaces/GitHub/socket-cli/node_modules/@pnpm/lockfile-file/lib/logger.js',
    '/Users/JounQin/Workspaces/GitHub/socket-cli/node_modules/@pnpm/lockfile-file/lib/write.js',
    '/Users/JounQin/Workspaces/GitHub/socket-cli/node_modules/@pnpm/lockfile-file/lib/index.js',
    '/Users/JounQin/Workspaces/GitHub/socket-cli/dist/module-sync/cli.js'
  ]
}

Node.js v23.7.0

OK, should be related to Node v23 only because it tries require(ESM) automatically.

@pvdz pvdz self-requested a review March 26, 2025 13:54
@pvdz pvdz removed their request for review March 26, 2025 14:40
@jdalton
Copy link
Collaborator

jdalton commented Mar 30, 2025

Updated independently. Thank you @JounQin!

@jdalton jdalton closed this Mar 30, 2025
@JounQin JounQin deleted the chore/import_resolver branch March 30, 2025 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants