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

fail positives with "no-named-as-default" #128

Open
Mister-Hope opened this issue Aug 15, 2024 · 8 comments
Open

fail positives with "no-named-as-default" #128

Mister-Hope opened this issue Aug 15, 2024 · 8 comments
Labels
help wanted Extra attention is needed question Further information is requested

Comments

@Mister-Hope
Copy link

image

Any ideas why this is reported?

@JounQin
Copy link
Member

JounQin commented Aug 15, 2024

It's said you should use import { HeroInfo } instead

@JounQin JounQin closed this as not planned Won't fix, can't repro, duplicate, stale Aug 15, 2024
@Mister-Hope
Copy link
Author

Mister-Hope commented Aug 15, 2024

image

Stop closing my issue, I am not a noob as I am in charge for a few 2k+ star projects. I already told you I found a fail positive.

This is the generated dts file from npm:

image

So why should I use import { HeroInfo }

Edited: Sorry for my attitude with the first edition, I am at covid with fever, so in a bad mood. Please reopen this, if you need a reproduction I can try to provide one.

@JounQin
Copy link
Member

JounQin commented Aug 15, 2024

Stop closing my issue, I am not a noob as I am in charge for a few 2k+ star projects. I already told you I found a fail positive.

You didn't provide any useful info in your initial issue.


It seems like a limitation, HeroInfo is already declared as an interface, so importing it as default would make HeroInfo interface unavailable any longer. The property usage should use another name instead of HeroInfo for the default importing identifier.

@JounQin JounQin added the question Further information is requested label Aug 15, 2024
@Mister-Hope
Copy link
Author

Thanks for the reply, BTW I updated my second reply, just a remind in case you miss it.

@SukkaW
Copy link
Collaborator

SukkaW commented Aug 26, 2024

It seems like a limitation, HeroInfo is already declared as an interface, so importing it as default would make the HeroInfo interface unavailable any longer. The property usage should use another name instead of HeroInfo for the default importing identifier.

eslint-plugin-import-x treats type export and value export identically when tracking modules. While it's possible to distinguish whether an export is type-only through AST, it would be quite challenging.

It's undoubtedly bad practice to have a named type-only export and a default value-only export share the same name. I don't know whether we should address this issue or not. If someone wants to open a PR addressing this, I would be more than happy to review and merge it.

@SukkaW SukkaW reopened this Aug 26, 2024
@SukkaW SukkaW added the help wanted Extra attention is needed label Aug 26, 2024
@meteorlxy
Copy link

IMO, to avoid naming the default export as the same name of a type is also the purpose of the rule. So I'd treat it as a valid warning.

@SukkaW
Copy link
Collaborator

SukkaW commented Aug 28, 2024

IMO, to avoid naming the default export as the same name of a type is also the purpose of the rule. So I'd treat it as a valid warning.

But what about third-party libraries? I don't want to make this rule ignore all third-party libraries.

@AndreaPontrandolfo
Copy link

Related #136

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

No branches or pull requests

5 participants