Skip to content

import-x/order mismatches typescript language server organize imports for # prefixed imports #286

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

Open
Shinigami92 opened this issue Apr 10, 2025 · 7 comments · May be fixed by #287
Open

Comments

@Shinigami92
Copy link
Member

Minimal reproducible: https://github.com/Shinigami92/eslint-plugin-import-x-repro

input

import { internA } from "#a";
import { scopeA } from "@a/a";
import { localA } from "./a";

console.log({ internA, scopeA, localA });

actual

import-x/order reports error

expected

should not report, because when running typescript's lsp organize imports, then the above input order is enforced

context

the # prefix comes from package.json "imports": { ... } field: https://nodejs.org/api/packages.html#imports

@JounQin
Copy link
Member

JounQin commented Apr 10, 2025

It depends on where the final module path resolved, mostly, #a would be resolved as internal group, imports as top is not a already-have feature, but I think it could be add a new option for this?

@Shinigami92
Copy link
Member Author

It depends on where the final module path resolved, mostly, #a would be resolved as internal group, imports as top is not a already-have feature, but I think it could be add a new option for this?

Hey @JounQin 🙂
I personally do not care how it is implemented under the hood, either with or without a new group. But the default behavior should be compatible with TS LSP.

I'm also not sure if this is an issue in eslint-plugin-import-x or a dependency the plugin is just using.

But I'm potentially willing to implement the fix with a PR if I get some instructions where to start. 😺

@JounQin
Copy link
Member

JounQin commented Apr 10, 2025

But the default behavior should be compatible with TS LSP.

There is no such convention?

But I'm potentially willing to implement the fix with a PR if I get some instructions where to start. 😺

I'm fine with adding a new option, but I don't know how to name it, you can give your choice via a PR. The sorting ranks happen at

function computeRank(

@Shinigami92
Copy link
Member Author

There is no such convention?

I'm not sure if there is something like it in eslint-plugin-import-x

In @simonhaenisch's prettier-plugin-organize-imports we called the native language server function: https://github.com/simonhaenisch/prettier-plugin-organize-imports/blob/e79ed64e5ecd05bdd65140b814b37c9a1a360767/lib/organize.js#L24-L30

This is then the same as calling it in VSCode manually: https://code.visualstudio.com/docs/typescript/typescript-refactoring#_organize-imports

I'm fine with adding a new option, but I don't know how to name it, you can give your choice via a PR. The sorting ranks happen at

eslint-plugin-import-x/src/rules/order.ts

Line 715 in 877bbbb

function computeRank(

OK thx, I will have a look into it after company work in my private time 👍

@JounQin
Copy link
Member

JounQin commented Apr 10, 2025

I'm not sure if there is something like it in eslint-plugin-import-x

I know those tools follow TS LSP by using it under the hood, but eslint-plugin-import(-x) and eslint-plugin-simple-import-sort are created before such things AFAIK, we could try to match what you proposed but not using LSP service which requires typescript in dependency.

cc @SukkaW

@Shinigami92
Copy link
Member Author

I know those tools follow TS LSP by using it under the hood, but eslint-plugin-import(-x) and eslint-plugin-simple-import-sort are created before such things AFAIK, we could try to match what you proposed but not using LSP service which requires typescript in dependency.

Matching it is totally fine 👍
We could potentially think about to use typescript as dev-dependency to ensure via unit test, but a one-time setup with static tests is also totally fine

@JounQin
Copy link
Member

JounQin commented Apr 10, 2025

typescript is surely already a dev dep, it's pure TypeScript project. 🍺

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

Successfully merging a pull request may close this issue.

2 participants