Skip to content

docs: add why section on the top to clarify #323

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
May 16, 2025
Merged

docs: add why section on the top to clarify #323

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
35a0087
docs: add `why` section on the top to clarify
JounQin May 14, 2025
0d2833a
docs: add the proof link of `heavy dependency on Typescript`
JounQin May 15, 2025
ae2c473
Apply suggestions from code review
JounQin May 15, 2025
2db56e5
Apply suggestions from code review
JounQin May 15, 2025
360a34b
chore: let's make AI happy
JounQin May 15, 2025
f74abd6
Apply suggestions from code review
JounQin May 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .nano-staged.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export { default } from '@1stg/lint-staged/tsc'
export { default } from '@1stg/nano-staged/tsc'
38 changes: 38 additions & 0 deletions README.md
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I've been swamped this week but hoped to get to reviewing this before merge. Filing a bookmark to come back to you with a review soon. Sorry for the delay!

Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,16 @@

This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, and prevent issues with misspelling of file paths and import names. All the goodness that the ES2015+ static module syntax intends to provide, marked up in your editor.

It started as a fork of [`eslint-plugin-import`] using [`get-tsconfig`] to replace [`tsconfig-paths`] and heavy [`typescript`] under the hood, making it faster, through less [heavy dependency on Typescript](https://github.com/import-js/eslint-plugin-import/blob/da5f6ec13160cb288338db0c2a00c34b2d932f0d/src/exportMap/typescript.js#L16), and cleaner dependencies altogether.
Copy link
Contributor

Choose a reason for hiding this comment

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

to replace tsconfig-paths and heavy typescript under the hood, making it faster

  • How?
  • Why?

Copy link
Member Author

Choose a reason for hiding this comment

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

How?

replace tsconfig-paths and heavy typescript with single get-tsconfig?

Why?

typescript is heavy to load at https://github.com/import-js/eslint-plugin-import/blob/da5f6ec13160cb288338db0c2a00c34b2d932f0d/src/exportMap/typescript.js#L16


[`eslint-plugin-i` is now `eslint-plugin-import-x`](https://github.com/un-ts/eslint-plugin-import-x/issues/24#issuecomment-1991605123)

**IF YOU ARE USING THIS WITH SUBLIME**: see the [bottom section](#sublimelinter-eslint) for important info.

## TOC <!-- omit in toc -->

- [Why](#why)
- [Differences](#differences)
- [Installation](#installation)
- [Configuration (legacy: `.eslintrc*`)](#configuration-legacy-eslintrc)
- [TypeScript](#typescript)
Expand Down Expand Up @@ -50,6 +54,34 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a
- [Changelog](#changelog)
- [License](#license)

## Why

Because a lot of issues can not be fixed easily without API changes, see

- <https://github.com/import-js/eslint-plugin-import/issues/1479>
- <https://github.com/import-js/eslint-plugin-import/issues/2108>
- <https://github.com/import-js/eslint-plugin-import/issues/2111>
Comment on lines +61 to +63
Copy link
Contributor

Choose a reason for hiding this comment

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

(more on this on the next line) Having three issue links is a good start, but all three require a ton of reading. Two are labeled as help wanted. Someone cursorily reading these docs could easily come away with the conclusion that these issues are just pending in upstream, not actually blockers.


But [`eslint-plugin-import`] refused to accept BREAKING CHANGES for these issues, so we have to fork it.

And then [`eslint-plugin-import`] claimed to be allowed to update with BREAKING CHANGES at <https://github.com/un-ts/eslint-plugin-import-x/issues/170>,
but it still doesn't happen or even seems to happen: <https://github.com/import-js/eslint-plugin-import/pull/3091>.

We haven't resolved all the issues yet, but we are working on it which could happen in the next major version (v5): <https://github.com/un-ts/eslint-plugin-import-x/issues/235>.

## Differences

So what's the differences from `eslint-plugin-import` exactly?

- we target [Node `^18.18.0 || ^20.9.0 || >=21.1.0`](https://github.com/un-ts/eslint-plugin-import-x/blob/8b2d6d3b612eb57fb68c3fddec25b02fc622df7c/package.json#L12) + [ESLint `^8.57.0 || ^9.0.0`](https://github.com/un-ts/eslint-plugin-import-x/blob/8b2d6d3b612eb57fb68c3fddec25b02fc622df7c/package.json#L71), while `eslint-plugin-import` targets [Node `>=4`](https://github.com/import-js/eslint-plugin-import/blob/da5f6ec13160cb288338db0c2a00c34b2d932f0d/package.json#L6) and [ESLint `^2 || ^3 || ^4 || ^5 || ^6 || ^7.2.0 || ^8 || ^9`](https://github.com/import-js/eslint-plugin-import/blob/da5f6ec13160cb288338db0c2a00c34b2d932f0d/package.json#L115C16-L115C64)
- we don't depend on old and outdated dependencies, so we have short dependency depths, see also <https://github.com/un-ts/eslint-plugin-import-x/pull/11>
- `eslint-plugin-import` uses `tsconfig-paths` + `typescript` itself to load `tsconfig`s while we use the single `get-tsconfig` instead, which is much faster and cleaner
Copy link
Contributor

Choose a reason for hiding this comment

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

much faster

Prove it! Show me the data! 😄

As a developer, I have been trained to not believe any performance comparison not backed up by data. One should always back perf comparisons up with repeatable experiments.

Note that isolated experiments in get-tsconfig itself are not enough. The package might be fast in isolation but equal -or even slower!- in real-world usage. I need actual data from this specific plugin use case.

Copy link
Member Author

@JounQin JounQin May 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Prove it! Show me the data! 😄

https://github.com/privatenumber/get-tsconfig#features

I can't say for 3rd part libraries. And I'm not interested to prove it because it may be hard as you said.

Similar package: https://github.com/dominikg/tsconfck with benchmark https://github.com/dominikg/tsconfck/blob/main/docs/benchmark.md

And get-tsconfig starts because tsconfck doesn't have sync API: dominikg/tsconfck#31 (comment), see also dominikg/tsconfck#183

cc @privatenumber

Copy link
Contributor

Choose a reason for hiding this comment

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

cleaner

Why would an end-user care about this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because we're cleaning the ecosystem? https://e18e.dev/guide/cleanup.html

- `eslint-plugin-import` uses [`resolve`] which doesn't support `exports` field in `package.json` while we use [`unrs-resolver`] instead, which is a [napi-rs] project so it's much faster
- Our [v3 resolver](./resolvers/README.md#v3) interface shares a single `resolver` instance by default which is used all across resolving chains so it would benefit from caching and memoization out of box
- ...

The list could be longer in the future, but we don't want to make it too long here, hope you enjoy and let's get started.

## Installation

```sh
Expand Down Expand Up @@ -662,13 +694,19 @@ Detailed changes for each release are documented in [CHANGELOG.md](./CHANGELOG.m
[MIT][] © [JounQin][]@[1stG.me][]

[`@typescript-eslint/parser`]: https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/parser
[`eslint-plugin-import`]: https://github.com/import-js/eslint-plugin-import
[`eslint-import-resolver-typescript`]: https://github.com/import-js/eslint-import-resolver-typescript
[`eslint_d`]: https://www.npmjs.com/package/eslint_d
[`eslint-loader`]: https://www.npmjs.com/package/eslint-loader
[`get-tsconfig`]: https://github.com/privatenumber/get-tsconfig
[`tsconfig-paths`]: https://github.com/dividab/tsconfig-paths
[`typescript`]: https://github.com/microsoft/TypeScript
[`unrs-resolver`]: https://github.com/unrs/unrs-resolver
[`resolve`]: https://www.npmjs.com/package/resolve
[`externals`]: https://webpack.github.io/docs/library-and-externals.html
[1stG.me]: https://www.1stG.me
[JounQin]: https://github.com/JounQin
[MIT]: http://opensource.org/licenses/MIT
[napi-rs]: https://github.com/napi-rs/napi-rs
[node]: https://www.npmjs.com/package/eslint-import-resolver-node
[webpack]: https://www.npmjs.com/package/eslint-import-resolver-webpack
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@
},
"devDependencies": {
"@1stg/commitlint-config": "^5.0.6",
"@1stg/lint-staged": "^4.0.9",
"@1stg/nano-staged": "^0.1.1",
"@1stg/prettier-config": "^5.1.4",
"@1stg/remark-preset": "^3.1.1",
"@1stg/simple-git-hooks": "^2.0.1",
Expand Down
30 changes: 15 additions & 15 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,31 +16,31 @@ __metadata:
languageName: node
linkType: hard

"@1stg/config@npm:^1.0.4, @1stg/config@npm:^1.0.5":
"@1stg/config@npm:^1.0.5":
version: 1.0.5
resolution: "@1stg/config@npm:1.0.5"
checksum: 10c0/f7e4d638f45ea458fc80763ad26402f4606fd2cf37edb282777ab804b1c8abea866185e8287328754675261dc0bcd741d55a88338844bbf79ddc476dbe9b9503
languageName: node
linkType: hard

"@1stg/lint-staged@npm:^4.0.9":
version: 4.0.9
resolution: "@1stg/lint-staged@npm:4.0.9"
"@1stg/nano-staged@npm:^0.1.1":
version: 0.1.1
resolution: "@1stg/nano-staged@npm:0.1.1"
dependencies:
"@1stg/config": "npm:^1.0.4"
"@1stg/prettier-config": "npm:^5.0.1"
"@1stg/tsconfig": "npm:^3.0.2"
"@1stg/config": "npm:^1.0.5"
"@1stg/prettier-config": "npm:^5.1.1"
"@1stg/tsconfig": "npm:^3.0.3"
"@pkgr/core": "npm:^0.2.2"
prettier: "npm:^3.5.3"
peerDependencies:
lint-staged: ">=12.1.0"
checksum: 10c0/bc576e1b9e8f0fe260f9b3d00b631baa3e74088c41f03936be7028e549fcc57f0210fc8bdd69e574935653e4315fbc91e3e579776ccd0af95448b7ab15780b87
nano-staged: ">=0.8.0"
checksum: 10c0/3e0652c36c06e5c14836bf2329eb0da9d8cf6e9bb1a59ef4bc27fb85672eea685936d1b9d997ec56c3673e5458ee8cfaf34a79be701798c29b48b542bb0b0dc0
languageName: node
linkType: hard

"@1stg/prettier-config@npm:^5.0.1, @1stg/prettier-config@npm:^5.1.4":
version: 5.1.4
resolution: "@1stg/prettier-config@npm:5.1.4"
"@1stg/prettier-config@npm:^5.1.1, @1stg/prettier-config@npm:^5.1.4":
version: 5.2.0
resolution: "@1stg/prettier-config@npm:5.2.0"
dependencies:
"@1stg/config": "npm:^1.0.5"
"@prettier/plugin-pug": "npm:^3.3.0"
Expand All @@ -57,7 +57,7 @@ __metadata:
prettier-plugin-toml: "npm:^2.0.4"
peerDependencies:
prettier: ^3.0.0
checksum: 10c0/7950dca3881be0604eff57191b34a0f7efc0377b756225ba7ec969c81ef5cf5db2183d574679c4a1d9224e5597e110d5e78f476aab768e15d434b1c5509bbc3b
checksum: 10c0/a9879a9a0a84aac41d6896009f109ca03f2544e836bc388df3c581a45c4b47e2369b2104eaf35f3bff9d22fef6f2c6bb0f9941f1381b89f5ad0388e805cec028
languageName: node
linkType: hard

Expand Down Expand Up @@ -94,7 +94,7 @@ __metadata:
languageName: node
linkType: hard

"@1stg/tsconfig@npm:^3.0.2, @1stg/tsconfig@npm:^3.0.3":
"@1stg/tsconfig@npm:^3.0.3":
version: 3.0.3
resolution: "@1stg/tsconfig@npm:3.0.3"
peerDependencies:
Expand Down Expand Up @@ -6346,7 +6346,7 @@ __metadata:
resolution: "eslint-plugin-import-x@workspace:."
dependencies:
"@1stg/commitlint-config": "npm:^5.0.6"
"@1stg/lint-staged": "npm:^4.0.9"
"@1stg/nano-staged": "npm:^0.1.1"
"@1stg/prettier-config": "npm:^5.1.4"
"@1stg/remark-preset": "npm:^3.1.1"
"@1stg/simple-git-hooks": "npm:^2.0.1"
Expand Down