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

☂️ eslint-plugin-import #1117

Open
Tracked by #20279
Boshen opened this issue Oct 30, 2023 · 20 comments
Open
Tracked by #20279

☂️ eslint-plugin-import #1117

Boshen opened this issue Oct 30, 2023 · 20 comments
Labels
A-linter Area - Linter

Comments

@Boshen
Copy link
Member

Boshen commented Oct 30, 2023

Warning

This comment is maintained by CI. Do not edit this comment directly.
To update comment template, see https://github.com/oxc-project/oxc/tree/main/tasks/lint_rules

This is tracking issue for eslint-plugin-import.

There are 44(+ 1 deprecated) rules.

  • 0/8 recommended rules are remaining as TODO
    • All done! 🎉
  • 28/36 not recommended rules are remaining as TODO

To get started, run the following command:

just new-import-rule <RULE_NAME>

Then register the rule in crates/oxc_linter/src/rules.rs and also declare_all_lint_rules at the bottom.

Recommended rules

✨: 7, 🚫: 1 / total: 8
Status Name Docs
🚫 import/no-unresolved https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/no-unresolved.md
import/named https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/named.md
import/default https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/default.md
import/namespace https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/namespace.md
import/export https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/export.md
import/no-named-as-default https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/no-named-as-default.md
import/no-named-as-default-member https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/no-named-as-default-member.md
import/no-duplicates https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/no-duplicates.md

✨ = Implemented, 🚫 = No need to implement

Not recommended rules

✨: 8, 🚫: 0 / total: 36
Status Name Docs
import/no-namespace https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/no-namespace.md
import/no-mutable-exports https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/no-mutable-exports.md
import/extensions https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/extensions.md
import/no-restricted-paths https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/no-restricted-paths.md
import/no-internal-modules https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/no-internal-modules.md
import/group-exports https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/group-exports.md
import/no-relative-packages https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/no-relative-packages.md
import/no-relative-parent-imports https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/no-relative-parent-imports.md
import/consistent-type-specifier-style https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/consistent-type-specifier-style.md
import/no-self-import https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/no-self-import.md
import/no-cycle https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/no-cycle.md
import/no-named-default https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/no-named-default.md
import/no-anonymous-default-export https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/no-anonymous-default-export.md
import/no-unused-modules https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/no-unused-modules.md
import/no-commonjs https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/no-commonjs.md
import/no-amd https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/no-amd.md
import/first https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/first.md
import/max-dependencies https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/max-dependencies.md
import/no-extraneous-dependencies https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/no-extraneous-dependencies.md
import/no-absolute-path https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/no-absolute-path.md
import/no-nodejs-modules https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/no-nodejs-modules.md
import/no-webpack-loader-syntax https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/no-webpack-loader-syntax.md
import/order https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/order.md
import/newline-after-import https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/newline-after-import.md
import/prefer-default-export https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/prefer-default-export.md
import/no-default-export https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/no-default-export.md
import/no-named-export https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/no-named-export.md
import/no-dynamic-require https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/no-dynamic-require.md
import/unambiguous https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/unambiguous.md
import/no-unassigned-import https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/no-unassigned-import.md
import/no-useless-path-segments https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/no-useless-path-segments.md
import/dynamic-import-chunkname https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/dynamic-import-chunkname.md
import/no-import-module-exports
import/no-empty-named-blocks https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/no-empty-named-blocks.md
import/exports-last https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/exports-last.md
import/no-deprecated https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/no-deprecated.md

✨ = Implemented, 🚫 = No need to implement

Deprecated rules

✨: 0, 🚫: 0 / total: 1
Status Name Docs
import/imports-first https://github.com/import-js/eslint-plugin-import/blob/7b25c1cb95ee18acc1531002fd343e1e6031f9ed/docs/rules/imports-first.md

✨ = Implemented, 🚫 = No need to implement

@Boshen Boshen added the A-linter Area - Linter label Oct 30, 2023
@XantreDev

This comment was marked as resolved.

@XantreDev

This comment was marked as outdated.

@XantreDev

This comment was marked as resolved.

@camc314

This comment was marked as resolved.

@XantreDev

This comment was marked as resolved.

@Boshen
Copy link
Member Author

Boshen commented Jan 19, 2024

@XantreGodlike To be honest I don't really get this rule, and there's so many issues with it https://github.com/search?q=repo%3Aimport-js%2Feslint-plugin-import+no-named-as-default&type=issues

But for default exports, you may add a Option<Atom> because you can default export a function, which may not have a name.

@yordis
Copy link

yordis commented Jan 24, 2024

yarn pnp support is huge for us 🙏🏻 🙏🏻 🙏🏻 🙏🏻 🙏🏻 🙏🏻 🙏🏻 🙏🏻 🙏🏻 🙏🏻 🙏🏻 🙏🏻 🙏🏻

@XantreDev
Copy link
Contributor

@yordis Is it separate rule?

@XantreDev
Copy link
Contributor

XantreDev commented Feb 17, 2024

There is no new-import-rule

@Boshen
Copy link
Member Author

Boshen commented Feb 18, 2024

There is not new-import-rule

The test cases eslint-plugin-import's aren't static enough for us to convert automatically, all the test cases are ported manually :-(

@lachieh
Copy link

lachieh commented Feb 20, 2024

Regarding yarn pnp support, it's a method of loading dependencies using a cache of zip files rather than the node_modules folder. It uses a custom loader resolution file that exports a generated list of packages and their zip counterparts. More information here: https://yarnpkg.com/features/pnp

@Boshen
Copy link
Member Author

Boshen commented Feb 22, 2024

I want to tackle the list of super slow rules that use ExportMap in eslint-plugin-import:

  • default (recommended)
  • export
  • named (recommended)
  • namespace (recommended)
  • no-cycle
  • no-deprecated
  • no-named-as-default (recommended)
  • no-named-as-default-member (recommended)
  • no-unused-modules

Wish me luck.

@Boshen
Copy link
Member Author

Boshen commented Feb 22, 2024

To port the tests, we'll need to clone eslint-plugin-import, add a console.log to copy the code over.

console.log("let pass = vec![" + valid.map((test) => `r#\"${test.code.trim()}\"#`).join(',\n') + "];\n")
console.log("let fail = vec![" + invalid.map((test) => `r#\"${test.code.trim()}\"#`).join(',\n') + "];")

@XantreDev
Copy link
Contributor

Great news)

@Boshen
Copy link
Member Author

Boshen commented Feb 24, 2024

After battling cjs / esm for 3 days ... I'm going to cut the scope and focus on ESM only. i.e. Only lint if the imported file is a ESM file.

Most of the rules are working as intended unless a CJS file is imported, which may contain any kind of bizarre export :-)


Performance wise, we should expect import-plugin + all the other rules finish within a second on a M2.

In a typical project with thousands of files:

AFFiNE canary ❯ ~/github/oxc/target/release/oxlint --import-plugin -D correctness -D no-cycle
Finished in 224ms on 1295 files with 88 rules using 8 threads.
Found 0 warnings and 0 errors.

@Boshen
Copy link
Member Author

Boshen commented Feb 26, 2024

Maybe a good addition: https://github.com/thepassle/barrel-begone

@steelbrain
Copy link

Sharing some data on how much some ESLint rules take in the world. Would be nice to prioritise the slow ones

Rule                                          | Time (ms) | Relative
:---------------------------------------------|----------:|--------:
import/namespace                              | 27496.040 |    24.5%
@typescript-eslint/await-thenable             |  8834.873 |     7.9%
import/no-extraneous-dependencies             |  8648.620 |     7.7%
@typescript-eslint/no-floating-promises       |  7816.454 |     7.0%
react/destructuring-assignment                |  5010.386 |     4.5%
@typescript-eslint/strict-boolean-expressions |  4807.847 |     4.3%
import/extensions                             |  4265.243 |     3.8%
local/disallow-null-attributes                |  4235.864 |     3.8%
@typescript-eslint/no-unused-vars             |  4053.110 |     3.6%
react/no-this-in-sfc                          |  2530.419 |     2.3%

@Boshen
Copy link
Member Author

Boshen commented Jun 17, 2024

import/no-unused-modules is useful and should be prioritized.

@voxpelli
Copy link

voxpelli commented Jul 1, 2024

import/no-unused-modules is useful and should be prioritized.

Not sure if that rule even belongs in a linter or if a separate tool like knip is a better idea: https://knip.dev/

@protoEvangelion
Copy link

import/no-restricted-paths This one is a blocker to restrict imports to prevent importing code that shouldn't be in a mono repo. https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/no-restricted-paths.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linter Area - Linter
Projects
None yet
Development

No branches or pull requests

8 participants