Repository: alangpierce/sucrase
Pull request #723 (2022-07-14 19:49:40)

Fix invalid output code when eliding TS star imports
From some manual testing, I noticed that the code:
```ts
import A, * as B from "./A";
console.log(A);
```
becomes this invalid code:
```ts
import A, from "./A";
console.log(A);
```
The `* as B` was being elided because it's treated as a type-only import, but in
this case we need to elide the comma as well since there's no concept of
"trailing commas" here. The code to do that is a little clunky, but I ended up
always removing the comma and using a boolean to track whether it should be
added back.

codecov[bot]: # [Codecov](https://codecov.io/gh/alangpierce/sucrase/pull/723?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Alan+Pierce) Report
> Merging [#723](https://codecov.io/gh/alangpierce/sucrase/pull/723?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Alan+Pierce) (98ee525) into [main](https://codecov.io/gh/alangpierce/sucrase/commit/66da4d1ebafb948f54cc136d30cf9b47293da8d9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Alan+Pierce) (66da4d1) will **increase** coverage by `0.10%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##             main     #723      +/-   ##
==========================================
+ Coverage   87.05%   87.15%   +0.10%     
==========================================
  Files          54       54              
  Lines        5702     5708       +6     
  Branches     1343     1345       +2     
==========================================
+ Hits         4964     4975      +11     
+ Misses        470      466       -4     
+ Partials      268      267       -1     
```


| [Impacted Files](https://codecov.io/gh/alangpierce/sucrase/pull/723?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Alan+Pierce) | Coverage Δ | |
|---|---|---|
| [src/transformers/ESMImportTransformer.ts](https://codecov.io/gh/alangpierce/sucrase/pull/723/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Alan+Pierce#diff-c3JjL3RyYW5zZm9ybWVycy9FU01JbXBvcnRUcmFuc2Zvcm1lci50cw==) | `93.87% <100.00%> (+3.80%)` | :arrow_up: |

:mega: Codecov can now indicate which changes are the most critical in Pull Requests. [Learn more](https://about.codecov.io/product/feature/runtime-insights/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Alan+Pierce)

github-actions[bot]: ## Benchmark results
**Before this PR:** 345.1 thousand lines per second
**After this PR:**  345.2 thousand lines per second

**Measured change:** 0.04% faster (0.74% slower to 0.21% faster)
**Summary:** Likely no significant difference

Patch:
diff --git a/src/transformers/ESMImportTransformer.ts b/src/transformers/ESMImportTransformer.ts
index 87f9123..2a2c394 100644
--- a/src/transformers/ESMImportTransformer.ts
+++ b/src/transformers/ESMImportTransformer.ts
@@ -161,6 +161,7 @@ export default class ESMImportTransformer extends Transformer {
     }
 
     let foundNonTypeImport = false;
+    let needsComma = false;
 
     if (this.tokens.matches1(tt.name)) {
       if (this.isTypeName(this.tokens.identifierName())) {
@@ -172,23 +173,38 @@ export default class ESMImportTransformer extends Transformer {
         foundNonTypeImport = true;
         this.tokens.copyToken();
         if (this.tokens.matches1(tt.comma)) {
-          this.tokens.copyToken();
+          // We're in a statement like:
+          // import A, * as B from './A';
+          // or
+          // import A, {foo} from './A';
+          // where the `A` is being kept. The comma should be removed if an only
+          // if the next part of the import statement is elided, but that's hard
+          // to determine at this point in the code. Instead, always remove it
+          // and set a flag to add it back if necessary.
+          needsComma = true;
+          this.tokens.removeToken();
         }
       }
     }
 
     if (this.tokens.matches1(tt.star)) {
-      if (this.isTypeName(this.tokens.identifierNameAtIndex(this.tokens.currentIndex() + 2))) {
+      if (this.isTypeName(this.tokens.identifierNameAtRelativeIndex(2))) {
         this.tokens.removeToken();
         this.tokens.removeToken();
         this.tokens.removeToken();
       } else {
+        if (needsComma) {
+          this.tokens.appendCode(",");
+        }
         foundNonTypeImport = true;
         this.tokens.copyExpectedToken(tt.star);
         this.tokens.copyExpectedToken(tt.name);
         this.tokens.copyExpectedToken(tt.name);
       }
     } else if (this.tokens.matches1(tt.braceL)) {
+      if (needsComma) {
+        this.tokens.appendCode(",");
+      }
       this.tokens.copyToken();
       while (!this.tokens.matches1(tt.braceR)) {
         const specifierInfo = getImportExportSpecifierInfo(this.tokens);


Test patch:
diff --git a/test/typescript-test.ts b/test/typescript-test.ts
index d4f64d1..b05fd3b 100644
--- a/test/typescript-test.ts
+++ b/test/typescript-test.ts
@@ -1549,6 +1549,29 @@ describe("typescript transform", () => {
     );
   });
 
+  it("produces proper import statements when eliding ESM imported names", () => {
+    assertTypeScriptESMResult(
+      `
+      import keep1, * as elide1 from "./m1";
+      import keep2, * as keep3 from "./m2";
+      import elide2, * as keep4 from "./m3";
+      import keep5, {elide3} from "./m4";
+      import keep6, {keep7} from "./m5";
+      import elide4, {keep8} from "./m6";
+      console.log(keep1, keep2, keep3, keep4, keep5, keep6, keep7, keep8);
+    `,
+      `
+      import keep1 from "./m1";
+      import keep2, * as keep3 from "./m2";
+      import * as keep4 from "./m3";
+      import keep5, {} from "./m4";
+      import keep6, {keep7} from "./m5";
+      import {keep8} from "./m6";
+      console.log(keep1, keep2, keep3, keep4, keep5, keep6, keep7, keep8);
+    `,
+    );
+  });
+
   it("handles import() types", () => {
     assertTypeScriptESMResult(
       `