Should it await next.value when not passing a mapper function?

[nicolo-ribaudo]

Consider this async iterable:

const it = {
  [Symbol.asyncIterator]() {
    return {
      async next() {
        if (i > 2) return { done: true };
        i++;
        return { value: Promise.resolve(i), done: false }
      }
    }
  }
}

Unless I'm reading the spec wrong, await Array.fromAsync(it) returns [Promise.resolve(1), Promise.resolve(2), Promise.resolve(3)] while await Array.fromAsync(it, x => x) returns [1, 2, 3].

Currently, when using Array.from, x => x is the "default mapper function" (even if it's not specified as such): Array.from(something) is always equivalent to Array.from(something, x => x). However, there is no "default function" that can be passed to Array.fromAsync.

If we changed Array.fromAsync(something) to await the next.value values, await Array.fromAsync(it) would always return the same result as await Array.fromAsync(it, x => x). In practice, it means making it behave like

const arr = [];
for await (const nextValue of it) arr.push(await nextValue);

[nicolo-ribaudo]

I just noticed that the readme says (emphasis is mine)

mapfn is a mapping callback, which is called on each value yielded from the input – the result of which is awaited then added to the array. Unlike Array.from, mapfn may be an async function.) By default, this is essentially an identity function.

[ljharb]

If the default is identity, then explicitly passing identity must of course produce the same values. My intuition is that if the explicit identity awaits, then the implicit one also should.

[js-choi]

Yes, good catch; I agree. The value of each iteration should be awaited when no mapping function is given. This is a spec bug, and I will fix it soon.

[zloirock]

Iteration helpers do not await in similar cases.

In for-await-of, nextValue is a promise, required explicit awaiting.

I think that it should be agreed upon between both proposals.

[zloirock]

tc39/proposal-iterator-helpers#168

[zloirock]

@js-choi it's better to leave it open until aligning it with the iterator helpers proposal.

[js-choi]

For what it’s worth, the current spec (as of #20) causes each input value in Array.fromAsync([ 0, 1, 2 ]) to be Awaited twice (since it is necessarily equivalent to Array.fromAsync([ 0, 1, 2 ], x => x): Awaiting the input value then Awaiting the result of x => x). I consider this double-Awaiting in this narrow case (sync-from-async iteration with default mapping function) acceptable behavior.

[brad4d]

I'm hoping that this bit of code clarifies what this issue is about.

// Draft polyfill
Array.asyncFrom = async function(iterable, mapFn = x => x, thisArg = null) {
  const result = [];
  for await (const value of iterable) {
    // NOTE: If the iterable produces a Promise, `value` will be its resolved
    // value, because the for-await syntax awaits it.
    // See: https://github.com/tc39/proposal-async-iteration/issues/15
    const mapFnResult = mapFn.call(thisArg, value);

    // This issue (https://github.com/tc39/proposal-array-from-async/issues/19)
    // is about deciding whether the `await` should appear here or not.
    result.push(await mapFnResult);
  }
  return result;
}

Assuming I haven't gotten this wrong, I think we should have the await shown above, because that is the most consistent with the behavior of for-await, which awaits the individual elements it is iterating over.

[js-choi]

Some representatives at the plenary a few days ago stated that they wanted to avoid double awaiting and optimize for the more-common case of omitting the mapping function, so we will need to revisit this issue.

At the very least, we will need to do a thorough comparison of every choice we have, before presenting to plenary again.

[RubyTunaley]

I mean the way I see it there's only really 5 options:

Only await mapped values if they are promise-like

This requires a check for a then property with type 'function' on each returned mapped value, but if omitting mapfn is like 95% of cases then this might be fine, since when undefined is passed the runtime can branch into a fast path that doesn't do that check.

The type check is there to alleviate this concern:

For instance, when we don't supply amapping call back and we have an AC and DC link input. We can, we can not await anything. Which we can't because we want some sort of identity callback equivalency.

- JSC, TC39 Meeting Notes, January 2022

This solution seems to be the one the committee was leaning towards, although I'm unsure whether they would be happy with the behaviour of a mapfn like x => Math.random() < 0.5 ? x : Promise.resolve(x).

Have null and undefined mean different things

• If the mapfn parameter is undefined, execute with no mapping and without double-await.
• Else:
  • If the mapfn parameter is null, set mapfn to x => x.
  • Execute with mapping and with double-await.

Array.from currently doesn't accept null for mapfn so it would be a good idea to modify it so that it does too, although in that case it can have the same behaviour as undefined.

Split the function

The function could be split into Array.fromAsync and something like Array.fromAsyncMap, but that's inconsistent with how Array.from works and it can't be fixed without either breaking the web or breaking the parameter symmetry between from and fromAsync.

Drop mapping

Just get developers to write (await Array.fromAsync(blah)).map(x => x) (or with iterator-helpers await Array.fromAsync(blah.map(x => x))).

Are there any stats available for how often Array.from's mapfn parameter is even used?

Accept either double-awaiting everything or having undefined behave differently from x => x.

The committee has made its opinion clear on double-awaiting everything, but having undefined behave differently is unintuitive (which is why the null option exists).

[bakkot]

await Array.fromAsync(it) returns [Promise.resolve(1), Promise.resolve(2), Promise.resolve(3)] while await Array.fromAsync(it, x => x) returns [1, 2, 3]

This seems like the right answer to me. And Array.fromAsync(it, x => { console.log(x); return 0; }) would print three Promises and return [0, 0, 0].

I am OK with having undefined behave differently from x => x, because x => x isn't acting as the identity function here - its result is being awaited. (One way to look at this is that the second argument is an async function [edit: rather, to be precise, it's a function composed with await], even if you happen to write it as a sync function, and there isn't an identity async function.)

