forEach return value

[kriskowal]

As written today, the return value of forEach is undefined. I believe this would miss an opportunity to return the value from the final {done: true, value} iteration result of the underlying iteration such that:

const ten = (function *() {
  return 10;
})().forEach();
ten === 10 // true

[zloirock]

It's inconsistency with Array / Set / Map .forEach methods.

[zloirock]

However, since they have no way to pass a value at the end of the iteration, maybe it's acceptable.

[kriskowal]

The final value upon iterating any of those types is undefined, which is consistent.

[bakkot]

Iterator helpers explicitly (and intentionally) do not forward the arguments to or result from return, so even if we did this, if you wrote something like iter.map(f).return() you would necessarily see undefined.

Since the value from calling return isn't particularly part of the iterator protocol (i.e., it is explicitly ignored by for-of and ...), I am reluctant to surface it here.

[kriskowal]

I agree with that behavior and believe that is coherent with what I’m proposing.

More importantly, the iterator protocol currently does provide a vessel for observing the return value of a generator (and generator-like iterations like Agoric’s notifier streams) but currently the only way to observe that value is to hand-roll the iterator protocol. To the iterator protocol’s credit, this allowed us to implement Promise.async in user-space terms of generators, many years before async functions landed in the language.

I posit that, if the language provided any mechanism to observe the final value, it would be either the value of a for expression, or the value returned by a forEach method, and that there would be no other reasonable place to reveal it short of unrolling the iterator protocol. Also, if we decided that we missed an opportunity to reveal the final value in the return side of forEach later, it would be a breaking change.

I bring this up now because at Agoric we have a concrete case where a hypothetical forAwaitEach that returns a promise for the final value would relieve a pain-point. Agoric/agoric-sdk#5924

[bakkot]

I agree with that behavior and believe that is coherent with what I’m proposing.

Given that, maybe it would make more sense for the Generator prototype to override the forEach it inherits from the Iterator prototype? Though I suppose there's not much value in drawing that distinction, and would be more annoying for manually implemented iterators which wanted to use this part of the generator protocol.

Also, if we decided that we missed an opportunity to reveal the final value in the return side of forEach later, it would be a breaking change.

I suspect not, actually, at least for the first few years - I doubt many people would be relying on getting undefined, especially from a generator which actually didn't return undefined - but I can see the risk.

I don't think this is obviously a bad idea to include, but will need to think more about it.

a hypothetical forAwaitEach that returns a promise for the final value would relieve a pain-point

With this proposed change, that would just be forEach on an async iterator, right?

[kriskowal]

a hypothetical forAwaitEach that returns a promise for the final value would relieve a pain-point
With this proposed change, that would just be forEach on an async iterator, right?

Bearing in mind that forAwaitEach is off-topic, this is what I imagine:

In the way that for await can lift a sync iterator, I can imagine forAwaitEach being useful on sync iterators for cases with an async callback, as in:

await Number.range(100).forAwaitEach(async job => {
  // some async work to perform serially
});

Or, for bounded concurrency (10) on some (1000) jobs:

const jobs = Number.range(1000); // assuming iterator, not iterable
await Promise.all(Number.range(10).map(() => jobs.forAwaitEach(async job => {
  // some async work to perform in one of the 10 concurrent workers
}));

Suffice it to say, I’m really excited about how expressive the standard library is getting when some of the in-flight proposals mature.

[bakkot]

Ah, I think you want toAsync, which lifts a sync iterator into an async iterator.

await Number.range(100).toAsync().forEach(async job => {
  // some async work to perform serially
});

will do what you want, I believe.

[kriskowal]

That would suffice.

[zloirock]

Number.range(100) Number.range(0, 100) by the current proposal.

[ptomato]

Alternatively, forEach could return another iterator with the original values, equivalent to .map(v => { sideEffectFn(v); return v; }). This could be useful for print-debugging your iterator chains by simply inserting .forEach(console.log).

[bakkot]

@ptomato forEach needs to eagerly consume the rest of the iterator; that's what it's for. So it can't really return another iterator. You want .tap, discussed some in this thread.

[zloirock]

In the rest methods from this proposal, the final value is ignored. Why it should not be ignored in .forEach, but .map should ignore it and return { value: undefined, done: true }?

[bakkot]

Why it should not be ignored in .forEach, but .map should ignore it and return { value: undefined, done: true }?

I'm not totally sure that it shouldn't be ignored in forEach, but the cases really are meaningfully different: map is applying a transformation to the iterator, whereas forEach is consuming it. We've chosen not to expose generator-protocol stuff when applying transformations to iterators, since the transformation is only applied to a portion of the generator protocol, but I don't think that necessarily means we can't expose any of it when consuming them.

[kriskowal]

I agree that map should not forward the final iteration result. map provides a transform function for medial results but no transform for the final result, so creating a blank is appropriate.

It might be appropriate to forward the final iteration result from filter. The input and output types iterator types match at least.

To reïterate my argument for exposing the final value, the iterator protocol has a final value that is currently vestigial unless you code directly to the protocol. Unrolling the protocol is cumbersome and in my opinion, forEach is the most sensible way to expose it ergonomically, short of introducing (for) expression syntax.

[michaelficarra]

At the presentation to committee, there was no support for this change and 1 delegate preferred the status quo to match Array.prototype. Closing.