Repository: alangpierce/sucrase (ts)

Issue:
TypeScript type-only re-exports not always properly elided
[Playground link](https://sucrase.io/#transforms=jsx%2Ctypescript&compareWithTypeScript=true&compressedCode=H4sIALd%2BpGQAA%2BNKrSjILypRqHasVUgrys9VUM%2FNTynNSTVUt%2BaCSpVUFqQqVDuhyhsh5KvBCpxR5Y2R5FFlTJBkXHQgpruiKjFVtwYAx32muZgAAAA%3D)

Example code with different re-export cases:
```ts
export {A} from 'module1';
export type {B} from 'module2';
export {type C} from 'module3';
export {} from 'module4';
export {D, type E} from 'module5';
```
With the TypeScript transform enabled, the expected behavior is to fully elide modules 2, 3, and 4, so that they aren't imported, but Sucrase currently only elides module 2. (Babel keeps module 4 but tsc elides it, and I'm considering TypeScript's implementation to be the authoritative one.)

With the Flow transform enabled, only modules 1, 2, and 4 are valid syntax, and Sucrase correctly only elides module 2. With no type transforms enabled, only modules 1 and 4 are valid syntax, and Sucrase correctly doesn't elide either of them.
TypeScript automatic named export elision incorrectly happens for re-export
Code snippet ([playground link](https://sucrase.io/#transforms=jsx%2Ctypescript&code=type%20T%20%3D%20number%3B%0Aexport%20%7BT%7D%20from%20%22foo%22%3B%0A))
```ts
type T = number;
export {T} from "foo";
```
Expected behavior is that the `T` on the second line is a re-exported and should be preserved. Instead, Sucrase mistakenly thinks that the `T` is a type-only export and elides it. For this sort of usage-based automatic export elision, Sucrase needs to distinguish `export {...}` and `export {...} from ...` statements and only do automatic elision in the first case.

Pull request:
Properly elide type-only re-export statements when targeting ESM
Fixes #803
Fixes #805

When handling TypeScript, fix two issues involving statements starting with `export {`:
* Re-exports need to remove the module import if all exported names are explicitly elided.
* Re-exports should NOT do the implicit export elision that's needed for normal multi-exports.

Patch:
diff --git a/src/transformers/CJSImportTransformer.ts b/src/transformers/CJSImportTransformer.ts
index 9972da6..db62ad6 100644
--- a/src/transformers/CJSImportTransformer.ts
+++ b/src/transformers/CJSImportTransformer.ts
@@ -790,7 +790,7 @@ export default class CJSImportTransformer extends Transformer {
     this.tokens.removeInitialToken();
     this.tokens.removeToken();
 
-    const exportStatements = [];
+    const exportStatements: Array<string> = [];
     while (true) {
       if (this.tokens.matches1(tt.braceR)) {
         this.tokens.removeToken();
diff --git a/src/transformers/ESMImportTransformer.ts b/src/transformers/ESMImportTransformer.ts
index 41b814e..1768366 100644
--- a/src/transformers/ESMImportTransformer.ts
+++ b/src/transformers/ESMImportTransformer.ts
@@ -314,8 +314,14 @@ export default class ESMImportTransformer extends Transformer {
   }
 
   /**
-   * In TypeScript, we need to remove named exports that were never declared or only declared as a
-   * type.
+   * Handle a statement with one of these forms:
+   * export {a, type b};
+   * export {c, type d} from 'foo';
+   *
+   * In both cases, any explicit type exports should be removed. In the first
+   * case, we also need to handle implicit export elision for names declared as
+   * types. In the second case, we must NOT do implicit named export elision,
+   * but we must remove the runtime import if all exports are type exports.
    */
   private processNamedExports(): boolean {
     if (!this.isTypeScriptTransformEnabled) {
@@ -324,9 +330,14 @@ export default class ESMImportTransformer extends Transformer {
     this.tokens.copyExpectedToken(tt._export);
     this.tokens.copyExpectedToken(tt.braceL);
 
+    const isReExport = this.isReExport();
+    let foundNonTypeExport = false;
     while (!this.tokens.matches1(tt.braceR)) {
       const specifierInfo = getImportExportSpecifierInfo(this.tokens);
-      if (specifierInfo.isType || this.shouldElideExportedName(specifierInfo.leftName)) {
+      if (
+        specifierInfo.isType ||
+        (!isReExport && this.shouldElideExportedName(specifierInfo.leftName))
+      ) {
         // Type export, so remove all tokens, including any comma.
         while (this.tokens.currentIndex() < specifierInfo.endIndex) {
           this.tokens.removeToken();
@@ -336,6 +347,7 @@ export default class ESMImportTransformer extends Transformer {
         }
       } else {
         // Non-type export, so copy all tokens, including any comma.
+        foundNonTypeExport = true;
         while (this.tokens.currentIndex() < specifierInfo.endIndex) {
           this.tokens.copyToken();
         }
@@ -345,9 +357,33 @@ export default class ESMImportTransformer extends Transformer {
       }
     }
     this.tokens.copyExpectedToken(tt.braceR);
+
+    if (isReExport && !foundNonTypeExport) {
+      // This is a type-only re-export, so skip evaluating the other module. Technically this
+      // leaves the statement as `export {}`, but that's ok since that's a no-op.
+      this.tokens.removeToken();
+      this.tokens.removeToken();
+      removeMaybeImportAttributes(this.tokens);
+    }
+
     return true;
   }
 
+  /**
+   * Starting at `export {`, look ahead and return `true` if this is an
+   * `export {...} from` statement and `false` if this is a plain multi-export.
+   */
+  private isReExport(): boolean {
+    let closeBraceIndex = this.tokens.currentIndex();
+    while (!this.tokens.matches1AtIndex(closeBraceIndex, tt.braceR)) {
+      closeBraceIndex++;
+    }
+    return (
+      this.tokens.matchesContextualAtIndex(closeBraceIndex + 1, ContextualKeyword._from) &&
+      this.tokens.matches1AtIndex(closeBraceIndex + 2, tt.string)
+    );
+  }
+
   /**
    * ESM elides all imports with the rule that we only elide if we see that it's
    * a type and never see it as a value. This is in contrast to CJS, which


Test patch:
diff --git a/test/typescript-test.ts b/test/typescript-test.ts
index 8c7447c..144f12e 100644
--- a/test/typescript-test.ts
+++ b/test/typescript-test.ts
@@ -3736,4 +3736,51 @@ describe("typescript transform", () => {
       {transforms: ["typescript"]},
     );
   });
+
+  it("fully elides type-only re-exports", () => {
+    assertTypeScriptImportResult(
+      `
+      export {A} from 'module1';
+      export type {B} from 'module2';
+      export {type C} from 'module3';
+      export {} from 'module4';
+      export {D, type E} from 'module5';
+    `,
+      {
+        expectedCJSResult: `"use strict";${ESMODULE_PREFIX}${CREATE_NAMED_EXPORT_FROM_PREFIX}
+      var _module1 = require('module1'); _createNamedExportFrom(_module1, 'A', 'A');
+      ;
+      
+      
+      var _module5 = require('module5'); _createNamedExportFrom(_module5, 'D', 'D');
+    `,
+        expectedESMResult: `
+      export {A} from 'module1';
+      ;
+      export {};
+      export {};
+      export {D,} from 'module5';
+    `,
+      },
+    );
+  });
+
+  it("does not elide re-exports of names that happen to be type names", () => {
+    assertTypeScriptImportResult(
+      `
+      type T = number;
+      export {T} from 'module1';
+    `,
+      {
+        expectedCJSResult: `"use strict";${ESMODULE_PREFIX}${CREATE_NAMED_EXPORT_FROM_PREFIX}
+      
+      var _module1 = require('module1'); _createNamedExportFrom(_module1, 'T', 'T');
+    `,
+        expectedESMResult: `
+      
+      export {T} from 'module1';
+    `,
+      },
+    );
+  });
 });