tc39 / proposal-iterator-helpers

Methods for working with iterators in ECMAScript
https://tc39.es/proposal-iterator-helpers
1.33k stars 35 forks source link

What happens if you pass a sync iterator (not iterable) to AsyncIterator.from? #237

Closed bakkot closed 2 years ago

bakkot commented 2 years ago

If you have an object { next() {...} }, and it doesn't have a Symbol.iterator or Symbol.asyncIterator method, there's no way to tell if that's sync or async.

If you pass such a thing to AsyncIterator.from, the result is supposed to be an async iterator. Right now, any time a sync iterable is passed to something which expects an async iterable, and that sync iterator yields Promises, the spec will lift from Iterator<Promise<T>> to AsyncIterator<T>, by awaiting the .value property (in %AsyncFromSyncIteratorPrototype%.next).

But here there's no way to tell if the passed thing is sync or async. So should values from the .value property be awaited or not?

That is:

let iter = { next(){ return { done: false, value: Promise.resolve(0); } } };
let aIter = AsyncIterator.from(iter);
let { value } = await aIter.next();
value // 0, or Promise.resolve(0)?
devsnek commented 2 years ago

we should assume its an async iterator, because you called AsyncIterator.from

bakkot commented 2 years ago

I feel like it's not totally unreasonable to call AsyncIterator.from because you had a sync iterator and you wanted an async iterator.

devsnek commented 2 years ago

at the risk of sounding like a shitpost, AsyncIterator.from(Iterator.from(x))?

bakkot commented 2 years ago

That does happen to work, but only because Iterator.from(x) returns an iterable. But it's not something people are going to try without being told to try it, I would think.

bergus commented 2 years ago

I had absolutely expected AsyncIterator.from to have the same behaviour as CreateAsyncFromSyncIterator. Why is there even a WrapForValidAsyncIteratorPrototype separate from the AsyncFromSyncIteratorPrototype?

bakkot commented 2 years ago

The primary purpose of AsyncIterator.from is to ensure that the result inherits from AsyncIterator.prototype, in the same way that Iterator.from ensures the result inherits from Iterator.prototype.

michaelficarra commented 2 years ago

Also of note: if the iterator has a Symbol.iterator (possibly from its prototype being Iterator.prototype), its iterable-ness will take precedence over its iterator-ness. I think it would be really strange if the result differed for those cases.

bakkot commented 2 years ago

I am tempted to resolve this by saying the from methods only take iterables, not iterators. That makes them marginally less useful for wrapping manually produced iterators; on the other hand, it's quite unusual to vend iterators, as opposed to iterables, so I don't think that would actually come up much.

michaelficarra commented 2 years ago

I've updated #233 to address this. It aligns flatMap with the respective from function.

Iterator.prototype.flatMap and Iterator.from:

AsyncIterator.prototype.flatMap and AsyncIterator.from:

Unfortunately, this does mean that async flatMap will treat sync iterators differently if they have a Symbol.iterator, indicating their sync-ness. I think this is our best option.