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

Enable statically analyzable TDZ checks by default #15285

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nicolo-ribaudo
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo commented Dec 16, 2022

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

Now that the TDZ transform is more stable, this PR enables TDZ checks for statically analyzable TDZ errors by default. In a future minor I would like to add a noDynamicTDZ assumption (that defaults to true) that, when not enabled, makes the block scoping plugin inject the dynamic TDZ checks.

The tdz option will then become unnecessary, and can be removed in Babel 8. It's currently one of the few options that require users to explicitly list the plugin in their config.

As a "spec compliancy" PR, I don't think that this one needs to wait for a minor.

This PR is blocked on fixing #15175, to avoid introducing a regression in the default behavior.

@nicolo-ribaudo nicolo-ribaudo added the PR: Spec Compliance 👓 A type of pull request used for our changelog categories label Dec 16, 2022
@babel-bot
Copy link
Collaborator

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

@liuxingbaoyu
Copy link
Member

liuxingbaoyu commented Dec 16, 2022

Hopefully this won't have a performance hit. I am worried that the default enablement may be too demanding for us, and any wrong scope information may cause problems, and it seems that there are not many users who enable the tdz option now.

FYI: https://bugs.webkit.org/show_bug.cgi?id=199866 TDZ validation has had serious performance issues even in browsers.

@nicolo-ribaudo nicolo-ribaudo changed the title Fix tdz analysis for reassigned captured for bindings Enable statically analyzable TDZ checks by default Dec 16, 2022
@nicolo-ribaudo
Copy link
Member Author

I'll take some measurements.

@nicolo-ribaudo nicolo-ribaudo marked this pull request as draft December 16, 2022 09:50
@nicolo-ribaudo
Copy link
Member Author

@liuxingbaoyu Wdyt about just having a noTDZ (defaults to true) assumption instead, without changing the default behavior? My end goal is to make it easier to enable tdz checks, and currently having it as a plugin option makes it hard.

@liuxingbaoyu
Copy link
Member

This sounds good to me.
I'm not sure if @JLHwung would prefer TDZ + disabled by default.
babel/website#2701 (review)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Spec Compliance 👓 A type of pull request used for our changelog categories Spec: TDZ
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants