Skip to content

fix: support mutated outer decorated class binding#16387

Merged
JLHwung merged 3 commits intobabel:mainfrom
JLHwung:fix-decorator-class-binding
Mar 28, 2024
Merged

fix: support mutated outer decorated class binding#16387
JLHwung merged 3 commits intobabel:mainfrom
JLHwung:fix-decorator-class-binding

Conversation

@JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Mar 27, 2024

Q                       A
Fixed Issues? Fixes #16356
Patch: Bug Fix? Yes
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

In this PR we generate an dedicated scope for a decorated class when its outer class binding will be mutated, because the class method, decorators and computed keys may still refer to the internal class binding.

This PR does not improve current class tdz check.

@JLHwung JLHwung added PR: Bug Fix 🐛 A type of pull request used for our changelog categories Spec: Decorators labels Mar 27, 2024
@@ -0,0 +1,145 @@
{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test is disabled due to #16386.

@babel-bot
Copy link
Collaborator

babel-bot commented Mar 27, 2024

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/56585

t.variableDeclarator(t.cloneNode(classOuterBindingLocal)),
t.variableDeclarator(classOuterBindingDelegateLocal),
]),
t.blockStatement([
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We use a block statement to simulate the inner scope so that it can inherit the Yield and Await production parameter.

@capture(() => K)
@assertUninitialized(() => K)
class K {
//todo: add the assertUninitialized decorator when we properly implement class tdz
Copy link
Member

Choose a reason for hiding this comment

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

Could you for now assert that we throw the TDZ error, so that when we fix it we don't forget to update the test?

@JLHwung JLHwung merged commit 91f55bf into babel:main Mar 28, 2024
@JLHwung JLHwung deleted the fix-decorator-class-binding branch March 28, 2024 13:55
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jun 28, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Bug Fix 🐛 A type of pull request used for our changelog categories Spec: Decorators

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: The internal class binding captured in element decorators should not be mutated

4 participants