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

build: enable consistent type imports rule #24

Closed
wants to merge 2 commits into from

Conversation

ad1992
Copy link

@ad1992 ad1992 commented May 18, 2024

Enabled the consistent-type-imports so its clear from imports whether type is being imported.

I have auto fixed the codebase, however I am happy to revert if that is unnecessary and change the rule to throw warning instead.

cc @cpojer

@cpojer
Copy link
Contributor

cpojer commented May 18, 2024

Thanks for the contribution! I looked at this rule previously but chose not to enable it. Here is what I landed on before:

  • Importing a JavaScript object only by type, and then using that object in the same file requires removing the type from import which seems inconvenient. There is an autofix, but it still makes things slower or requires me to stop and go back.
  • I don't really make a difference whether I only import a type or not as almost all downsides from importing the real thing are mitigated in this codebase. There are no side effects in the code base, all code is more-or-less required anyway, and rollup+esbuild are strong at removing unused imports.
  • One import statement should be for one file. Having possibly two for each doesn't seem easy to browse (This can be changed via the plugin's settings, I assume?).

So in the end, what is the upside? Is it only a minor change in how modules are bundled, or is there something big I'm missing that makes this behavior better than the current situation?

@cpojer cpojer force-pushed the main branch 3 times, most recently from e7d0373 to e7114fa Compare May 21, 2024 22:39
@cpojer
Copy link
Contributor

cpojer commented May 26, 2024

I asked on Twitter: https://x.com/cpojer/status/1794462636219801852

Then a GitHub issue was surfaced in which a TS team member said:

The intention is for you not to use import type unless you need to.

Let's not move forward with this.

@cpojer cpojer closed this May 26, 2024
@ad1992
Copy link
Author

ad1992 commented May 27, 2024

Hi @cpojer sorry I was busy so couldn't back on this one.
This is just a user preference and has nothing to do with bundling as the typings will be removed irrsepective of how its imported.
Hence I agree with your reasonings and since its not needed in the codebase so better to close it.

@ad1992 ad1992 deleted the aakansha/consistent-type-import branch May 27, 2024 18:20
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.

2 participants