Skip to content

Allow async iterators to specify return algorithms #805

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

Merged
merged 2 commits into from
Oct 16, 2019

Conversation

domenic
Copy link
Member

@domenic domenic commented Sep 25, 2019

Closes #804. Notably this allows consumers to not worry about resolving their promise with the correct iterator result.

This is on top of the stage-setting commits in #802; only the last commit is worth reviewing.

One choice I made here where the right answer wasn't immediately obvious is that calling return() while a call to next() is outstanding will reject with no side effects. This mirrors what we've done in the Streams design, and seems simpler than e.g. waiting for 1+ outstanding next()s to settle. This will only occur when manually manipulating the async iterator (as opposed to using for-await-of) so it's not a very mainline scenario. /cc @ricea in case he has any thoughts.


Preview | Diff

@marcoscaceres
Copy link
Member

Reported @fallenAngelSpeaks2Me

Copy link
Member

@Ms2ger Ms2ger left a comment

Choose a reason for hiding this comment

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

Thanks and sorry for the delay.

index.bs Outdated
@@ -4307,6 +4307,21 @@ iteration, or resolves with a tuple containing two elements:
1. a value of the first type given in the declaration;
1. a value of the second type given in the declaration.

The prose may also define an <dfn export>asynchronous iterator return</dfn> algorithm. This
algorithm receives the instance of the [=interface=] that is being iterated, the async iterator
itself, and a single argument value. This algorithm is invoked in the case of premature termination
Copy link
Member

Choose a reason for hiding this comment

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

And the single argument value is an {{any}} value?

index.bs Outdated
<emu-val>undefined</emu-val>, « |error| »).
1. Return |returnPromiseCapability|.\[[Promise]].

1. If |object|'s [=default asynchronous iterator object/ongoing promise=] is not undefined, then:
Copy link
Member

Choose a reason for hiding this comment

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

Check if this undefined needs an emu-val? I don't remember which way this one goes.

Copy link
Member Author

Choose a reason for hiding this comment

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

Mmm nice catch. Since we made it an IDL Promise, we should probably union it with null (another IDL value), not undefined (which is always <emu-val>undefined</emu-val>, a JS value). I switched all instances to do so.

Closes #804. Notably this allows consumers to not worry about resolving their promise with the correct iterator result.
@domenic domenic force-pushed the async-iterator-return branch from 82802b7 to 01691c3 Compare October 16, 2019 11:02
@domenic domenic merged commit d87d389 into master Oct 16, 2019
@domenic domenic deleted the async-iterator-return branch October 16, 2019 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Async iterators should be allowed to have an optional return() algorithm
3 participants