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: False positive TDZ check in class computed property keys #16429

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

liuxingbaoyu
Copy link
Member

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

@liuxingbaoyu liuxingbaoyu added PR: Bug Fix 🐛 A type of pull request used for our changelog categories Spec: Class Fields labels Apr 13, 2024
@babel-bot
Copy link
Collaborator

babel-bot commented Apr 13, 2024

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

Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a 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?

Copy link
Contributor

@JLHwung JLHwung left a 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._.

@liuxingbaoyu
Copy link
Member Author

Yes! It was just a simple fix because a false negative is better than a false positive here. :)

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: Class Fields
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: False positive TDZ check in class computed property keys
4 participants