import-js/eslint-plugin-importPublic

NotificationsYou must be signed in to change notification settings
Fork1.6k
Star5.8k

[Fix] no-duplicates with type imports#2716

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Draft

snewcomer wants to merge10 commits intoimport-js:main

base:main

Choose a base branch

fromsnewcomer:sn/fix-no-duplicates

Draft

[Fix] no-duplicates with type imports#2716

snewcomer wants to merge10 commits intoimport-js:mainfromsnewcomer:sn/fix-no-duplicates

Conversation

Copy link

Contributor

snewcomer commentedFeb 12, 2023•
edited
Loading

Follow up fixes to#2475

Ref bug PRs:

snewcomer force-pushed thesn/fix-no-duplicates branch 2 times, most recently fromab7a2d8 tod2292bbCompare

February 12, 2023 20:13

ljharb reviewed

Feb 13, 2023

View reviewed changes

Copy link

Member

ljharb left a comment

Choose a reason for hiding this comment

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

and so on, all the loops should be iteration methods whenever possible :-)

src/rules/no-duplicates.js OutdatedShow resolvedHide resolved

Copy link

Contributor

simmo commentedFeb 14, 2023

Am I correct this would fix#2675 ? If so this is my new favourite PR. 😄

If you need any testing done, I can probably help.

Copy link

runspired commentedFeb 14, 2023

all the loops should be iteration methods whenever possible

Is there a reason for this guidance?

Copy link

Member

ljharb commentedFeb 14, 2023

@runspired loops make code harder to understand and reason about. It's explained in the airbnb styleguide as well.

snewcomer force-pushed thesn/fix-no-duplicates branch from3025495 to9abf816Compare

February 19, 2023 21:56

This was referencedFeb 20, 2023

[import/no-duplicates]{"prefer-inline": true} does not work#2715

Open

typescript: inline typesemberjs/ember.js#20378

Draft

ljharb reviewed

Feb 20, 2023

View reviewed changes

Copy link

Member

ljharb left a comment

Choose a reason for hiding this comment

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

Look forward to the other 2 fixes :-)

src/rules/no-duplicates.js OutdatedShow resolvedHide resolved

mrm007 mentioned this pull request

Apr 8, 2023

Reorganize testssnewcomer/eslint-plugin-import#1

Open

Copy link

mrm007 commentedApr 8, 2023

Look forward to the other 2 fixes :-)

@ljharb @snewcomer I am very interested in these fixes, so I wanted to take a stab at the remaining TODO items. As it turns out, they were already covered by this PR!

I reorganised some tests (inspired bytests/src/rules/consistent-type-specifier-style.js) to prove that both TypeScript and Flow work — seesnewcomer#1

Test results

