Skip to content

Port TS#63043: Implement walkBindingPattern for declaration emit of destructured parameter properties #2751

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DanielRosenwasser merged 3 commits into from
Feb 18, 2026
Merged

Port TS#63043: Implement walkBindingPattern for declaration emit of destructured parameter properties #2751

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bc52e7a
Initial plan
Copilot Feb 11, 2026
ab6e0ba
Port TS#63043: Fix transform crash with destructured parameter property
Copilot Feb 11, 2026
598ba83
Port TS#63154: Add continue after recursing into nested binding patterns
Copilot Feb 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion internal/transformers/declarations/transform.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1430,7 +1430,7 @@ func (tx *DeclarationTransformer) transformClassDeclaration(input *ast.ClassDecl
parameterProperties = append(parameterProperties, updated)
} else {
// Pattern - this is currently an error, but we emit declarations for it somewhat correctly
// !!! is this worth reimplementing? We never made it not-an-error
parameterProperties = append(parameterProperties, tx.walkBindingPattern(param.Name().AsBindingPattern(), param)...)
}
}
tx.state.getSymbolAccessibilityDiagnostic = oldDiag
Expand Down Expand Up @@ -1542,6 +1542,27 @@ func (tx *DeclarationTransformer) transformClassDeclaration(input *ast.ClassDecl
)
}

func (tx *DeclarationTransformer) walkBindingPattern(pattern *ast.BindingPattern, param *ast.Node) []*ast.Node {
var elems []*ast.Node
for _, elem := range pattern.Elements.Nodes {
if ast.IsOmittedExpression(elem) {
continue
}
if ast.IsBindingPattern(elem.Name()) {
elems = append(elems, tx.walkBindingPattern(elem.Name().AsBindingPattern(), param)...)
continue
}
elems = append(elems, tx.Factory().NewPropertyDeclaration(
tx.ensureModifiers(param),
elem.Name(),
Comment on lines +1551 to +1557
Copy link
Copy Markdown
Member

@DanielRosenwasser DanielRosenwasser Feb 17, 2026

Choose a reason for hiding this comment

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

There's a bug in the original code and the port - I assume the IsBindingPattern branch is supposed to continue. I'm pretty sure that this code will crash in the new compiler:

export class C {
  constructor(public [[x]]: any[]) {}
}

I've opened up microsoft/TypeScript#63153

Copy link
Copy Markdown
Member

@DanielRosenwasser DanielRosenwasser Feb 18, 2026

Choose a reason for hiding this comment

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

PR is at microsoft/TypeScript#63154

Pretty sure no test is required for this (we'll get one after we update the submodule), but basically we need a continue statement in the IsBindingPattern branch.

Copy link
Copy Markdown
Member

@jakebailey jakebailey Feb 18, 2026

Choose a reason for hiding this comment

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

@copilot Read microsoft/TypeScript#63154 and port it to this PR

Copy link
Copy Markdown
Contributor Author

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

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

Added the continue statement in 598ba830. The walkBindingPattern now recurses into nested binding patterns and skips emitting a PropertyDeclaration for the pattern itself, matching the fix in TS#63154.

nil, /*questionOrExclamationToken*/
tx.ensureType(elem, false),
nil, /*initializer*/
))
Comment on lines +1547 to +1561
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

walkBindingPattern unconditionally calls ast.IsBindingPattern(elem.Name()) and then emits a PropertyDeclaration with elem.Name().

  • In array binding patterns, the parser represents elisions (leading ,) as BindingElement nodes with name == nil (see BindingElement.name comment). In that case elem.Name() is nil and ast.IsBindingPattern(nil) will panic.
  • For nested destructuring (e.g. { a: { b } }), elem.Name() can be a BindingPattern. Emitting a PropertyDeclaration with a binding pattern name will later panic in the printer (emitPropertyName only handles identifiers/literals/computed/private identifiers).

Consider: (1) skip elements with elem.Name() == nil, and (2) when elem.Name() is a binding pattern, recurse and continue (only emit a property when the binding name is an identifier/property-name node).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

@DanielRosenwasser DanielRosenwasser Feb 18, 2026

Choose a reason for hiding this comment

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

I guess Copilot caught the same thing I called out but I didn't see the comment.

}
return elems
}

func (tx *DeclarationTransformer) transformVariableStatement(input *ast.VariableStatement) *ast.Node {
visible := false
for _, decl := range input.DeclarationList.AsVariableDeclarationList().Declarations.Nodes {
Expand Down
9 changes: 9 additions & 0 deletions ...baselines/reference/submodule/compiler/declarationEmitDestructuringParameterProperties.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,16 @@ class C3 {

//// [declarationEmitDestructuringParameterProperties.d.ts]
declare class C1 {
x: string;
y: string;
z: string;
constructor([x, y, z]: string[]);
}
type TupleType1 = [string, number, boolean];
declare class C2 {
x: string;
y: number;
z: boolean;
constructor([x, y, z]: TupleType1);
}
type ObjType1 = {
Expand All @@ -48,5 +54,8 @@ type ObjType1 = {
z: boolean;
};
declare class C3 {
x: number;
y: string;
z: boolean;
constructor({ x, y, z }: ObjType1);
}
28 changes: 0 additions & 28 deletions ...ines/reference/submodule/compiler/declarationEmitDestructuringParameterProperties.js.diff
View file
Open in desktop

This file was deleted.

9 changes: 9 additions & 0 deletions ...aselines/reference/submodule/compiler/declarationEmitDestructuringParameterProperties2.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,16 @@ class C3 {

//// [declarationEmitDestructuringParameterProperties2.d.ts]
declare class C1 {
x: string;
y: string;
z: string;
constructor([x, y, z]: string[]);
}
type TupleType1 = [string, number, boolean];
declare class C2 {
x: string;
y: number;
z: boolean;
constructor([x, y, z]: TupleType1);
}
type ObjType1 = {
Expand All @@ -48,5 +54,8 @@ type ObjType1 = {
z: boolean;
};
declare class C3 {
x: number;
y: string;
z: boolean;
constructor({ x, y, z }: ObjType1);
}
28 changes: 0 additions & 28 deletions ...nes/reference/submodule/compiler/declarationEmitDestructuringParameterProperties2.js.diff
View file
Open in desktop

This file was deleted.