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

"await" within array comprehension returns a parse error #182

Open
nolanlawson opened this Issue Mar 1, 2015 · 3 comments

Comments

Projects
None yet
2 participants
@nolanlawson

nolanlawson commented Mar 1, 2015

This async function tries to return an array of values concurrently resolved by different Promises (i.e. an equivalent for Promise.all()):

async function demo() {
  try {
    let things = ['foo', 'bar', 'baz'];

    let promises = things.map((thing) => returnAPromise(thing));

    let results = [for (promise of promises) await promise];

    console.log('posted many docs simultaneously, results are ', results);
  } catch (err) {
    console.log(err);
  }
}

This fails with an error:

Error : Parsing file /.../index.js: Unexpected token (40:38)

My current workaround is to build up the results array like this:

let results = [];
for (let promise of promises) {
  results.push(await promise);
}

I have a live example of the code here. To reproduce, just check out the code and do npm i && npm run build.

Thanks in advance, and let me know if this is just an error in my understanding or not in Regenerator.

@benjamn

This comment has been minimized.

Show comment
Hide comment
@benjamn

benjamn Mar 1, 2015

Collaborator

Regenerator does not currently know how to translate ComprehensionExpression nodes, so you may have to run the transform for converting them to ES5 before you run Regenerator. Let me know if that's a pain, or seems like it can't be done outside Regenerator for some reason. I would prefer to keep that transform separate from Regenerator, especially since comprehensions did not end up making it into the ES6 spec (so their syntax is not final); however, I'm not fundamentally opposed to translating comprehensions. In fact, async functions are a good example of a feature that could be translated before Regenerator runs, but that can be translated to more efficient code by Regenerator.

I think a more idiomatic way of doing what you're trying to do is to use Promise.all:

let results = await Promise.all(promises);
Collaborator

benjamn commented Mar 1, 2015

Regenerator does not currently know how to translate ComprehensionExpression nodes, so you may have to run the transform for converting them to ES5 before you run Regenerator. Let me know if that's a pain, or seems like it can't be done outside Regenerator for some reason. I would prefer to keep that transform separate from Regenerator, especially since comprehensions did not end up making it into the ES6 spec (so their syntax is not final); however, I'm not fundamentally opposed to translating comprehensions. In fact, async functions are a good example of a feature that could be translated before Regenerator runs, but that can be translated to more efficient code by Regenerator.

I think a more idiomatic way of doing what you're trying to do is to use Promise.all:

let results = await Promise.all(promises);
@nolanlawson

This comment has been minimized.

Show comment
Hide comment
@nolanlawson

nolanlawson Mar 1, 2015

Heh, you're right, Promise.all is a tidy solution there. :) I've been trying to solve this without using the Promise global, though, just as a demo of what ES7 is capable of. But it's certainly shorter this way.

If the comprehension syntax is final yet, then no worries about adding it to this library. And thanks for the explanation!

nolanlawson commented Mar 1, 2015

Heh, you're right, Promise.all is a tidy solution there. :) I've been trying to solve this without using the Promise global, though, just as a demo of what ES7 is capable of. But it's certainly shorter this way.

If the comprehension syntax is final yet, then no worries about adding it to this library. And thanks for the explanation!

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Aug 4, 2015

Thank you for reporting this issue and appreciate your patience. We've notified the core team for an update on this issue. We're looking for a response within the next 30 days or the issue may be closed.

ghost commented Aug 4, 2015

Thank you for reporting this issue and appreciate your patience. We've notified the core team for an update on this issue. We're looking for a response within the next 30 days or the issue may be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment