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

Create parser plugin "topLevelAwait" #10449

Merged
merged 8 commits into from Oct 29, 2019

Conversation

nicolo-ribaudo
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo commented Sep 17, 2019

Q                       A
Fixed Issues? Fixes #1, Fixes #2
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link babel/website#2115
Any Dependency Changes?
License MIT

It is a Stage 3 proposal: https://github.com/tc39/proposal-top-level-await

This is different from the allowAwaitOutsideFunction because the top-level-await proposal only allows it in modules, while allowAwaitOutsideFunction also allows it in scripts. Also, I think that we need the plugin rather than the option for consistency with all the other plugins.

Ref: babel/proposals#44, #7637

@nicolo-ribaudo nicolo-ribaudo added PR: New Feature 🚀 A type of pull request used for our changelog categories pkg: parser labels Sep 17, 2019
@babel-bot
Copy link
Collaborator

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

@nicolo-ribaudo nicolo-ribaudo added this to the v7.7.0 milestone Sep 17, 2019
@hzoo
Copy link
Member

hzoo commented Oct 1, 2019

Awesome! Should we add something like a flag using caller for webpack support?

function supportsStaticESM(caller) {
return !!(caller && caller.supportsStaticESM);
}
function supportsDynamicImport(caller) {
return !!(caller && caller.supportsDynamicImport);
}

@nicolo-ribaudo
Copy link
Member Author

We can do it when we have @babel/plugin-syntax-top-level-await

@nicolo-ribaudo nicolo-ribaudo force-pushed the top-level-await branch 2 times, most recently from d7ba3c6 to 371115b Compare October 3, 2019 08:09
@nicolo-ribaudo
Copy link
Member Author

The failing test262 tests are fixed by tc39/test262#2388

@nicolo-ribaudo nicolo-ribaudo added the PR: Needs Review A pull request awaiting more approvals label Oct 17, 2019
@@ -2183,6 +2183,7 @@ 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;
Copy link
Member

Choose a reason for hiding this comment

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

If top-level await is only valid in modules, should seeing one set sawUnambiguousESM? In other words, how should this behave with a sourceType of unambiguous? Currently, parsing await 0 with unambiguous yields an AST with sourceType of script, even though if you'd specified script it would not have parsed:

> require('./').parse('await 0', { plugins: ['topLevelAwait'], sourceType: 'unambiguous' }).program.sourceType
'script'
> require('./').parse('await 0', { plugins: ['topLevelAwait'], sourceType: 'script' }).program.sourceType
Thrown:
{ [SyntaxError: Unexpected token, expected ";" (1:6)
] pos: 6, loc: Position { line: 1, column: 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.

This is tricky because, for exmaple, this is both a valid script and a valid module:

await
0

I can set it to a module when await the next invalid token is on the same line.

@nicolo-ribaudo nicolo-ribaudo force-pushed the top-level-await branch 2 times, most recently from c9112da to 00b0989 Compare October 18, 2019 09:04
@buildsize
Copy link

buildsize bot commented Oct 18, 2019

File name Previous Size New Size Change
babel-preset-env.js 2.73 MB 2.73 MB 513 bytes (0%)
babel-preset-env.min.js 1.65 MB 1.65 MB 277 bytes (0%)
babel.js 2.92 MB 2.92 MB 513 bytes (0%)
babel.min.js 1.61 MB 1.61 MB 277 bytes (0%)
test262.tap 4.84 MB [deleted]

"column": 7
}
},
"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.

This is a script that contains an await expression? I thought that wasn’t allowed.

Copy link
Member Author

Choose a reason for hiding this comment

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

We allow it using the allowAwaitOutsideFunction option, but it's not in the spec.
It's used, for example, by @babel/template users who might then wrap it in an async function.

@@ -2234,9 +2235,18 @@ export default class ExpressionParser extends LValParser {
);
}

if (!this.scope.inFunction && !this.options.allowAwaitOutsideFunction) {
if (this.hasPrecedingLineBreak()) {
Copy link
Contributor

@JLHwung JLHwung Oct 21, 2019

Choose a reason for hiding this comment

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

I think the following examples are also ambiguous

await + 0
await - 0
await ( 0 )
await [ 0 ]
await / 0 /u

I am not sure if they are complete but we can always lookahead charcode to fix them.

this.match(tt.plusMin) ||
this.match(tt.parenL) ||
this.match(tt.bracketL) ||
this.match(tt.backQuote) ||
Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome! I missed await ` 0 ` .

I come up with a new case: If one turns on v8intrinsic, the following could be ambiguous, too.

await %a()

@nicolo-ribaudo
Copy link
Member Author

I was initially going to add an afterExpr property to the tokens and then check this.state.type.afterExpr && this.state.type.startsExpr, but since it would only be used here I kept the this.match logic instead.

Co-Authored-By: Brian Ng <bng412@gmail.com>
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 Spec: Top Level Await
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants