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

Jump to bottom

Parse async do expressions #13043

Merged
merged 13 commits into from Apr 7, 2021
Merged

Parse async do expressions #13043

merged 13 commits into from Apr 7, 2021

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Mar 23, 2021

Q                       A
Fixed Issues? Parsing and generator support for Async Do Expressions
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature? Yes
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

The AST design follows ESTree async do expressions experimental proposal.

For parsing, a new parser plugin asyncDoExpressions is introduced. Since the ECMA proposal is built off on Do Expressions, parsing an async do expressions (async do {}) requires both parser plugin doExpressions and asyncDoExpressions.

We also skip async do expressions on proposal-do-expressions since it is not supported yet. The transform support will be landed in later PRs.

@JLHwung JLHwung added PR: New Feature 🚀 A type of pull request used for our changelog categories pkg: parser Spec: Async do expressions labels Mar 23, 2021
@codesandbox-ci
Copy link

codesandbox-ci bot commented Mar 23, 2021 •
edited

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 652f932:

Sandbox Source
babel-repl-custom-plugin Configuration
babel-plugin-multi-config Configuration

@babel-bot
Copy link
Collaborator

babel-bot commented Mar 23, 2021 •
edited

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

nicolo-ribaudo
nicolo-ribaudo reviewed Mar 26, 2021
View reviewed changes
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.

Can you add a test for a statement starting with async do? They are different from sync do expressions because async ones don't have the do-while ambiguity.

And also one with a line break between async and do in a statement position.

packages/babel-parser/src/parser/expression.js Show resolved Hide resolved
jridgewell
jridgewell approved these changes Mar 26, 2021
View reviewed changes
packages/babel-plugin-proposal-do-expressions/src/index.js
const { node } = path;
if (node.async) {
// Async do expressions are not yet supported
return;
Copy link
Member

Choose a reason for hiding this comment

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

Nit: path.buildCodeFrameError?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I lean to skip these paths otherwise if we support them in proposal-async-do-expressions, the error here will impose a restriction that proposal-async-do-expressions must run before proposal-do-expressions.

We have similar approach on skipping for await of in for-of transform:

babel/packages/babel-plugin-transform-for-of/src/index.js

Lines 58 to 60 in 16d8300

if (isAwait) {
return;
}

@nicolo-ribaudo nicolo-ribaudo added this to the v7.14.0 milestone Mar 26, 2021
nicolo-ribaudo
nicolo-ribaudo reviewed Mar 26, 2021
View reviewed changes
packages/babel-parser/src/plugin-utils.js
) {
throw new Error(
"'asyncDoExpressions' requires 'doExpressions', please add 'doExpressions' to parser plugins.",
);
Copy link
Member

Choose a reason for hiding this comment

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

If we add .missingPlugin = ["doExpressions"] to the error (similarly to what expectPlugin does), @babel/core will suggest adding plugin-proposal-do-expressions to the config.

JLHwung
JLHwung commented Mar 26, 2021
View reviewed changes
packages/babel-generator/test/fixtures/async-do-expressions/retain-lines/output.js Outdated
@@ -0,0 +1,3 @@
/* leading comments
*/(async do
{1;}) + 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think we will need parentheses here.

@nicolo-ribaudo nicolo-ribaudo added the PR: Ready to be Merged A pull request with already two approvals, but waiting for the next minor release label Mar 26, 2021
@JLHwung JLHwung force-pushed the parse-async-do-expressions branch from 5aab1a9 to 652f932 Compare April 6, 2021 18:37
@JLHwung JLHwung changed the base branch from main to feat-7.14.0/async-do-expressions April 7, 2021 14:39
@JLHwung
Copy link
Contributor Author

JLHwung commented Apr 7, 2021

Merged to feat-7.14.0/async-do-expressions.

@JLHwung JLHwung merged commit a6b98a7 into babel:feat-7.14.0/async-do-expressions Apr 7, 2021
@JLHwung JLHwung deleted the parse-async-do-expressions branch April 7, 2021 16:24
@JLHwung JLHwung mentioned this pull request Apr 7, 2021
Merged
2 tasks
nicolo-ribaudo pushed a commit that referenced this pull request Apr 12, 2021
@JLHwung @nicolo-ribaudo
Parse async do expressions (#13043)
ee581c3 
* parse async do expressions

* add test cases

* update test fixtures

* chore: add syntax-async-do-expressions

* generater support

* fix: do not transform async do expressions

* chore: add asyncDoExpressions to missing plugin helpers

* update ast types

* add more test cases

* throw when asyncDoExpressions is enabled but not doExpressions

* avoid add parentheses for async do expressions

* address review comments

* chore: update parser typings
nicolo-ribaudo pushed a commit that referenced this pull request Apr 20, 2021
@JLHwung @nicolo-ribaudo
Parse async do expressions (#13043)
1989353 
* parse async do expressions

* add test cases

* update test fixtures

* chore: add syntax-async-do-expressions

* generater support

* fix: do not transform async do expressions

* chore: add asyncDoExpressions to missing plugin helpers

* update ast types

* add more test cases

* throw when asyncDoExpressions is enabled but not doExpressions

* avoid add parentheses for async do expressions

* address review comments

* chore: update parser typings
nicolo-ribaudo pushed a commit that referenced this pull request Apr 28, 2021
@JLHwung @nicolo-ribaudo
Parse async do expressions (#13043)
28d7442 
* parse async do expressions

* add test cases

* update test fixtures

* chore: add syntax-async-do-expressions

* generater support

* fix: do not transform async do expressions

* chore: add asyncDoExpressions to missing plugin helpers

* update ast types

* add more test cases

* throw when asyncDoExpressions is enabled but not doExpressions

* avoid add parentheses for async do expressions

* address review comments

* chore: update parser typings
@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 Jul 8, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@hzoo hzoo hzoo left review comments

@existentialism existentialism existentialism approved these changes

@jridgewell jridgewell jridgewell approved these changes

@nicolo-ribaudo nicolo-ribaudo nicolo-ribaudo approved these changes

Assignees
No one assigned
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: parser PR: New Feature 🚀 A type of pull request used for our changelog categories PR: Ready to be Merged A pull request with already two approvals, but waiting for the next minor release Spec: Async do expressions
Projects
None yet
Milestone
v7.14.0
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@JLHwung @babel-bot @existentialism @jridgewell @hzoo @nicolo-ribaudo