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

[Fix] no-duplicates: type import fixer generates valid TypeScript #2149

Merged
merged 1 commit into from
Aug 18, 2021

Conversation

GoodForOneFare
Copy link
Contributor

Fixes #2114, #2130

Note that #2114 suggests the correct fix looks like:

import type {default as dnajs, AccountQuery} from '@netflix-internal/dnajs';

but this PR will generate:

import type dnajs from '@netflix-internal/dnajs';
import type {AccountQuery} from '@netflix-internal/dnajs';

This avoids conflicts with no-named-default mentioned in #2130.

@ljharb
Copy link
Member

ljharb commented Jul 17, 2021

@GoodForOneFare are you no longer interested in landing this?

@nwalters512
Copy link
Contributor

@ljharb would you have otherwise accepted this PR? I'd be happy to open a new PR and see it through, we've been bitten by this before.

@ljharb
Copy link
Member

ljharb commented Aug 12, 2021

@nwalters512 please do not open a new PR - i'd prefer @GoodForOneFare to restore the branch and reopen this one, and i can take your branch and pull it into this PR.

I haven't had a chance to review this one, but any fixer that's safe is one I'm always anxious to add.

@nwalters512
Copy link
Contributor

Sure - how long would you like to wait for @GoodForOneFare to reopen? If they've abandoned it entirely, I would still very much like to get this fix landed.

@GoodForOneFare
Copy link
Contributor Author

Hi folks! I only had a short window in which I could contribute, so @nwalters512 can pick this up without stepping on my toes 😸

fwiw, even with this fix, enabling this + consistent-type-imports was too noisy for our largest repo 😞 If I was picking this back up, I'd get both rules to play nice by only mandating import type when there are no non-type imports for a dependency.

@ljharb
Copy link
Member

ljharb commented Aug 16, 2021

@GoodForOneFare any chance you could restore the branch and reopen the PR? That way @nwalters512 can post a link to a branch and I can pull in the commits, without needing an extra PR.

@GoodForOneFare GoodForOneFare restored the no-duplicate-types branch August 16, 2021 17:36
@GoodForOneFare
Copy link
Contributor Author

Done 👍 All yours, @nwalters512!

@ljharb
Copy link
Member

ljharb commented Aug 16, 2021

@nwalters512 please comment with a link to a rebased/updated branch, and i'll pull it in :-)

@ljharb ljharb marked this pull request as draft August 16, 2021 21:49
…ype imports

Co-authored-by: Nathan Walters <nwalters@nerdwallet.com>
Co-authored-by: Gord Pearson <gord.pearson@shopify.com>
@nwalters512
Copy link
Contributor

@ljharb here you go! https://github.com/nwalters512/eslint-plugin-import/tree/no-duplicates-types

@ljharb ljharb changed the title [Fix] no-duplicates: valid default + named types fixer [Fix] no-duplicates: type import fixer generates valid TypeScript Aug 18, 2021
@ljharb ljharb added bug and removed enhancement labels Aug 18, 2021
@ljharb ljharb marked this pull request as ready for review August 18, 2021 06:48
@ljharb ljharb merged commit 712ee49 into import-js:master Aug 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

no-duplicates false positive and autofix creates a syntax error
3 participants