-
Notifications
You must be signed in to change notification settings - Fork 3
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
should AsyncIterator.zip
await values yielded from sync iterators?
#24
Comments
Strong preference for awaiting |
@conartist6 say more about why? |
In my mind the arguments to I think I would favor not awaiting if async iterators themselves weren't defined to await on both const iter = [Promise.resolve("awaited")];
for await(const value of iter) console.log(value);
// awaited |
That it's a pain to sidestep this awaiting (when you want to) is no more or less true with |
This is the only static method other than |
To me that has nothing to do with belonging to a different category of methods that should have fundamentally different input types. The reasons why |
I think that it's pretty natural to treat the receiver and arguments differently. For example, methods on the prototype (like This is the only place other than |
What I hear you saying is that so far the language has never anywhere broken the rule that it awaits promises in async iteration APIs. |
No? I don't know how you got that from what I said. And in any case, the point is that we're passing a sync iterable here. We could choose to |
I'm not going to link to it because I don't think we want the attention, but this echos the monadic argument in The ship sailed on monadic promises a decade ago so I think the most consistent thing is for |
That argument applies to |
It should work by promoting a sync iterator to async because there is a natural linguistic/ontological expectation that the "The definition of |
Moreover if you don't define a standard method that means "perform the zip operation on async streams" it's really going to hurt, and it would hurt that that name doesn't imply that implementation |
This method will perform the natural zip operation on async iterables either way. Again, the only question here is how to handle sync inputs. There are multiple sensible ways to generalize the natural zip operation on async iterables to handle sync inputs. |
I would expect every item to be implicitly wrapped in AsyncIterator.from. |
How do you figure? If adding an a-priori coercion to an async iterable changes the result, it seems clear to me that the method does not treat its arguments as async iterables. |
There will be no coercion of async iterables. We're only discussing what to do with sync iterables. |
If the inputs are conceputally async, then sync iterables would be coerced. Do we agree on that? The main question is whether the inputs are conceptually async or whether they are conceptually understood to be a mixture of sync and async iterables. |
Or maybe I should phrase it as the inverse? Coercing sync iterables creates an accurate, minimal conceptual model for the method's behavior |
I'd also remind that while it's easy to imagine this: AsyncIterator.zip([asyncIterable, AsyncIterator.wrap(syncIterable)]); In practice the more common place the issue will crop is in code with syntactic structure more like this: AsyncIterator.zip(iterables); So to be safe in such a situation you would always need to write: AsyncIterator.zip(iterables.map(AsyncIterables.wrap)); which at least is less wordy than most efficient/correct implementation (which nobody is going to bother to write out): AsyncIterator.zip(iterables.map(iter => {
return iterable[Symbol.asyncIterator] ? iter : AsyncIterable.wrap(iter);
})); Say you're an intern or junior dev and you read that last example without knowing why I wrote it, would you have any idea what I was trying to accomplish? |
One last thought: when you're designing an API in which different behaviors might be correct depending on the intentions of the user but the intentions of the user are not possible to deduce, then the only safe API design choice is to make the behavior simple and predictable so that the user is given a chance to declare their intention with knowledge of the choice they are making |
No. Sync iterables would be rejected. If sync iterables are accepted, then it's possible they're coerced, or it's possible they're handled explicitly, like anything else which accepts inputs of multiple types.
I would guess you're trying to coerce sync iterables to async ones. Why? Well, knowing that would require knowing more details of the behavior of zip than an junior dev could reasonably be expected to know, but at least it gives you something to google. But consider the reverse, where someone has to write
The behavior is simple either way. It can't be fully predictable because both behaviors are reasonable and there will certainly be some people who expect each. In such cases, a useful question is "which behavior allows those who are surprised to most easily notice and recover from getting the wrong behavior?", and I think that points in the direction of not unboxing promises, because it's much easier to recover from that if you expected it to unbox than it is to recover the unboxing behavior if you expected it not to. |
Your example doesn't make any sense to me: AsyncIterator.zip(iterables.map(x => [x])).map(y => y.map(x => x[0])) a simpler way to write that would be: Iterator.zip(iterables); |
...also you're not mapping over the individual iterables, just the array of them, so I think you really meant AsyncIterator.zip(iterables.map(iter => iter.map(v => [v]))).map(values => values.map(v => v[0])) If I'm correct about what you meant here is my amended example of how I would write it: const getIterator = obj => obj[Symbol.asyncIterator]?.() || obj[Symbol.iterator]?.();
Iterator.zip(iterables.map(getIterator)); It think that's a good bit less confusing than using an async method just to undo all the asyncness and return results synchronously. My way also has a lot less overhead. |
AsyncIterator.zip([asyncIterator, syncIterable])
should presumably work (once we getIterator.zip
).for await (let val of [Promise.resolve(0)])
will await the promise, binding0
toval
. I think necessarily that means thatAsyncIterator.from([Promise.resolve(0)])
must as well, such thatAsyncIterator.from([Promise.resolve(0)]).forEach(console.log)
will print0
. This preserves the invariant that async iterables never yield promises.But for
zip
, we don't necessarily have to match that behavior, because the results are boxed, not yielded directly, so the invariant is upheld either way. Question, then: shouldzip
await
values yielded by sync iterators?On the one hand, "it promotes to async iterator, and then zips that" is the obvious intuition, which suggests that it should
await
.On the other hand, that behavior is very easy to recover if it does not
await
values from the zipped things (simply do theAsyncIterator.from
yourself), whereas recovering the "not await" behavior is pretty annoying if it doesawait
them.Thoughts? @michaelficarra expressed a preference for not
await
ing.The text was updated successfully, but these errors were encountered: