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

chore: enforce types imports #237

Merged
merged 10 commits into from
Oct 11, 2023
Merged

Conversation

Yovach
Copy link
Contributor

@Yovach Yovach commented Oct 11, 2023

This PR, however small, adds the keyword 'type' to imports where possible.

I think it would be interesting to enable verbatimModuleSyntax as well.

@vercel
Copy link

vercel bot commented Oct 11, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
next-international ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 11, 2023 0:35am

Copy link
Owner

@QuiiBz QuiiBz left a comment

Choose a reason for hiding this comment

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

Thanks! I think we can use this ESLint rule to enforce it properly: https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/docs/rules/consistent-type-imports.md

I personally prefer the separate-type-imports fixStyle.

@Yovach
Copy link
Contributor Author

Yovach commented Oct 11, 2023

I added the separate-type-imports rule to eslint config and tried to add a rule to enforce the top-level type but you have to install a new dependency (https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/consistent-type-specifier-style.md#prefer-top-level) :/

.eslintrc Outdated Show resolved Hide resolved
@QuiiBz
Copy link
Owner

QuiiBz commented Oct 11, 2023

tried to add a rule to enforce the top-level type but you have to install a new dependency (import-js/eslint-plugin-import@main/docs/rules/consistent-type-specifier-style.md#prefer-top-level) :/

This rule seems redundant with @typescript-eslint/consistent-type-imports, I'm not sure to understand the difference between the two?

@QuiiBz QuiiBz changed the title Enforce types imports chore: enforce types imports Oct 11, 2023
@Yovach
Copy link
Contributor Author

Yovach commented Oct 11, 2023

tried to add a rule to enforce the top-level type but you have to install a new dependency (import-js/eslint-plugin-import@main/docs/rules/consistent-type-specifier-style.md#prefer-top-level) :/

This rule seems redundant with @typescript-eslint/consistent-type-imports, I'm not sure to understand the difference between the two?

The @typescript-eslint/consistent-type-imports allows us to use

import { type ReactElement } from 'react';

while it shouldn't.

The import/consistent-type-specifier-style prevents this

@QuiiBz
Copy link
Owner

QuiiBz commented Oct 11, 2023

Thanks for the explanation, it's a lot clearer. I think we can go ahed and add this new eslint-plugin-import package along with this other rule.

@Yovach
Copy link
Contributor Author

Yovach commented Oct 11, 2023

Issues will be fixed with --fix thanks to import/consistent-type-specifier-style

Copy link
Owner

@QuiiBz QuiiBz left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@QuiiBz QuiiBz merged commit 1c15986 into QuiiBz:main Oct 11, 2023
6 checks passed
@Yovach Yovach deleted the ref/enforce-types-imports branch October 12, 2023 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants