Skip to content

Fix CommonJS rewrites in shorthand destructuring defaults#4111

Merged
jakebailey merged 2 commits into
mainfrom
copilot/fix-destructuring-default-initializer
Jun 2, 2026
Merged

Fix CommonJS rewrites in shorthand destructuring defaults#4111
jakebailey merged 2 commits into
mainfrom
copilot/fix-destructuring-default-initializer

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 30, 2026

Imported and exported identifiers used inside shorthand destructuring default initializers were emitted unchanged in CommonJS output, causing runtime ReferenceErrors.

  • Identifier reference detection

    • Treat ShorthandPropertyAssignment.ObjectAssignmentInitializer as an expression reference position.
    • This allows existing CommonJS substitution logic to rewrite imported/exported identifiers there.
  • Regression coverage

    • Added a compiler emit test covering both imported and exported identifiers in shorthand destructuring defaults.
import { imported } from "./b";
export const exported = 2;
let a, b;
({ a = imported, b = exported } = {});

now emits:

({ a = b_1.imported, b = exports.exported } = {});

Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix imported identifier destructuring default not rewritten Fix CommonJS rewrites in shorthand destructuring defaults May 30, 2026
Copilot AI requested a review from jakebailey May 30, 2026 17:12
@jakebailey jakebailey marked this pull request as ready for review June 1, 2026 17:58
Copilot AI review requested due to automatic review settings June 1, 2026 17:58
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes CommonJS emit rewriting for imported/exported identifiers used in shorthand destructuring default initializers (e.g. ({ a = imported } = {})), preventing runtime ReferenceErrors by ensuring name substitution runs in that expression position.

Changes:

  • Update identifier reference detection to treat ShorthandPropertyAssignment.ObjectAssignmentInitializer as an expression reference position.
  • Add a new compiler emit regression test covering both imported and exported identifiers in shorthand destructuring defaults.
  • Add the corresponding reference baseline verifying emitted CommonJS output rewrites to require(...)/exports.* references.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
internal/transformers/utilities.go Extends IsIdentifierReference so identifier substitution applies within shorthand destructuring default initializers.
testdata/tests/cases/compiler/commonjsShorthandDestructuringDefaultImportExport.ts Adds a targeted compiler test for shorthand destructuring defaults referencing imports/exports under CommonJS.
testdata/baselines/reference/compiler/commonjsShorthandDestructuringDefaultImportExport.js Adds the reference baseline asserting correct rewritten CommonJS emit.

Copy link
Copy Markdown
Member

@RyanCavanaugh RyanCavanaugh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very disturbing to not have Strada test for this!

@jakebailey jakebailey added this pull request to the merge queue Jun 2, 2026
Merged via the queue into main with commit 90c540a Jun 2, 2026
22 checks passed
@jakebailey jakebailey deleted the copilot/fix-destructuring-default-initializer branch June 2, 2026 23:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Imported/exported identifier used as a shorthand destructuring default initializer is not rewritten

4 participants