-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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 inside async arrow params #10469
Conversation
const oldInClassProperty = this.state.inClassProperty; | ||
const oldYieldPos = this.state.yieldPos; | ||
const oldAwaitPos = this.state.awaitPos; | ||
this.state.maybeInArrowParameters = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is needed because this test started failing:
async function f() {
(function await() {});
}
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/11707/ |
language/expressions/async-arrow-function/early-errors-arrow-await-in-formals-default.js(default) | ||
language/expressions/async-arrow-function/early-errors-arrow-await-in-formals-default.js(strict mode) | ||
language/expressions/async-arrow-function/early-errors-arrow-await-in-formals.js(default) | ||
language/expressions/async-arrow-function/early-errors-arrow-await-in-formals.js(strict mode) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
interesting.. didn't know so much test were whitelisted. 🤔
// } | ||
// | ||
if (this.isAwaitAllowed() || oldMaybeInArrowParameters) { | ||
this.state.awaitPos = oldAwaitPos || this.state.awaitPos; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will oldAwaitPos
be 0
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's the default value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If topLevelAwait
is enabled, await
could appear at pos 0
. It is a bit confusing if we use oldAwaitPos = 0
to represent we haven't met await
yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right, Will update it to use -1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have also changed it for yieldPos
for consistency, even if it's not strictly necessary.
this.state.yieldPos = oldYieldPos || this.state.yieldPos; | ||
this.state.awaitPos = oldAwaitPos || this.state.awaitPos; | ||
// | ||
// Hi developer of the future :) If you are implementing generator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:)
// an async function: | ||
// | ||
// async function a() { | ||
// function b(param = async (await)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sloppy mode 😢
@@ -0,0 +1 @@ | |||
async (a = ({ await }) => {}) => {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually would have read this as a ReferenceError
at first but after paying closer attention the early error makes sense 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙃
Follow up to #7727