no-duplicates<snip>with types  Babel/Flow    no-duplicates      valid        ✓ import type { x } from './foo'; import y from './foo'        ✓ import type x from './foo'; import type y from './bar'        ✓ import type {x} from './foo'; import type {y} from './bar'        ✓ import type x from './foo'; import type {y} from './foo'        ✓       import type {} from './module';      import {} from './module2';            ✓       import type { Identifier } from 'module';      declare module 'module2' {        import type { Identifier } from 'module';      }      declare module 'module3' {        import type { Identifier } from 'module';      }            ✓ import { type x } from './foo'; import type y from 'foo'      invalid        ✓ import { type x } from './foo'; import y from './foo';        ✓ import type x from './foo'; import type y from './foo'        ✓ import type x from './foo'; import type x from './foo'        ✓ import type {x} from './foo'; import type {y} from './foo'        ✓ import {type x} from './foo'; import {y} from './foo'        ✓ import type {x} from './foo'; import {type y} from './foo'        ✓ import type {x} from './foo';import {type y} from './foo';        ✓ import {type x} from './foo'; import type {y} from './foo';        ✓ import {type x} from 'foo'; import type {y} from 'foo'        ✓ import {type x, C} from './foo'; import type {y} from './foo';        ✓ import type {y} from './foo'; import {type x, C} from './foo';        ✓ import {type x, C} from 'foo';import type {y} from 'foo';        ✓ import type {y} from './foo'; import {type x, C} from './foo';        ✓ import {type x} from './foo';import {type y} from './foo';        ✓ import {type x} from './foo';import {type y} from './foo';        ✓ import {type x} from './foo';import {type y} from './foo';import {type z} from './foo';        ✓ import {type x} from './foo';import {type y} from './foo';import A from './foo';        ✓ import {type x} from './foo';import {type y} from './foo';import A, { C } from './foo';        ✓ import {AValue, type x, BValue} from './foo';import {type y, CValue} from './foo';        ✓ import AValue from './foo'; import {type y} from './foo';        ✓ import {type y} from './foo';import AValue from './foo';        ✓ import AValue, {BValue} from './foo'; import {type y, CValue} from './foo';        ✓ import {AValue, type x, BValue} from './foo'; import type {y} from './foo';        ✓ import {AValue, type x, BValue} from './foo'; import type {y} from './foo'        ✓ import AValue, {type x, BValue} from './foo';import type {y} from './foo';        ✓ import AValue, {type x, BValue} from './foo'; import type {y} from './foo'        ✓ import { type C, } from './foo';import {AValue, BValue, } from './foo';  TypeScript    no-duplicates      valid        ✓ import type { x } from './foo'; import y from './foo'        ✓ import type x from './foo'; import type y from './bar'        ✓ import type {x} from './foo'; import type {y} from './bar'        ✓ import type x from './foo'; import type {y} from './foo'        ✓       import type {} from './module';      import {} from './module2';            ✓       import type { Identifier } from 'module';      declare module 'module2' {        import type { Identifier } from 'module';      }      declare module 'module3' {        import type { Identifier } from 'module';      }            ✓ import { type x } from './foo'; import type y from 'foo'      invalid        ✓ import { type x } from './foo'; import y from './foo';        ✓ import type x from './foo'; import type y from './foo'        ✓ import type x from './foo'; import type x from './foo'        ✓ import type {x} from './foo'; import type {y} from './foo'        ✓ import {type x} from './foo'; import {y} from './foo'        ✓ import type {x} from './foo'; import {type y} from './foo'        ✓ import type {x} from './foo';import {type y} from './foo';        ✓ import {type x} from './foo'; import type {y} from './foo';        ✓ import {type x} from 'foo'; import type {y} from 'foo'        ✓ import {type x, C} from './foo'; import type {y} from './foo';        ✓ import type {y} from './foo'; import {type x, C} from './foo';        ✓ import {type x, C} from 'foo';import type {y} from 'foo';        ✓ import type {y} from './foo'; import {type x, C} from './foo';        ✓ import {type x} from './foo';import {type y} from './foo';        ✓ import {type x} from './foo';import {type y} from './foo';        ✓ import {type x} from './foo';import {type y} from './foo';import {type z} from './foo';        ✓ import {type x} from './foo';import {type y} from './foo';import A from './foo';        ✓ import {type x} from './foo';import {type y} from './foo';import A, { C } from './foo';        ✓ import {AValue, type x, BValue} from './foo';import {type y, CValue} from './foo';        ✓ import AValue from './foo'; import {type y} from './foo';        ✓ import {type y} from './foo';import AValue from './foo';        ✓ import AValue, {BValue} from './foo'; import {type y, CValue} from './foo';        ✓ import {AValue, type x, BValue} from './foo'; import type {y} from './foo';        ✓ import {AValue, type x, BValue} from './foo'; import type {y} from './foo'        ✓ import AValue, {type x, BValue} from './foo';import type {y} from './foo';        ✓ import AValue, {type x, BValue} from './foo'; import type {y} from './foo'        ✓ import { type C, } from './foo';import {AValue, BValue, } from './foo';123 passing (560ms)

Copy link

niklasnatter commentedApr 11, 2023

Hey, it would be great to handle type imports withno-duplicates - Is there anything that I could help with?

Copy link

mrm007 commentedMay 23, 2023

Is there anything else we can help with to get this merged? It's a great improvement over the current behaviour.

Copy link

seanCodes commentedJun 29, 2023

Also eager to see this merged and happy to help where needed!

Copy link

oceandrama commentedJul 14, 2023•
edited
Loading

@snewcomer Hi! Sorry for mention, but this PR is super helpfull and it will be great to see this merged! Are there any additional things you would like to work on?
Maybe it also fixes#2792

Copy link

niklasnatter commentedJul 27, 2023

I havent tested yet, but this might be fixed by#2835

Copy link

Member

ljharb commentedJul 27, 2023

I've rebased this PR, but tests seem to be failing.@mrm007 sorry i hadn't looked at your PR-PR yet - would you mind rebasing yours, and then I can pull it in?

snewcomer force-pushed thesn/fix-no-duplicates branch froma987e22 tobfdb9d0Compare

July 28, 2023 15:09

Copy link

Member

ljharb commentedJul 28, 2023

@snewcomer is there a reason you overwrote my rebase?

snewcomerand others added10 commits

August 6, 2023 13:42