tc39 / proposal-array-from-async

Draft specification for a proposed Array.fromAsync method in JavaScript.
http://tc39.es/proposal-array-from-async/
BSD 3-Clause "New" or "Revised" License
179 stars 13 forks source link

Sync iterator’s and array-like objects’ values are not awaited #9

Closed js-choi closed 3 years ago

js-choi commented 3 years ago

On 64a49214f6a7036e7622ff4b4b9f938e277ae7ae, @zloirock left this comment:

this fallback will not properly work with sync iterators.

await Array.fromAsync([Promise.resolve(1), 2, Promise.resolve(3)]); // => [Promise.resolve(1), 2, Promise.resolve(3)]

Something like that could be simpler.

Let usingAsyncIterator be ? GetMethod(asyncItems, @@asyncIterator).
Let usingIterator be undefined.
If usingAsyncIterator is undefined,
  Let usingIterator be ? GetMethod(asyncItems, @@Iterator).
  If usingIterator is undefined,
    Let usingIterator be ? %Array.prototype.values%.
...
If usingAsyncIterator is not undefined,
  Let iteratorRecord be ? GetIterator(asyncItems, async, usingAsyncIterator).
Else,
  Let syncIteratorRecord be ? GetIterator(asyncItems, sync, usingIterator).
  Let iteratorRecord be ? CreateAsyncFromSyncIterator(syncIteratorRecord).

The problem is that the current specification will not properly await each value yielded from a synchronous iterator. (We do want await to be called on each value from a synchronous iterator—this is what for await does.)

This problem is due to how the spec currently incorrectly creates an synchronous iterator. Step 3.e.iii assigns iteratorRecord to GetIterator(asyncItems, async, usingAsyncOrSyncIterator), which results in a synchronous iterator that does not call await on each of its yielded values. What we need to do is copy GetIterator when it is called with an async hint. We need to call CreateAsyncFromSyncIterator on the created sync iterator, which in turn will create an Async-from-Sync Iterator object, whose next method will call await on each of its values. (Thanks @bakkot for the help).

I will fix this later.

zloirock commented 3 years ago

The same problem is with non-iterable array-like objects. The simplest solution above -)

js-choi commented 3 years ago

I’m a little confused. How does the current algorithm not work with non-iterable array-like objects?

If asyncItems is a non-iterable array-like object, then GetMethod(asyncItems, @@asyncIterator) and GetMethod(asyncItems, @@iterator) would be both undefined, which makes usingAsyncOrSyncIterator undefined, which means step 3.e is skipped and step 4.f etc. are executed.

zloirock commented 3 years ago

The current algorithm doesn't await the value (async-from-sync iterator behavior) and the result of the callback.


await Array.fromAsync({ 0: Promise.resolve(1), 1: 2, 2: Promise.resolve(3), length: 3 }); // => [Promise.resolve(1), 2, Promise.resolve(3)]

await Array.fromAsync({ 0: 1, 1: 2, 2: 3, length: 3 }, async (it) => it); // => [Promise.resolve(1), Promise.resolve(2), Promise.resolve(3)]
js-choi commented 3 years ago

Aha, that’s what you mean. I understand now—thank you for explaining. I will fix this when I can later.