Skip to content

Fix: Support parameter properties in class constructors #619

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

Closed
wants to merge 3 commits into from
Closed

Fix: Support parameter properties in class constructors #619

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
3e7da4e
Fix: Support parameter properties in class constructors
zanuka Mar 15, 2025
d92783c
fix: remove extra blank lines in runtimesyntax.go
zanuka Mar 16, 2025
afd7524
Merge branch 'main' into fix/536-param-props-not-supported
zanuka Mar 16, 2025
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
27 changes: 25 additions & 2 deletions internal/transformers/runtimesyntax.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -810,7 +810,30 @@ func (tx *RuntimeSyntaxTransformer) visitClassExpression(node *ast.ClassExpressi

func (tx *RuntimeSyntaxTransformer) visitConstructorDeclaration(node *ast.ConstructorDeclaration) *ast.Node {
modifiers := tx.visitor.VisitModifiers(node.Modifiers())
parameters := tx.emitContext.VisitParameters(node.ParameterList(), tx.visitor)
var nonParamPropertyParams []*ast.ParameterDeclaration
for _, param := range node.ParameterList().Nodes {
nonParamPropertyParams = append(nonParamPropertyParams, param.AsParameterDeclaration())
}
var newParams []*ast.Node
for _, param := range nonParamPropertyParams {
var paramModifiers *ast.ModifierList
if ast.IsParameterPropertyDeclaration(param.AsNode(), node.AsNode()) {
paramModifiers = nil
} else {
paramModifiers = param.Modifiers()
}
newParam := tx.factory.NewParameterDeclaration(
paramModifiers,
param.DotDotDotToken,
param.Name().Clone(tx.factory),
param.QuestionToken,
nil,
param.Initializer,
)
newParams = append(newParams, newParam)
}
parameters := tx.factory.NewNodeList(newParams)
parameters.Loc = node.ParameterList().Loc
body := tx.visitConstructorBody(node.Body.AsBlock(), node.AsNode())
return tx.factory.UpdateConstructorDeclaration(node, modifiers, nil /*typeParameters*/, parameters, nil /*returnType*/, body)
}
Expand Down Expand Up @@ -878,7 +901,7 @@ func (tx *RuntimeSyntaxTransformer) visitConstructorBody(body *ast.Block, constr
statements = append(statements, tx.transformConstructorBodyWorker(rest, superPath, parameterPropertyAssignments)...)
} else {
statements = append(statements, parameterPropertyAssignments...)
statements = append(statements, core.FirstResult(tx.visitor.VisitSlice(rest))...)
statements = append(statements, rest...)
}

statements = tx.emitContext.EndAndMergeVariableEnvironment(statements)
Expand Down
23 changes: 23 additions & 0 deletions internal/transformers/runtimesyntax_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,29 @@ func TestParameterPropertyTransformer(t *testing.T) {
constructor(x) {
this.x = x;
}
}`},
{title: "multiple parameter properties", input: `class Project {
Copy link
Contributor

@frodi-karlsson frodi-karlsson Mar 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Running this test without the changes on up to date main, it also passes. I'm not so sure this is fixing anything, as there doesn't seem to be an issue at first look. Are you experiencing something different?

Theres of course still an issue with a super call's position as it relates to the parameter initializations, which I think just needs a minimal fix (#547)

Copy link
Author

@zanuka zanuka Mar 18, 2025

Choose a reason for hiding this comment

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

Thanks for the comments, much appreciated. After further investigation and testing, I've confirmed that this PR is redundant. The parameter property transformation functionality is already properly implemented in the codebase through commit 8b95466 (PR #479).

I checked the following:

  1. All tests pass on the main branch without these changes
  2. The existing implementation already handles:
    • Parameter property detection
    • Modifier stripping
    • Property assignments in constructor
    • Super call handling
    • Multiple parameter properties

I'll close this PR since it would be duplicating existing functionality.

constructor(
public name: string,
public deadline: Date,
public budget: number
) {}

extendDeadline(days: number): void {
this.deadline.setDate(this.deadline.getDate() + days);
}
}`, output: `class Project {
name;
deadline;
budget;
constructor(name, deadline, budget) {
this.name = name;
this.deadline = deadline;
this.budget = budget;
}
extendDeadline(days) {
this.deadline.setDate(this.deadline.getDate() + days);
}
}`},
}

Expand Down