From 80f84a665463e12fb65d8b6c7bc8df4a8ea71c6b Mon Sep 17 00:00:00 2001
From: Alan Pierce <alan@benchling.com>
Date: Wed, 2 Dec 2020 01:21:14 -0800
Subject: [PATCH] Fix identifier role for top-level destructure declarations
 (#564)

Fixes #563
Fixes #473

The type-only export elision implemented in #433 had a false positive for
destructure delcarations, since `getDeclarationInfo` only looks at top-level
declarations and top-level expressions like `const {x} = ...` weren't being
marked correctly. To fix, we can mark object shorthand declarations as top-level
when applicable. I also refactored both relevant code paths to avoid the
repeated assignment left-hand side.
---
 src/parser/traverser/expression.ts | 14 ++++++++++----
 src/parser/traverser/lval.ts       | 10 ++++++----
 test/typescript-test.ts            | 28 ++++++++++++++++++++++++++++
 3 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/src/parser/traverser/expression.ts b/src/parser/traverser/expression.ts
index 5fb75b82..9421316e 100644
--- a/src/parser/traverser/expression.ts
+++ b/src/parser/traverser/expression.ts
@@ -837,13 +837,19 @@ function parseObjectProperty(isPattern: boolean, isBlockScope: boolean): void {
   // If we're in a destructuring, we've now discovered that the key was actually an assignee, so
   // we need to tag it as a declaration with the appropriate scope. Otherwise, we might need to
   // transform it on access, so mark it as a normal object shorthand.
+  let identifierRole;
   if (isPattern) {
-    state.tokens[state.tokens.length - 1].identifierRole = isBlockScope
-      ? IdentifierRole.ObjectShorthandBlockScopedDeclaration
-      : IdentifierRole.ObjectShorthandFunctionScopedDeclaration;
+    if (state.scopeDepth === 0) {
+      identifierRole = IdentifierRole.ObjectShorthandTopLevelDeclaration;
+    } else if (isBlockScope) {
+      identifierRole = IdentifierRole.ObjectShorthandBlockScopedDeclaration;
+    } else {
+      identifierRole = IdentifierRole.ObjectShorthandFunctionScopedDeclaration;
+    }
   } else {
-    state.tokens[state.tokens.length - 1].identifierRole = IdentifierRole.ObjectShorthand;
+    identifierRole = IdentifierRole.ObjectShorthand;
   }
+  state.tokens[state.tokens.length - 1].identifierRole = identifierRole;
 
   // Regardless of whether we know this to be a pattern or if we're in an ambiguous context, allow
   // parsing as if there's a default value.
diff --git a/src/parser/traverser/lval.ts b/src/parser/traverser/lval.ts
index 22fdd54c..9a6a8742 100644
--- a/src/parser/traverser/lval.ts
+++ b/src/parser/traverser/lval.ts
@@ -39,13 +39,15 @@ export function parseImportedIdentifier(): void {
 }
 
 export function markPriorBindingIdentifier(isBlockScope: boolean): void {
+  let identifierRole;
   if (state.scopeDepth === 0) {
-    state.tokens[state.tokens.length - 1].identifierRole = IdentifierRole.TopLevelDeclaration;
+    identifierRole = IdentifierRole.TopLevelDeclaration;
+  } else if (isBlockScope) {
+    identifierRole = IdentifierRole.BlockScopedDeclaration;
   } else {
-    state.tokens[state.tokens.length - 1].identifierRole = isBlockScope
-      ? IdentifierRole.BlockScopedDeclaration
-      : IdentifierRole.FunctionScopedDeclaration;
+    identifierRole = IdentifierRole.FunctionScopedDeclaration;
   }
+  state.tokens[state.tokens.length - 1].identifierRole = identifierRole;
 }
 
 // Parses lvalue (assignable) atom.
diff --git a/test/typescript-test.ts b/test/typescript-test.ts
index f997ace3..53797ce8 100644
--- a/test/typescript-test.ts
+++ b/test/typescript-test.ts
@@ -1595,6 +1595,34 @@ describe("typescript transform", () => {
     );
   });
 
+  it("preserves exported variables assigned with a destructure", () => {
+    assertTypeScriptImportResult(
+      `
+      const o = {x: 1};
+      const {x} = o;
+      const {x: y} = o;
+      type z = number;
+      export {x, y, z};
+    `,
+      {
+        expectedCJSResult: `"use strict";${ESMODULE_PREFIX}
+      const o = {x: 1};
+      const {x} = o;
+      const {x: y} = o;
+      
+      exports.x = x; exports.y = y;
+    `,
+        expectedESMResult: `
+      const o = {x: 1};
+      const {x} = o;
+      const {x: y} = o;
+      
+      export {x, y,};
+    `,
+      },
+    );
+  });
+
   it("elides export default when the value is an identifier declared as a type", () => {
     assertTypeScriptImportResult(
       `