Closed nicolo-ribaudo closed 2 years ago
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.
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.
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.
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.
@js-choi it's better to leave it open until aligning it with the iterator helpers proposal.
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.
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.
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.
I mean the way I see it there's only really 5 options:
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)
.
null
and undefined
mean different thingsmapfn
parameter is undefined
, execute with no mapping and without double-await.mapfn
parameter is null, set mapfn
to x => x
.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
.
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
.
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?
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).
await Array.fromAsync(it) returns [Promise.resolve(1), Promise.resolve(2), Promise.resolve(3)]
whileawait 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.)
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.
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:
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.)
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)
.)
Array.fromAsync(i)
should be equivalent to AsyncIterator.from(i).toArray()
.
Array.fromAsync(i, f)
should be equivalent to AsyncIterator.from(i).map(f).toArray()
.
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 ].
Consider this async iterable:
Unless I'm reading the spec wrong,
await Array.fromAsync(it)
returns[Promise.resolve(1), Promise.resolve(2), Promise.resolve(3)]
whileawait 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 toArray.from(something, x => x)
. However, there is no "default function" that can be passed toArray.fromAsync
.If we changed
Array.fromAsync(something)
to await thenext.value
values,await Array.fromAsync(it)
would always return the same result asawait Array.fromAsync(it, x => x)
. In practice, it means making it behave like