should flatMap flatten iterables or iterators?

[michaelficarra]

It currently flattens iterables. If X.prototype.flatMap flattens Xs, we should instead be flattening iterators.

As mentioned by @rbuckton in https://github.com/tc39/proposal-iterator-helpers/issues/117#issuecomment-1238559467.

[zloirock]

Flattening iterables is much more convenient than iterators.

[devsnek]

getting some serious deja vu from this question :smile:. I guess I would ask: what are some uses we can come up with for this flatMap method and what values are they dealing with?

[ljharb]

How does it currently flatten iterables? The spec steps seem to assume an iterator.

[bakkot]

flatMap step 3.a.vi is

vi. Let innerIterator be Completion(GetIterator(mapped, sync)).

and GetIterator consumes iterables.

[michaelficarra]

same for async flatMap step 3.a.viii

[ljharb]

gotcha - but if we removed the GetIterator call, then you wouldn't be able to return an array, right? that seems problematic - although you WOULD be able to return a string, which seems beneficial.

[bakkot]

gotcha - but if we removed the GetIterator call, then you wouldn't be able to return an array, right?

Right. You'd have to return [0, 1, 2].values().

[ljharb]

That seems like a pretty large ergonomics hit.

Now that I'm thinking about strings, though - currently, would i have to wrap it in [] to be able to flatMap over it and yield a full string? having to do [string].values() seems even worse than just array-wrapping.

[bakkot]

currently, would i have to wrap it in [] to be able to flatMap over it and yield a full string?

Yes.

This is arguably a reason to prefer only flattening iterators - if we flatten iterables, return "foo" will get spread; if we require iterators, then you have to actually think if you want "foo"[Symbol.iterator]() or [foo].values().

[ljharb]

for strings, i agree with you - but for every other value, it's a huge tax :-/

[bergus]

It currently flattens iterables. If X.prototype.flatMap flattens Xs, we should instead be flattening iterators.

As mentioned by @rbuckton in #117 (comment).

This has already been discussed about 3 years ago: https://github.com/tc39/proposal-iterator-helpers/issues/47#issuecomment-535230523, and the string problem/feature in #55. So I concur with @devsnek: Déjà vu :-)

[rbuckton]

Isn't string the only non-object iterable (if you exclude future tuples)? Would it make sense to have flatMap only flatten object iterables? You could still return a [..."foo"] if you intentionally wanted to spread a string, but I'd imagine spreading a string isn't generally what a user would intend.

Especially since each element yielded from an iterable string is, itself, also an iterable. If we were to add a .flat(), we'd likely have to special case strings anyways to avoid infinite recursion.

[rbuckton]

Also, I certainly wouldn't want to exclude iterables here. I just wonder if the behavior of flatMap shouldn't be like Iterator.from(), where you could return an iterable or a user-defined iterator that simply has a next.

[rbuckton]

Isn't string the only non-object iterable (if you exclude future tuples)? Would it make sense to have flatMap only flatten object iterables?

I just saw https://github.com/tc39/proposal-iterator-helpers/issues/55#issuecomment-541257212, but I'm not sure I agree. Strings are special, and flattening them is more than likely unintentional.

[michaelficarra]

This has already been discussed about 3 years ago

Thanks for the pointer @bergus. I had forgotten about that thread. A significant difference between then and now is that (after #55) we are now considering a flatMap that does not lift a non-flattenable value into a singleton flattenable structure like what Array.prototype.flatMap does. I am happy with that decision.

I just wonder if the behavior of flatMap shouldn't be like Iterator.from(), where you could return an iterable or a user-defined iterator that simply has a next.

I'm leaning toward this solution. What would be the downside of this?

1. If it's iterable, get an iterator and iterate that
2. If it's an object and "next" is callable, iterate the object
3. throw

[ljharb]

The downside is that you shouldn't have to wrap a value in an iterator just to be able to return it from the mapper.

[michaelficarra]

@ljharb you currently have to wrap it in an iterable, so I don't see how that is different. If you want to fight that fight, have at it in #55, but I strongly support that decision.

[ljharb]

Fair enough, although I don't think that decision has been brought to plenary yet, and it's a pretty big one.

[bergus]

I'm leaning toward this solution:

1. If it's iterable, get an iterator and iterate that
2. If it's an object and "next" is callable, iterate the object
3. throw

I see 4 variants there:

• a) test for Symbol.iterator, then test for next, else throw
• b) test for next, then test for Symbol.iterator, else throw
• c) test only for Symbol.iterator, else throw
• d) test only for next, else throw

I think the majority agrees that d) has bad developer experience (having to wrap all iterables in Iterator.from(…) or calling [Symbol.iterator]()) and is in general not what the language does in other places where it iterates anything. Between the other three, I don't see that much of a difference, given that all iterators which inherit from Iterator.prototype are iterable already. This holds true even for polyfilled iterators and most handcrafted iterators as well. For those that don't inherit from Iterator.prototype, if we did choose variant c), the rule would be simple: if you cannot invoke .flatMap() on it, you also cannot return it from the flatMap callback. I would still favor c) - which is also the current proposal - but if anyone feels strongly about non-iterable iterators, we could indeed adopt a). I would avoid variant b) though since it's slower for the common case of non-iterator iterables.

[michaelficarra]

@bergus I've opened https://github.com/tc39/proposal-iterator-helpers/pull/233 which takes variant a. Please review.

[devsnek]

I also think we should keep the currently specified semantics.