Skip to content

Commit

Permalink
Fix identifier role for top-level destructure declarations (#564)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
alangpierce authored Dec 2, 2020
1 parent 080fce6 commit 80f84a6
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 8 deletions.
14 changes: 10 additions & 4 deletions src/parser/traverser/expression.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
10 changes: 6 additions & 4 deletions src/parser/traverser/lval.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
28 changes: 28 additions & 0 deletions test/typescript-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
`
Expand Down

0 comments on commit 80f84a6

Please sign in to comment.