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: support mutated outer decorated class binding #16387

Merged
merged 3 commits into from Mar 28, 2024

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
51 checks passed
@JLHwung JLHwung deleted the fix-decorator-class-binding branch March 28, 2024 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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