[bakkot]

Note that for await does not await these values:

let i = 0;
let it = {
  [Symbol.asyncIterator]() {
    return {
      async next() {
        if (i > 2) return { done: true };
        i++;
        return { value: Promise.resolve(i), done: false }
      }
    }
  }
};

(async () => {
  for await (let v of it) {
    console.log(v); // prints a promise
  }
})();

I really think we should match that behavior when not passing the second argument.

[js-choi]

I think @bakkot gives some persuasive points, especially that the mapping function is actually essentially an async function, so it wouldn’t make sense for its identity to be x => x.

My priorities, in order, have always been:

1. Array.fromAsync(i) must be equivalent to Array.fromAsync(i, undefined) and Array.fromAsync(i, null). (For optional parameters, nullish arguments should be functionally equivalent to omitting the arguments. This is how every function in the core language is designed, and I believe it is also an explicit best practice in Web APIs.)

2. Array.fromAsync(i) must be equivalent to for await (const v of i). (The default case of fromAsync must match intuitions about for await (of), just like how from matches intuitions about for (of).)

3. Array.fromAsync(i) should be equivalent to AsyncIterator.from(i).toArray().

4. Array.fromAsync(i, f) should be equivalent to AsyncIterator.from(i).map(f).toArray().

5. Array.fromAsync(i, f) should conceptually but not strictly be equivalent to Array.from(i, f).

I lost sight of the second priority when I wrote #20.

Bakkot points out that the default mapping function of Array.fromAsync does not have to be x => x, and omitting the mapping function does not have to be equivalent to specifying x => x or some other function.

Therefore, I plan to revert my changes in #20. The default behavior without a mapping function will be to not await values yielded by async iterators. When a mapping function is given, the inputs supplied to the mapping function will be the values yielded by the input async iterator without awaiting; only the results of the mapping function will be awaited. This behavior should match AsyncIterator.prototype.toArray.

function createIt () {
  return {
    [Symbol.asyncIterator]() {
      let i = 1;
      return {
        async next() {
          if (i > 2) {
            return { done: true };
          }
          i++;
          return { value: Promise.resolve(i), done: false }
        },
      };
    },
  };
}

Without any mapping function:

result = [];
for await (const x of createIt()) {
  console.log(x);
  result.push(x);
}
// result is [ Promise.resolve(1), Promise.resolve(2), Promise.resolve(3) ].

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

With mapping function x => x:

result = await Array.fromAsync(createIt(), x => x);
// result is [ 1, 2, 3 ].

result = await AsyncIterator.from(createIt())
  .map(x => x)
  .toArray();
// result is [ 1, 2, 3 ].

With mapping function x => (console.log(x), x):

result = await Array.fromAsync(createIt(), x =>
  (console.log(x), x));
// Prints three promises.
// result is [ 1, 2, 3 ].

result = await AsyncIterator.from(createIt())
  .map(x => (console.log(x), x))
  .toArray();
// Prints three promises.
// result is [ 1, 2, 3 ].

With mapping function async x => (console.log(await x), await x)):

result = await Array.fromAsync(createIt(), async x =>
  (console.log(await x), await x));
// Prints 1, 2, then 3.
// result is [ 1, 2, 3 ].

result = await AsyncIterator.from(createIt())
  .map(async x => (console.log(await x), await x))
  .toArray();
// Prints 1, 2, then 3.
// result is [ 1, 2, 3 ].