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 keepUnusedImports option #811

Merged
merged 3 commits into from
Jul 14, 2023
Merged

Conversation

alangpierce
Copy link
Owner

@alangpierce alangpierce commented 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.

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.
@codecov
Copy link

codecov bot commented Jul 14, 2023

Codecov Report

Merging #811 (0b6e699) into main (a924d84) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #811      +/-   ##
==========================================
+ Coverage   88.68%   88.70%   +0.01%     
==========================================
  Files          56       56              
  Lines        6073     6082       +9     
  Branches     1444     1446       +2     
==========================================
+ Hits         5386     5395       +9     
  Misses        421      421              
  Partials      266      266              
Impacted Files Coverage Δ
src/Options-gen-types.ts 100.00% <ø> (ø)
src/Options.ts 100.00% <ø> (ø)
src/transformers/RootTransformer.ts 93.39% <ø> (ø)
src/CJSImportProcessor.ts 91.95% <100.00%> (+0.04%) ⬆️
src/index.ts 92.15% <100.00%> (ø)
src/transformers/CJSImportTransformer.ts 90.33% <100.00%> (+0.06%) ⬆️
src/transformers/ESMImportTransformer.ts 96.77% <100.00%> (+0.08%) ⬆️
src/util/shouldElideDefaultExport.ts 88.88% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@github-actions
Copy link

github-actions bot commented Jul 14, 2023

Benchmark results

Before this PR: 329.8 thousand lines per second
After this PR: 326 thousand lines per second

Measured change: 1.17% slower (2.1% slower to 0.26% slower)
Summary: Probably slower

@alangpierce alangpierce merged commit ceeb203 into main Jul 14, 2023
8 checks passed
@alangpierce alangpierce deleted the add-keep-unused-imports-option branch July 14, 2023 21:46
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.

Add option to disable TS import/export elision
1 participant