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 false positive and autofix creates a syntax error #2114

Closed
NickHeiner opened this issue Jun 3, 2021 · 12 comments · Fixed by #2149
Closed

no-duplicates false positive and autofix creates a syntax error #2114

NickHeiner opened this issue Jun 3, 2021 · 12 comments · Fixed by #2149

Comments

@NickHeiner
Copy link

NickHeiner commented Jun 3, 2021

From:

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

To:

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

The proper autocorrect is:

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

On version 2.20.0 (edit: and 2.23.4)

Another bug:

From:

import type Registration from './Registration';
import type { RouteResolver } from './Registration';

To:

import type Registration, { RouteResolver } from './Registration';

This gives TS error:

A type-only import can specify a default import or named bindings, but not both.ts(1363)
@ljharb
Copy link
Member

ljharb commented Jun 3, 2021

@NickHeiner that's an old version, can you try on the latest?

@NickHeiner
Copy link
Author

Ha! Blazing fast response. I literally was doing that. 😄

@ljharb
Copy link
Member

ljharb commented Jun 3, 2021

Thanks, that's definitely a bug.

Although I assume your OP example code is partially edited - can you provide the exact code that's failing, no "foo" or "bar"?

@NickHeiner
Copy link
Author

🤦 lol, if I was going to edit the input, I should have edited the output as well.

I've updated the OP to be unedited. Sorry for the churn.

@NickHeiner NickHeiner changed the title no-duplicates autofix creates a syntax error no-duplicates false positive and autofix creates a syntax error Jun 3, 2021
@ljharb
Copy link
Member

ljharb commented Jun 3, 2021

looks like in TypeScript, import type has to keep default and named separate. I'm not sure if Flow has the same limitation.

@ljharb ljharb added the flow label Jun 3, 2021
@NickHeiner
Copy link
Author

Yes, I think an import type and import statement need to be separate.

@ljharb
Copy link
Member

ljharb commented Jun 3, 2021

@NickHeiner yes but in this case it's two import type statements that can't be combined, despite the equivalent import statements being combineable.

@NickHeiner
Copy link
Author

Right. Also, I found what the right edit should be, and updated the OP to reflect it.

@ljharb
Copy link
Member

ljharb commented Jun 3, 2021

While { default as works, I do not think that's the proper autocorrect - i would never want to have default as in my codebase. I think those should just be left as two separate statements.

@NickHeiner
Copy link
Author

Heh, fair. I'm happy to have default as rather than eslint-disable import/no-duplicates for now. 😄 So maybe this is something that should be behind an option.

@ljharb
Copy link
Member

ljharb commented Jun 3, 2021

I think you can manually fix it to that - the lint rule wouldn't complain - so no option would be needed. eslint rules don't tend to have options that solely govern autofix behavior.

@GoodForOneFare
Copy link
Contributor

If someone has time for review, #2149 should fix this. It's not exactly the solution proposed here, but is hopefully good enough 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants