-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Normative: avoid mostly-redundant await
in async yield*
#2819
Conversation
yield*
await
in async yield*
e1b848b
to
4978c21
Compare
Tests: tc39/test262#3619 |
My understanding was that
and I understand this changes that. I think this is a footgun. Imagine you had the for-await version because you had a bit of logic before yielding and then you removed that logic. One might think that you would be able to simplify the code by replacing the for-await with yield*, which was working before, but I understand this will not work anymore. You would need to know how inner was implemented which is not a good idea. |
No, they're different in other ways - More importantly, though, this basically should not come up. Syntactic async generators, as well as the async-from-sync wrapper, will prevent you from having a Promise in the By a similar token, you might think that using Also, even if you are just reasoning by analogy to other code, |
Nice explanation! I think it's unfortunate that we use the same keyword ('yield') for these 2 different ways of producing the resulting value. |
* Extract asyncGeneratorDelegate/AsyncGenerator/awaitAsyncGenerator (no changes) * Remove one promise tick in yield* (tc39/ecma262#2819) * [babel 8] Don't pass the second parameter to asyncGeneratorDelegate
Spec PR: tc39/ecma262#2819 The PR moves the `Await` operation of out `AsyncGeneratorYield` and instead executes it before calling `AsyncGeneratorYield` from `Yield`. `BytecodeEmitter::emitYieldStar()` has an inlined implementation of the `AsyncGeneratorYield` operation. Removing the call to `emitAwaitInInnermostScope()` is equivalent to removing the `Await` operation in `AsyncGeneratorYield`. And in `BytecodeEmitter::emitYield()` we only need to remove the comment explaining why we call `emitAwaitInInnermostScope()`, because executing `Await` is now part of the normal evaluation of the `Yield` expression and no longer happens in `AsyncGeneratorYield`. Differential Revision: https://phabricator.services.mozilla.com/D156933
4978c21
to
99b176b
Compare
This is an alternative to #2818. See that PR for more context - this implements option 3 of the options listed there. Fixes #2813.
The main effect of this PR would be that if you had
then you would see printed
Promise.resolve(0)
instead of, as currently,0
. In other words, it would behave as if you'd writtenfor await (let x of inner)
instead ofof outer()
.The other effect is a reduction in the number of promise ticks.
No difference (except in timing) would be observable without manually implementing
Symbol.asyncIterator
because async generators (and the async-from-sync wrapper, see step 6) alreadyawait
values before yielding or returning them.