Skip to content
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.

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

Fix decorator emit crash #60224

Merged
merged 1 commit into from
Oct 14, 2024
Merged

Fix decorator emit crash #60224

merged 1 commit into from
Oct 14, 2024

Conversation

rbuckton
Copy link
Member

Fixes a crash when transforming Stage 3 decorators when both class element decorators and class constructor parameter decorators are present.

Fixes #58269

@typescript-bot typescript-bot added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels Oct 14, 2024
const decorators = getDecorators(node);
const parameters = getDecoratorsOfParameters(getFirstConstructorWithBody(node));
const parameters = useLegacyDecorators ? getDecoratorsOfParameters(getFirstConstructorWithBody(node)) : undefined;
Copy link
Member

Choose a reason for hiding this comment

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

Should getDecoratorsOfParameters take the parameter and return undefined instead of copying the ternary to all call sites?

Copy link
Member Author

Choose a reason for hiding this comment

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

Branching is cheaper at runtime than invocation, so probably not.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe a bit overoptimized but alright.

Copy link
Member Author

@rbuckton rbuckton Oct 14, 2024

Choose a reason for hiding this comment

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

In this case specifically, just passing the argument to getDecoratorsOfParameters means iterating over all class elements to find a constructor, which is unnecessary work.

@rbuckton rbuckton merged commit 40caf34 into main Oct 14, 2024
32 checks passed
@rbuckton rbuckton deleted the fix58269 branch October 14, 2024 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Debug Failure in transformClassLike in "ghost" after #56955
3 participants