Skip to content

Skip bound method assignments in JS declaration emit#4331

Draft
Copilot wants to merge 3 commits into
mainfrom
copilot/fix-behavior-difference-reassigning-class-method
Draft

Skip bound method assignments in JS declaration emit#4331
Copilot wants to merge 3 commits into
mainfrom
copilot/fix-behavior-difference-reassigning-class-method

Conversation

Copilot AI commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

JS declaration emit was synthesizing an extra property signature for constructor-bound methods, producing both a property and method declaration for the same class member.

export class C {
    constructor() {
        this.foo = this.foo.bind(this);
    }
    foo() {}
}
  • Declaration emit

    • Resolve the assigned member access for this.x = ... before deciding whether to synthesize a JS property declaration.
    • Track seen instance/static member names so no-check declaration emit also skips members already declared on the class.
  • Regression coverage

    • Added a compiler test for constructor-bound JS class methods.
    • Baselines now emit only:
export declare class C {
    constructor();
    foo(): void;
}

Co-authored-by: DanielRosenwasser <972891+DanielRosenwasser@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix behavior difference with method reassignments in constructor Skip bound method assignments in JS declaration emit Jun 16, 2026
Copilot AI requested a review from DanielRosenwasser June 16, 2026 08:05
expandoHosts map[ast.NodeId]*ast.Node // store the result of transforming expando hosts so they can be inserted later if the host is actually referenced
expandoMembers map[ast.NodeId][]*ast.Node // store any found expando _members_ after transforming them so *if* the host is referenced, they can be emitted alongside it
seenProperties collections.Set[*ast.Node]
seenPropertyNames collections.Set[string]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@copilot rethink this - since collectThisPropertyAssignments already has a seen table it prepopulates with non-this. class members, rather than having a second (incorrectly scoped) table, just change the map key from *ast.Node directly to a composite key that contains either the string property name (for named members) or the node (for dynamically named members).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

And include a bit for instance vs static in said composite key - no need to use string concatenation.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@copilot try again

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 9cf60b3 by using a composite seen key with property name/node plus instance/static state.

Co-authored-by: weswigham <2932786+weswigham@users.noreply.github.com>
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.

Behavior difference: Reassigning class method inside constructor will make tsgo emit an extra line of method signature

3 participants