-
Notifications
You must be signed in to change notification settings - Fork 19
feat(typescript): Update type import specifier rules #381
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
Conversation
3436daf to
9205cac
Compare
|
This will be followed by a bump of the eslint config packages on the module template, which also needs various other updates. |
|
I definitely like the idea of being consistent about type imports. I just want to make sure that
Is this something we need to be concerned with? |
|
There is also this bit in the ESM/CJS interoperability page in the docs:
and then at the bottom, when talking about caveats around
So, based on this page, it sounds like |
mcmire
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I re-read over the TypeScript documentation that I quoted and I think this is a good idea.
|
@mcmire awesome! This does work just fine with |
Reverts #381 temporarily because it includes breaking changes, and we'd like to release the non-breaking changes on `main` without a major version bump. This will be restored after release.
#418) Reverts #407, which itself was a revert of #381 because it had breaking changes, and we wanted to release some non-breaking things first. This PR restores the breaking changes in #381. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Replace @typescript-eslint/consistent-type-imports with import-x/consistent-type-specifier-style (prefer-top-level) and enable TypeScript verbatimModuleSyntax in tsconfigs. > > - **TypeScript ESLint config (`packages/typescript/src/index.mjs`, `rules-snapshot.json`)**: > - Add `import-x/consistent-type-specifier-style: ["error", "prefer-top-level"]` and keep `import-x/no-unresolved: "off"`. > - Remove `@typescript-eslint/consistent-type-imports`. > - **TypeScript config (`tsconfig.json`, `packages/typescript/tsconfig.json`)**: > - Enable `compilerOptions.verbatimModuleSyntax: true`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 245508b. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
Ref: MetaMask/ocap-kernel#175
This replaces
@typescript-eslint/consistent-type-importswithimport-x/consistent-type-specifier-style, and an exhortation to enableverbatimModuleSyntax(requires TypeScript >=5) in the tsconfigs of our packages.Heretofore, we have enforced the existence of type import specifiers, but not that they are stylistically consistent. The options are:
import type { a } from 'x'}import { type a } from 'x'}The rule
@typescript-eslint/consistent-type-importshas been responsible for this. This rule has an option namedfixTypes, but that only applies when a type-only import is not specified as a type-only import. In other words, it enforces the existence of type import specifiers, but not that they're stylistically consistent.Rather than using
@typescript-eslint/consistent-type-importsand its confusingly named options, we are better off enablingverbatimModuleSyntaxin ourtsconfigfiles, and enablingimport-x/consistent-type-specifier-style. In this way, the existence of type import specifiers is enforced bytsc, and theimport-xESLint plugins ensures that they are consistent. We use the "top-level" style because that results inimport type { a } from 'x';being elided completely from the JS output as opposed toimport { type a } from 'x';which would be emitted asimport {} from 'x';. I don't know if that's actually harmful, but it seems surprising, and I don't like surprises.Supporting documentation:
@typescript-eslint/consistent-type-importsrule documentationverbatimModuleSyntaxdocs