Elide import = statements that are only used as a type #441
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes #422 In both CJS and ESM, we now look at the identifier in `import A =` and decide whether to elide the entire statement in a similar way to normal TS import elision. Since there are only two forms that these statements can have, I just special cased the two when deleting statements. Technically to implement this totally correctly, I'd need something like a graph traversal: in an `import A = B.C;` statement, `B` is referenced as a value if `A` is ever referenced as a value, so deciding if an import should be elided requires following an arbitrary number of import statements. In practice, I think this syntax is only ever used to pull in types (for values, you can just use `const`, and the syntax is obscure anyway), so I treat it as a type statement that gets elided.
Codecov Report
@@ Coverage Diff @@
## master #441 +/- ##
==========================================
+ Coverage 81.16% 81.28% +0.12%
==========================================
Files 49 50 +1
Lines 5367 5392 +25
Branches 1200 1204 +4
==========================================
+ Hits 4356 4383 +27
+ Misses 712 711 -1
+ Partials 299 298 -1
Continue to review full report at Codecov.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #422
In both CJS and ESM, we now look at the identifier in
import A =and decidewhether to elide the entire statement in a similar way to normal TS import
elision. Since there are only two forms that these statements can have, I just
special cased the two when deleting statements.
Technically to implement this totally correctly, I'd need something like a graph
traversal: in an
import A = B.C;statement,Bis referenced as a value ifAis ever referenced as a value, so deciding if an import should be elidedrequires following an arbitrary number of import statements. In practice, I
think this syntax is only ever used to pull in types (for values, you can just
use
const, and the syntax is obscure anyway), so I treat it as a typestatement that gets elided.