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] Bad auto fix for typescript #2697

Open
latin-1 opened this issue Jan 29, 2023 · 5 comments
Open

[no-duplicates] Bad auto fix for typescript #2697

latin-1 opened this issue Jan 29, 2023 · 5 comments

Comments

@latin-1
Copy link

latin-1 commented Jan 29, 2023

Regardless of the prefer-inline option.

Input

import type { A } from "./mod";
import { type B, C } from "./mod";

Output

import type { A, type B, C } from "./mod";

cc @snewcomer

@snewcomer
Copy link
Contributor

snewcomer commented Jan 29, 2023

@latin-1 Thanks for the issue! Yes, it doesn't look like it is performing correctly. Both the type and inline type specifiers for an import statement are not allowed. Looking at this and a few other issues today.

https://www.typescriptlang.org/play?#code/JYWwDg9gTgLgBDAnmApnA3nAggGgctAITgF84AzKCEOAcgDoB6ECAE1oG4g

If prefer-inline: true, I'm guessing it should look like this

import { type A, type B, C } from "./mod";

If it is false or not specified, it should stay the same.

import type { A } from "./mod";
import { type B, C } from "./mod";

source of error - https://github.com/import-js/eslint-plugin-import/pull/2475/files#r1089976521

@snewcomer
Copy link
Contributor

snewcomer commented Jan 30, 2023

Furthermore, I wonder if instead we should make our best effort to collapse.

prefer-inline false

import type { A, B } from "./mod";
import { C } from "./mod";

or depending on the order of the original

import { C } from "./mod";
import type { A, B } from "./mod";
  • note about the code I have come to learn. The ‘first, rest imports’ doesn’t necessarily work too well when dealing with with deduping type imports.

The easier case to handle is prefer-inline true. However, if false, this isn’t terribly hard to handle as well. The above example shows that we could pull all inline types and type imports from other Type/InlineType ImportDeclarations to the first Type ImportDeclaration and leave the Value ImportDeclarations to their own import statement. The flexibility here is important esp when considering many (>2) duplicate import statements in any order - maybe the value import is first, maybe the type import is first.

@snewcomer
Copy link
Contributor

@ljharb Hi Jordan. So basically I have a POC to fix this issue but also make the process of collapsing ‘type’ duplicates more flexible. The original solution I did was entirely inflexible :). I was hoping to either get alignment for the previous comment code block, whether you wanted more explanation, or if you felt it should go elsewhere.

@ljharb
Copy link
Member

ljharb commented Jan 30, 2023

Yes, I think it would be ideal to maximally collapse, preserving order as best as can be (import/order can clean that up further).

@dylang
Copy link

dylang commented Sep 27, 2023

Possibly related: #2792

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

4 participants