-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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: False positive TDZ check in class computed property keys #16429
base: main
Are you sure you want to change the base?
Conversation
liuxingbaoyu
commented
Apr 13, 2024
Q | A |
---|---|
Fixed Issues? | Fixes #16386 |
Patch: Bug Fix? | √ |
Major: Breaking Change? | |
Minor: New Feature? | |
Tests Added + Pass? | √ |
Documentation PR Link | |
Any Dependency Changes? | |
License | MIT |
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/57129 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! If the arrow function in the key is immediately invoked, do we throw a TDZ error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can't statically infer whether a class binding can be resolved. For example
class C {
[f = () => C, Math.random() > .5 ? f() : Promise.resolve().then(f)]
}
will either throw a TDZ error or pass with roughly equal possibility.
Here is an approach if we would like to seriously match the tdz behaviour.
First we create an intermediate object so that we can intercept the binding identifier:
let _C = {
get _ () { throw new ReferenceError("C can not be accessed before initialization") },
set _ (_) { throw new ReferenceError("C is a constant binding") }
}
Then we overwrite the _
getterOrSetter when a class binding is assigned. For decorator transforms, this can be inserted immediately after the applyDecs
call
Object.defineProperty(_C, "_", { value: modified_class, writable: false })
Then we can replace all the reference identifiers from the inner binding C
to _C._
.
Yes! It was just a simple fix because a false negative is better than a false positive here. :) |