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

Disallow await as bound name in using declaration #15391

Merged
merged 5 commits into from Feb 2, 2023

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Jan 31, 2023

Q                       A
Fixed Issues? Implements tc39/proposal-explicit-resource-management#138
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

This PR starts by simplifying let-as-name binding checking, but then I realize that the using declaration has already banned binding patterns, which greatly simplifies the await-checking. Anyway I am pleased that we get rid of a parameter from the hot path checkIdentifier.

@JLHwung JLHwung added PR: Spec Compliance 👓 A type of pull request used for our changelog categories Spec: Explicit Resource Management labels Jan 31, 2023
@babel-bot
Copy link
Collaborator

babel-bot commented Jan 31, 2023

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

@JLHwung JLHwung force-pushed the ban-await-as-bound-name-in-using branch from a5a9380 to 097eb45 Compare February 1, 2023 01:05
@@ -0,0 +1,3 @@
{
"sourceType": "script"
Copy link
Member

Choose a reason for hiding this comment

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

Can you also add a test in modules? It throws that await is reserved in modules, right?

"type": "File",
"start":0,"end":113,"loc":{"start":{"line":1,"column":0,"index":0},"end":{"line":12,"column":1,"index":113}},
"errors": [
"SyntaxError: Can not use 'await' as identifier inside an async function. (2:8)",
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated, but this error message is wrong (we are not in an async function, but at the top-level of a module).

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 don't differentiate between module top level and async function as long as [Await] production parameter set. Maybe rephrase it as "Can not use 'await' as identifier inside an async context"?

@JLHwung JLHwung merged commit e5e923d into babel:main Feb 2, 2023
@JLHwung JLHwung deleted the ban-await-as-bound-name-in-using branch February 2, 2023 14:58
@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 May 5, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 5, 2023
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 pkg: parser PR: Spec Compliance 👓 A type of pull request used for our changelog categories Spec: Explicit Resource Management
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants