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 top-level await parsing by default #13387

Merged
merged 4 commits into from Aug 3, 2021

Conversation

nicolo-ribaudo
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo commented May 28, 2021

Q                       A
Fixed Issues? Ref: babel/proposals#74 (comment)
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature? Yes
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

@nicolo-ribaudo nicolo-ribaudo added PR: New Feature 🚀 A type of pull request used for our changelog categories pkg: parser labels May 28, 2021
@@ -1,3 +1,4 @@
{
"sourceType": "module"
"sourceType": "module",
"throws": "Unexpected token (1:6)"
Copy link
Member Author

Choose a reason for hiding this comment

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

The input code is await = foo();. We can potentially recover here I think, parsing await as an identifier.

@@ -2,7 +2,7 @@
"type": "File",
"start":0,"end":20,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":20}},
"errors": [
"SyntaxError: Unexpected reserved word 'await'. (1:6)"
"SyntaxError: Can not use 'await' as identifier inside an async function. (1:6)"
Copy link
Member Author

Choose a reason for hiding this comment

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

The input code is const await = foo();. As reported on twitter by @KFlash, this error message is confusing (https://twitter.com/buzyescayadev/status/1398240006670454784).

I will open a follow-up PR to fix it, to keep this one as only switching the plugin on by default.

Copy link
Contributor

Choose a reason for hiding this comment

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

We could just change the error message to "... async function or top level module".

Copy link

@KFlash KFlash May 28, 2021

Choose a reason for hiding this comment

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

@JLHwung then what will be the error message when you are parsing in an non-async func body or arrow? Same error?
None of this is consistent with the error message thrown when e.g. parsing out a formal param list with await.

await = x is an unexpected token? Where is that token? Between 1 - 6 there are 3x token and 2x whitespace

function x () { await } trigger an different error message again.

Copy link
Member Author

Choose a reason for hiding this comment

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

The token is the one at line 1, column 6: any editor, or the Babel cli, or Babel integrations will point to the =.

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe we can always throw Unexpected reserved word 'await'. in modules (since the reason we throw is that it's reserved), and keep throwing the current error in async functions inside scripts (await is not reserved there, but it's special-cased to be illegal).

@babel-bot
Copy link
Collaborator

babel-bot commented May 28, 2021

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

@codesandbox-ci
Copy link

codesandbox-ci bot commented May 28, 2021

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 62473e4:

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

@@ -221,6 +222,7 @@ exportSpecifierAndLocalMemberDeclaration.ts
exportStarFromEmptyModule.ts
exportStarNotElided.ts
expressionsForbiddenInParameterInitializers.ts
extendedUnicodePlaneIdentifiers.ts
Copy link
Contributor

Choose a reason for hiding this comment

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

I have no idea why this test is in allowlist:

Source

Babel parser parses successfully, as expected. (repl)

decorators/migrated_0003.js
export_import_reserved_words/migrated_0003.js
export_statements/export_trailing_comma.js
for_await_loops/migrated_0000.js
Copy link
Contributor

Choose a reason for hiding this comment

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

For reference, this file now parses successfully due to top level await,

for await (let x of e) {}

and we assume all the Flow parser tests are running with module source type.

@@ -5,6 +5,7 @@ JSX/invalid_unpaired_rcurly.js
JSX_invalid/migrated_0000.js
arrow_function/object_return_type.js
arrow_function_invalid/migrated_0002.js
async_arrow_functions/with_type_parameters_types_disabled.js
Copy link
Contributor

Choose a reason for hiding this comment

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

This test is added because Flow has a new test options: types: boolean that can control whether parameter types should be parsed:

https://github.com/facebook/flow/blob/master/src/parser/test/flow/async_arrow_functions/with_type_parameters_types_disabled.options.json

We don't have such an option.

Copy link
Member Author

Choose a reason for hiding this comment

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

From a parsing perspective, is it the same as all: true?

Comment on lines +18 to +30
comment_interning/class_method.js
comment_interning/class_property.js
comment_interning/declare_export_declaration.js
comment_interning/declare_interface.js
comment_interning/declare_type_alias.js
comment_interning/decorator.js
comment_interning/function_declaration.js
comment_interning/import.js
comment_interning/interface.js
comment_interning/object_type.js
comment_interning/remove_type_trailing_comments.js
comment_interning/super.js
comment_interning/type_alias.js
Copy link
Contributor

Choose a reason for hiding this comment

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

Babel is throwing on these cases because of duplicate class declaration class C. The Flow parser is tolerant on duplicate binding, which are throw later by the type checker.

startPos,
this.hasPlugin("topLevelAwait")
? Errors.AwaitNotInAsyncContext
: Errors.AwaitNotInAsyncFunction,
Copy link
Contributor

Choose a reason for hiding this comment

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

We can remove unused Errors.AwaitNotInAsyncFunction.

@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 Jul 26, 2021
@RedGuy12
Copy link

does this mean i no longer need @babel/plugin-syntax-top-level-await anymore?

@nicolo-ribaudo
Copy link
Member Author

Correct! But you need to make sure to use @babel/parser ^7.15.0 (if you are using @babel/core@^7.15.0 it depends on a new enough version of @babel/parser).

@RedGuy12
Copy link

i'm only using @babel/eslint-parser while i wait for eslint v8 to ship with top-level await support

here is the related part of my config:

	parser: "@babel/eslint-parser",
	parserOptions: {
		requireConfigFile: false,
		sourceType: "module",
		babelOptions: {
			plugins: [
				"@babel/plugin-syntax-top-level-await"
			],
		},
	}

can I just remove plugins and it will work? or do I have to do other things as well? thanks

@nicolo-ribaudo
Copy link
Member Author

Yes, it should work.

@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 Nov 17, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 17, 2021
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: 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants