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

Implement support for async generator functions and for-await statements #3473

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
10 participants
@zenparsing
Contributor

zenparsing commented Apr 18, 2016

This PR depends on babel/babylon@b926e40. Tests will fail until the next babylon release, but submitting early for feedback.

This change implements the async iteration proposal, currently at stage 2. It includes the following features:

  • Transforms async generator functions (async function* g() { }) to wrapped generator functions, similar to the current async-to-generator transform.
  • Transforms for-await statements into for loops containing yield expressions.

Thanks!

Implementation Notes

Additional comments inline.

Async Generator Functions

Async generator functions are transformed into regular generator functions, wrapped with a call to a new helper function (asyncGenerator.wrap). Within the async generator function, await expressions are translated into yield expressions, where the argument is "boxed" using another helper function (asyncGenerator.await). The wrapper function detects instances of the "boxed" values and knows that it should "pump" the generator with the awaited value instead of yielding to the consumer.

Rather than duplicate the async wrapping transform in babel-helper-remap-async-to-generator, I chose to refactor that helper to allow an optional await wrapping callee.

for-await

Since for-await statements can also appear inside of regular async functions and must take into account whether or not await expressions are "boxed", the for-await translation is done inside babel-helper-remap-async-to-generator. Most of the for-await transform logic comes from the existing for-of transformer.

(function () {
_this2;
});
() => {

This comment has been minimized.

@zenparsing

zenparsing Apr 18, 2016

Contributor

I looks to me like the current async-to-generator transform is unnecessarily transforming arrows that are nested within nested function declarations and expressions.

Is there some reason why arrows nested within a nested function would need to be translated?

@zenparsing

zenparsing Apr 18, 2016

Contributor

I looks to me like the current async-to-generator transform is unnecessarily transforming arrows that are nested within nested function declarations and expressions.

Is there some reason why arrows nested within a nested function would need to be translated?

@zenparsing zenparsing referenced this pull request Apr 19, 2016

Closed

Babel support #25

1 of 2 tasks complete

@hzoo hzoo added this to the 6.x milestone Apr 21, 2016

@jprichardson

This comment has been minimized.

Show comment
Hide comment
@jprichardson

jprichardson Apr 30, 2016

Since this functionality was in Babel v5, any idea when this will make it into the current Babel?

jprichardson commented Apr 30, 2016

Since this functionality was in Babel v5, any idea when this will make it into the current Babel?

@hzoo

This comment has been minimized.

Show comment
Hide comment
@hzoo
Member

hzoo commented May 4, 2016

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io May 6, 2016

Current coverage is 87.84%

Merging #3473 into master will increase coverage by +<.01%

  1. 5 files (not in diff) in packages were modified. more
    • Misses +2
    • Hits +3
  2. 3 files (not in diff) in packages were created. more
@@             master      #3473   diff @@
==========================================
  Files           194        197     +3   
  Lines         10119      10173    +54   
  Methods        1535       1544     +9   
  Messages          0          0          
  Branches       2245       2253     +8   
==========================================
+ Hits           8886       8936    +50   
- Misses         1233       1237     +4   
  Partials          0          0          

Powered by Codecov. Last updated by 2607f35...f19c10d

codecov-io commented May 6, 2016

Current coverage is 87.84%

Merging #3473 into master will increase coverage by +<.01%

  1. 5 files (not in diff) in packages were modified. more
    • Misses +2
    • Hits +3
  2. 3 files (not in diff) in packages were created. more
@@             master      #3473   diff @@
==========================================
  Files           194        197     +3   
  Lines         10119      10173    +54   
  Methods        1535       1544     +9   
  Messages          0          0          
  Branches       2245       2253     +8   
==========================================
+ Hits           8886       8936    +50   
- Misses         1233       1237     +4   
  Partials          0          0          

Powered by Codecov. Last updated by 2607f35...f19c10d

@zenparsing

This comment has been minimized.

Show comment
Hide comment
@zenparsing

zenparsing May 6, 2016

Contributor

Can someone help explain the failing "codecov/changes" check? I can quite figure out what's going on there.

Contributor

zenparsing commented May 6, 2016

Can someone help explain the failing "codecov/changes" check? I can quite figure out what's going on there.

@hzoo

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo May 6, 2016

Member

I think there are some issues with codecov (recently), I wouldn't worry about it atm

Member

hzoo commented May 6, 2016

I think there are some issues with codecov (recently), I wouldn't worry about it atm

@zenparsing

This comment has been minimized.

Show comment
Hide comment
@zenparsing

zenparsing May 6, 2016

Contributor

OK. It seems like we should have some eval tests, but I'm not sure if that's really possible given the async nature of this transform? Are there any functional tests for async functions?

Contributor

zenparsing commented May 6, 2016

OK. It seems like we should have some eval tests, but I'm not sure if that's really possible given the async nature of this transform? Are there any functional tests for async functions?

@hzoo

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo May 13, 2016

Member

I don't think there have been - only https://github.com/babel/babel/blob/master/packages/babel-plugin-transform-async-to-generator/test/fixtures/regression/T6882/exec.js which was a test for hoisting http://phabricator.babeljs.io/T6882. Otherwise I would check out the existing exec.js tests

Member

hzoo commented May 13, 2016

I don't think there have been - only https://github.com/babel/babel/blob/master/packages/babel-plugin-transform-async-to-generator/test/fixtures/regression/T6882/exec.js which was a test for hoisting http://phabricator.babeljs.io/T6882. Otherwise I would check out the existing exec.js tests

@jprichardson

This comment has been minimized.

Show comment
Hide comment
@jprichardson

jprichardson May 31, 2016

Anymore progress on this? This unlocks a lot of potential and code broke since it was in Babel v5 :(

jprichardson commented May 31, 2016

Anymore progress on this? This unlocks a lot of potential and code broke since it was in Babel v5 :(

@baslr

This comment has been minimized.

Show comment
Hide comment
@baslr

baslr Jun 6, 2016

Then, finally we can iterate nicely over a database query cursor

baslr commented Jun 6, 2016

Then, finally we can iterate nicely over a database query cursor

@hzoo hzoo modified the milestones: 6.x, Minor Jun 13, 2016

@jprichardson

This comment has been minimized.

Show comment
Hide comment
@jprichardson

jprichardson Jun 14, 2016

Is there anything I can help with here to get this integrated into a Babel release?

jprichardson commented Jun 14, 2016

Is there anything I can help with here to get this integrated into a Babel release?

@zenparsing

This comment has been minimized.

Show comment
Hide comment
@zenparsing

zenparsing Jun 14, 2016

Contributor

@jprichardson There a failing code coverage test which I can't make heads or tails of, and there really ought to be some sort of "exec" tests, but I haven't had time to sit down and figure out how to make that work for async operations.

Contributor

zenparsing commented Jun 14, 2016

@jprichardson There a failing code coverage test which I can't make heads or tails of, and there really ought to be some sort of "exec" tests, but I haven't had time to sit down and figure out how to make that work for async operations.

@hzoo

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Jun 21, 2016

Member

@zenparsing Don't worry about the coverage issue like I mentioned above

Member

hzoo commented Jun 21, 2016

@zenparsing Don't worry about the coverage issue like I mentioned above

@hzoo hzoo modified the milestones: Next Minor, Minor Jun 22, 2016

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jun 24, 2016

Just found out that Babel does NOT transpile arr.includes to ES5 arr.indexOf.

ghost commented Jun 24, 2016

Just found out that Babel does NOT transpile arr.includes to ES5 arr.indexOf.

@loganfsmyth

This comment has been minimized.

Show comment
Hide comment
@loganfsmyth

loganfsmyth Jun 24, 2016

Member

@wifiextender Correct, you load babel-polyfill for that. This has nothing to do with this thread though.

Member

loganfsmyth commented Jun 24, 2016

@wifiextender Correct, you load babel-polyfill for that. This has nothing to do with this thread though.

@tonyxiao

This comment has been minimized.

Show comment
Hide comment
@tonyxiao

tonyxiao Jul 10, 2016

According to transform-regenerator docs we already support async generators? https://github.com/babel/babel/blob/master/packages/babel-plugin-transform-regenerator/README.md

tonyxiao commented Jul 10, 2016

According to transform-regenerator docs we already support async generators? https://github.com/babel/babel/blob/master/packages/babel-plugin-transform-regenerator/README.md

@Kovensky

This comment has been minimized.

Show comment
Hide comment
@Kovensky

Kovensky Jul 11, 2016

Member

@tonyxiao that is if you use the regenerator runtime. This seems to be for use with native generators.

Member

Kovensky commented Jul 11, 2016

@tonyxiao that is if you use the regenerator runtime. This seems to be for use with native generators.

@tonyxiao

This comment has been minimized.

Show comment
Hide comment
@tonyxiao

tonyxiao Jul 11, 2016

I see. @Kovensky got it thanks.

tonyxiao commented Jul 11, 2016

I see. @Kovensky got it thanks.

@alexeyraspopov

This comment has been minimized.

Show comment
Hide comment
@alexeyraspopov

alexeyraspopov Sep 7, 2016

@zenparsing, do you still have plans to finish this? It will be awesome!

alexeyraspopov commented Sep 7, 2016

@zenparsing, do you still have plans to finish this? It will be awesome!

@jprichardson

This comment has been minimized.

Show comment
Hide comment
@hzoo

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Sep 7, 2016

Member

Yes but we need a code review and some more exec tests as mentioned above - unless we want users to be our testers 😛

Member

hzoo commented Sep 7, 2016

Yes but we need a code review and some more exec tests as mentioned above - unless we want users to be our testers 😛

@domenic

This comment has been minimized.

Show comment
Hide comment
@domenic

domenic Sep 15, 2016

FYI, we're hoping to push the async iteration proposal to stage 3 at the next TC39 meeting (in two weeks' time). I totally understand if the Babel maintainers don't have the time to take this pull request over the finish line, but it would be ideal if Babel supported such a stage 3 feature.

domenic commented Sep 15, 2016

FYI, we're hoping to push the async iteration proposal to stage 3 at the next TC39 meeting (in two weeks' time). I totally understand if the Babel maintainers don't have the time to take this pull request over the finish line, but it would be ideal if Babel supported such a stage 3 feature.

@hzoo

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Sep 18, 2016

Member

I'l try to take a look this week or at least finish up the tests so we can get this used/tested

Member

hzoo commented Sep 18, 2016

I'l try to take a look this week or at least finish up the tests so we can get this used/tested

@hzoo

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Sep 24, 2016

Member

@zenparsing can you fixup the merge conflicts? If I don't get around to the tests we'll just merge as is so we can get something out there like @domenic said

Member

hzoo commented Sep 24, 2016

@zenparsing can you fixup the merge conflicts? If I don't get around to the tests we'll just merge as is so we can get something out there like @domenic said

@hzoo hzoo self-assigned this Sep 24, 2016

@hzoo

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Sep 26, 2016

Member

Going to merge manually in another pr

Member

hzoo commented Sep 26, 2016

Going to merge manually in another pr

@hzoo

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Sep 27, 2016

Member

Made #4576, feel free to comment there. (fixed merge conflicts, added async exec test)

Member

hzoo commented Sep 27, 2016

Made #4576, feel free to comment there. (fixed merge conflicts, added async exec test)

@hzoo hzoo closed this Sep 27, 2016

@hzoo

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Sep 27, 2016

Member

Thanks @zenparsing, sorry for the super long delay

Member

hzoo commented Sep 27, 2016

Thanks @zenparsing, sorry for the super long delay

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