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

Add option to disable TS import/export elision #791

Closed
alangpierce opened this issue Apr 1, 2023 · 0 comments · Fixed by #811
Closed

Add option to disable TS import/export elision #791

alangpierce opened this issue Apr 1, 2023 · 0 comments · Fixed by #811

Comments

@alangpierce
Copy link
Owner

alangpierce commented Apr 1, 2023

Follow-up from #783

Background

TypeScript 5.0 released a new option --verbatimModuleSyntax, and this issue tracks the implementation of some kind of equivalent option. The new TS option is described here: https://devblogs.microsoft.com/typescript/announcing-typescript-5-0/#verbatimmodulesyntax .

The option actually has two behaviors:

  1. Disable all automatic import/export elision (removal of type-only or unused imports and exports), and instead require explicit type syntax.
  2. Disallow all ESM -> CJS conversion.

I'm glad to see behavior 1 as a replacement for the now-deprecated --importsNotUsedAsValues and --preserveValueImports, since it has much simpler behavior and fully avoids the need for Sucrase to track variables and scopes.

I have mixed feelings about behavior 2, and I'm not sure it makes sense for Sucrase to bundle the two behaviors together the way TS does. The TS behavior seems to be entirely error-checking, not affecting emit, so one interpretation is that Sucrase should simply ignore behavior 2 since Sucrase leaves error checking to other tools. Another interpretation is that Sucrase should disallow setting this flag and the imports transform at the same time, or have the flag automatically disable the imports transform, but both of those seem a bit strange and arbitrary as opposed to the two options being independently configurable (which should be easy to implement). The codebase I work with at work is an example of a situation where ESM -> CJS transformation is still needed and it would be nice to avoid import/export elision for better understandability.

That said, I am happy to see a potential future path where .cts files no longer attempt to simulate ESM in CJS, which would mean that Sucrase would be able to eventually fully sunset the imports transform even if CJS has some occasional legacy uses.

Details

The plan for this task is to create a new flag that disables all import/export elision from the typescript transform. This was actually previously proposed with a PR in #615 , so that will be a good starting point. All normal ESM syntax will need a type keyword to remove the whole statement or parts of it. When used with the imports transform, the type syntax will determine whether to remove the require call.

Since the behavior is a little different from TS verbatimModuleSyntax (since there's no behavior 2 mentioned above), it's probably best to give it a different name. The decision on name can come later, but some options that come to mind:

  • keepUnusedImports (used in the linked PR)
  • preserveTSImports
  • disableImportExportElision
alangpierce added a commit that referenced this issue Jul 14, 2023
Supersedes #615
Fixes #791

This PR builds off of #615 with a number of additional tests and cases.
High-level, it introduces a new option `keepUnusedImports` that disables all
automatic import and export elision done by TypeScript, somewhat analogous to
the `verbatimModuleSyntax` TS option.
alangpierce added a commit that referenced this issue Jul 14, 2023
Supersedes #615
Fixes #791

This PR builds off of #615 with a number of additional tests and cases. High-level, it introduces a new option `keepUnusedImports` that disables all automatic import and export elision done by TypeScript, somewhat analogous to the `verbatimModuleSyntax` TS option. For the most part, this is just a matter of flagging off the name tracking and elision code
for both imports and exports in the various places that they happen.

When this mode is enabled, some of the preprocessing steps can be skipped, and scope handling can be fully skipped with
the imports transform also disabled. Performance numbers measured by `yarn benchmark jest-dev`:
* With `imports` and `typescript` transforms: 855k -> 910k lines per second (6% faster)
* With only `typescript` transform: 935k -> 1110k lines per second (19% faster)

Some additional details:
* While this feature is intended to be used with TypeScript, I decided to also implement the behavior for Flow as well. Flow's implementation in Babel will remove an import statement if all individual names are removed, but `flow-remove-types` won't do this, so Sucrase `keepUnusedImports` matches the behavior of `flow-remove-types`.
* Babel doesn't currently implement this option, so for the playground I mapped it to `onlyRemoveTypeImports`, which
  seems to have the closest behavior.
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 a pull request may close this issue.

1 participant