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

[no-duplicates] prefer-inline breaks unassigned and top-level type imports #3130

Open
oleasteo opened this issue Dec 23, 2024 · 2 comments
Open

Comments

@oleasteo
Copy link

oleasteo commented Dec 23, 2024

"import/no-duplicates": ["error", { "prefer-inline": true }],

Will detect an error in the following typescript code:

import type { X } from "xyz";

import "xyz";

// [...]

It will auto-fix by removing the import "xyz"; statement. This is not the desired behavior. I would expect it to stay as is.

Merry Christmas 🎄

@ljharb
Copy link
Member

ljharb commented Dec 23, 2024

I would expect prefer-inline: true to require import { type X } from 'xyz'; for the first line; does it not?

For the second line, I assume this is because xyz is side-effecting, and you want to ensure type-stripping leaves the evaluation intact?

@oleasteo
Copy link
Author

It does not require import { type X } from 'xyz';. It does prefer mixed imports over one import for types and a separate one for named runtime.

import { abc } from "xyz";
import type { X } from "xyz";

would be changed to

import { abc, type X } from "xyz";

but if no named runtime imports exist, it won't touch import type ... declarations (which is consistent with typescript verbatimModuleSyntax).

For the second line, I assume this is because xyz is side-effecting, and you want to ensure type-stripping leaves the evaluation intact?

Precisely.


FYI, the style I'm aiming for with all the relevant rules:

  1. make sure type-only use is type-imported (@typescript-eslint/consistent-type-imports)
  2. avoid duplicate imports, including mixed type/runtime declarations (import/no-duplicates { "prefer-inline": true })
  3. if only type imports exist, make it top-level import type {...} (@typescript-eslint/no-import-type-side-effects)
    • this allows verbatimModuleSyntax to strip type-only imports
  4. don't touch import "..." statements at all; they're meant to explicitly define side-effect imports
    • I would prefer to automatically group them to the start / end of the import block, unless eslint-disabled per file, but that's a different beast

Personally, I think this is a perfect solution to clean imports. Especially, if combined with import/order to group import type separately.

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

No branches or pull requests

2 participants