Should `flatMap()`'s `Mapper` *always* convert values to an Observable?

[domfarolino]

I discovered that iterator helpers's flatMap() does not try and coerce everything that the Mapper callback returns, into an Iterable itself (to iterate over and add primitives to the output). If the Mapper callback returns non-iterables, those just get added to the final flattened array with no more coercion than that. For example:

const obj = {
  [Symbol.iterator]() {
    console.log('Test');
    return [1, 2, 3];
  }
};

// I expected this to invoke the custom @@iterator impl.
// But it doesn't! Instead, it accepts `v` as a primitive, adding it to the final output.
[obj].flatMap(v => v);

Right, now Observable#flatMap() is spec'd such that it always tries to convert the return value of Mapper to an Observable, via the Observable#from() semantics. This means that if the mapper callback simply outputs a primitive (or something that doesn't convert to an Observable), then the final Observable outputs an error. Should we change this to the more relaxed semantics that Iterator helpers's flatMap() has, or should we keep it strict because that's the way Observables have always worked?

[bakkot]

The example that you have is invoking Array.prototype.flatMap, not iterator helper's flatMap. They behave differently. The rule we decided on was that X.prototype.flatMap should flatten Xs (though we immediately broke this rule by making Iterator.prototype.flatMap flatten both iterators and iterables).

If you try this with iterator helpers, it does in fact invoke your Symbol.iterator method:

const obj = {
  [Symbol.iterator]() {
    console.log('Test');
    return [1, 2, 3];
  }
};

[obj].values().flatMap(v => v).toArray();

(Of course, since the Symbol.iterator method is malformed in that it does not return an iterator, this throws. But it does print Test first.)

Array.prototype.flatMap and Iterator.prototype.flatMap differ in their handling of non-X values: Array.prototype.flatMap will add them to the result, and Iterator.prototype.flatMap will throw. Personally I think the behavior of Iterator.prototype.flatMap is much more reasonable; that's the one you should follow.

The reason for the behavior of Array.prototype.flatMap is to be consistent with Array.prototype.flat. Unless you're adding a .flat method, which I don't think there's any reason to do, you shouldn't accept values which aren't Observables or at least straightforwardly coerced to Observables.

[domfarolino]

OK I think that makes a lot of sense. Yeah, I suppose if an object inside the original collection is not itself an iterable, then that's precisely what the Mapper is there for, to give the developer an opportunity to turn it into one so it can be added to the final result properly.

Doing the same thing for Observables makes sense to me, so I'll keep the spec as-is, where we always expect the Mapper to return something coercible to an Observable via from() (and error out otherwise). Thanks @bakkot.