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: TopLevelAwait should respect await identifiers defined in sub scope. #10947

Merged
merged 1 commit into from Jan 3, 2020

Conversation

@JLHwung
Copy link
Contributor

JLHwung commented Dec 31, 2019

Q                       A
Fixed Issues? Babel incorrectly parse AwaitExpression when topLevelAwait is enabled
Patch: Bug Fix? Yes
License MIT

This PR includes commits in #10946 because one of the test case relies on that PR. Only commits descending from b60de33 should be reviewed.

This PR assigns SCOPE_ASYNC to the program scope if topLevelAwait is enabled and sourceType is module. By doing so we can reused the inAsync check in isAwaitAllowed, this can avoid parsing await expression when await is an identifier which is not defined in the top level.

The added test cases are parsed successfully before this PR, which should throw.

@JLHwung JLHwung force-pushed the JLHwung:fix-top-level-await branch from b60de33 to 0c66c47 Dec 31, 2019
@@ -2186,7 +2191,9 @@ export default class ExpressionParser extends LValParser {
isAwaitAllowed(): boolean {
if (this.scope.inFunction) return this.scope.inAsync;
if (this.options.allowAwaitOutsideFunction) return true;
if (this.hasPlugin("topLevelAwait")) return this.inModule;
if (this.hasPlugin("topLevelAwait")) {
return this.inModule && this.scope.inAsync;

This comment has been minimized.

Copy link
@nicolo-ribaudo

nicolo-ribaudo Jan 1, 2020

Member

Since at this point this.scope.inFunction is false, doesn't this.inModule already imply this.scope.inAsync?

This comment has been minimized.

Copy link
@JLHwung

JLHwung Jan 1, 2020

Author Contributor

Nope. For example

class C {
  p = await 0;
}

Even top level await is enabled, await cant not be allowed here because it is not allowed in field initializers.

I think the idea of top level await is that the whole module is wrapped in an async function, which is what I do here to assign SCOPE_ASYNC to the program scope. Then we will need to handle cases where await is not allowed.

@nicolo-ribaudo nicolo-ribaudo merged commit 6ee8c97 into babel:master Jan 3, 2020
4 of 5 checks passed
4 of 5 checks passed
build (13.x)
Details
test262 Workflow: test262
Details
Travis CI - Pull Request Build Passed
Details
build-standalone Workflow: build-standalone
Details
e2e Workflow: e2e
Details
@nicolo-ribaudo nicolo-ribaudo deleted the JLHwung:fix-top-level-await branch Jan 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.