Skip to content

Disallow await inside async arrow params#10469

Merged
nicolo-ribaudo merged 2 commits intobabel:masterfrom
nicolo-ribaudo:await-async-arrow
Oct 2, 2019
Merged

Disallow await inside async arrow params#10469
nicolo-ribaudo merged 2 commits intobabel:masterfrom
nicolo-ribaudo:await-async-arrow

Conversation

@nicolo-ribaudo
Copy link
Copy Markdown
Member

Q                       A
Fixed Issues?
Patch: Bug Fix? Yes
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

Follow up to #7727

@nicolo-ribaudo nicolo-ribaudo added PR: Spec Compliance 👓 A type of pull request used for our changelog categories pkg: parser Spec: Async Functions labels Sep 19, 2019
const oldInClassProperty = this.state.inClassProperty;
const oldYieldPos = this.state.yieldPos;
const oldAwaitPos = this.state.awaitPos;
this.state.maybeInArrowParameters = false;
Copy link
Copy Markdown
Member Author

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() {});
}

@babel-bot
Copy link
Copy Markdown
Collaborator

babel-bot commented Sep 20, 2019

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

@JLHwung JLHwung self-requested a review September 20, 2019 23:55
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)
Copy link
Copy Markdown
Member

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;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Will oldAwaitPos be 0?

Copy link
Copy Markdown
Member Author

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.

Copy link
Copy Markdown
Contributor

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.

Copy link
Copy Markdown
Member Author

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

Copy link
Copy Markdown
Member Author

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
Copy link
Copy Markdown
Member

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)) {
Copy link
Copy Markdown
Member

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 }) => {}) => {};
Copy link
Copy Markdown
Member

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 🤔

Copy link
Copy Markdown
Member

@existentialism existentialism left a comment

Choose a reason for hiding this comment

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

🙃

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: Spec Compliance 👓 A type of pull request used for our changelog categories Spec: Async Functions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants