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(repo): enable consistent-type-imports for typescript files #1325

Merged
merged 3 commits into from
Nov 27, 2022

Conversation

jerry-lllman
Copy link
Contributor

@jerry-lllman jerry-lllman commented Oct 19, 2022

Rollup Plugin Name: {name}

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes (bugfixes and features will not be merged without tests)
  • no

Breaking Changes?

  • yes (breaking changes will not be merged unless absolutely necessary)
  • no

If yes, then include "BREAKING CHANGES:" in the first commit message body, followed by a description of what is breaking.

List any relevant issue numbers:

Description

we added eslint rule '@typescript-eslint/consistent-type-imports': 'error'
It can make some optimizations.
For example, if you import typescript, the whole typescript library will be imported, but the import type typescript this library will not be imported, saving a lot of volume.

typescript documentation is described as follows
https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-8.html#type-only-imports-and-export

Copy link
Member

@lukastaegert lukastaegert left a comment

Choose a reason for hiding this comment

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

Thank you. IMO though, doing this manually is an unmaintainable chore. If we want this, we should throw

'@typescript-eslint/consistent-type-imports': 'error'

into the ESLint config. This rule has an auto-fixer.

@jerry-lllman
Copy link
Contributor Author

ok, i add it, and changed the code that does not conform to this rule.

btw, my first commit is not manually, it's fixed by this rule~

@lukastaegert
Copy link
Member

btw, my first commit is not manually, it's fixed by this rule~

😅 Makes sense

@shellscape shellscape force-pushed the master branch 4 times, most recently from b353836 to 3038271 Compare October 21, 2022 19:01
@shellscape
Copy link
Collaborator

@jerry-lllman Please fill in the Description of the pull request template. I'm familiar with the rule and it's intent and purpose, but why do we need to enforce this? Please make your case in the Description.

@shellscape shellscape changed the title refactor: distinguish between types of import chore(repo): use eslint to force distinguishing between types of import Oct 24, 2022
@jerry-lllman
Copy link
Contributor Author

@jerry-lllman Please fill in the Description of the pull request template. I'm familiar with the rule and it's intent and purpose, but why do we need to enforce this? Please make your case in the Description.

@shellscape I have added the description🥳

@shellscape shellscape changed the title chore(repo): use eslint to force distinguishing between types of import chore(repo): enable consistent-type-imports for typescript files Nov 27, 2022
@shellscape shellscape merged commit a2e582a into rollup:master Nov 27, 2022
@shellscape
Copy link
Collaborator

thanks!

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.

3 participants