RFC: "Auto-wrapping" values in concatMap, mergeMap, et al?

[benlesh]

Since JavaScript introduced Array#flatMap, it follows the same sort of "auto-wrapping" (or unwrapping, depending on if you're a glass-is-half-empty person, I guess) that Promise then does, wherein it doesn't matter if you return an array or not.

I'm not saying that we SHOULD do this work, I'm asking the community to comment on it.

1. What use cases do you see?
2. What problems do you see?
3. Does it help or hurt?

It's definitely a breaking change we could only introduce in a major version.

Proposal

Any non-observable type. That is to say anything that isn't an ObservableInput (Arrays, iterables, AsyncIterators, promises, etc) would be automatically wrapped in operators like concatMap, mergeMap, et al.

source$.pipe(
  concatMap(() => 43)
)

Prior Art

Array#flatMap

And inevitably this will work with Iterable and AsyncIterable (cc @mattpodwysocki) due to the iterator helpers proposal.

[1, 2, 3].flatMap(() => 42); // [42, 42, 42]

Promise#then, Promise#catch

Promise.resolve(1).then(() => 42); // Promise<42>

Promise.reject('bad').catch(() => 42); // Promise<42>

Cons

1. Strings are iterables, and that gets confusing.

One issue I could see is strings confusing folks because they're iterables.

// they don't really type like this, just illustrating outcomes
source$.pipe(concatMap(() => 32)); // Observable<32>
source$.pipe(concatMap(() => 'hi')) // Observable<'h' | 'i'>

• Array#flatMap doesn't have this problem really because it doesn't auto-convert iterables.
• There's probably an argument to be made that RxJS shouldn't do this with strings, because it's almost always a mistake. But that's also inconsistent (and I hate inconsistent APIs, lol)

2. Adding new conversions may get harder.

During 7.x, we were able to start handling Async Iterable conversion in a non-breaking way. (Well, in retrospect it may have broken people that were trying to create an Observable<AsyncIterator<T>> or something, but that never came up, and we considered it to be too far fetched).

If another type gets added down the road, let's say they add "signals" to the platform in some sense, we'd need a major release to start handling them appropriately if we wanted to convert them automatically.

Pros

1. There would be no more "You provided an invalid object where a stream was expected" errors.

Because everything would just pass through. If you get an error, it should be thrown where you tried to use an incorrect type.

2. Eliminates most of the need for of.

Which is a notably ugly API. Honestly most people could get around of(1) on the cheap by just using [1], but whatever.

source$.pipe(
  concatMap(isCached => isCached ? of(value) : fetchValue())
)

// becomes

source$.pipe(
  concatMap(isCached => isCached ? value : fetchValue())
)

3. void could mean EMPTY at this point.

I think this means a bit less "magic" that people need to know about RxJS. But then again, another way to look at it is that void meaning "empty" is more magical.

source$.pipe(
   concatMap(n => {
     if (n > 2) {
        return someOtherObservable$
     }
     // return EMPTY
   })
)

[hoc081098]

We make our life complicated 😂

[tomastrajan]

The interaction with strings and the resulting inconsistent API with an exception case sounds like a too much of a cost to pay to go from [someVal] to someVal IMO.

[benlesh]

It's worth noting here that the TC39's async iterator helpers API will throw if an iterable primitive is returned (which is only really string at this point, but eventually tuples).

This goes back to my statement that most of the time when a string is returned in a concatMap it's probably a mistake.

[voliva]

I also think I'd like to keep the current API. Cons outweight the Pros by far:

Cons

1. Strings are iterables, and that gets confusing.
2. Adding new conversions may get harder.

Both are related - Having an inconsistent API to do something special if typeof v === "string" is just something that raises that's not a good approach. Adding support for other type of objects in the long run can be challenging.

Pros

1. There would be no more "You provided an invalid object where a stream was expected" errors.

In a way, it's a good thing we get this exception. It's annoying because it's hard to debug (because of RxJS stack traces, but that's another topic), but it's actually good to receive this error to know that somewhere you messed up and you're returning a value you're not supposed to. If RxJS "swallows" this error through the new API, who knows what could happen on other streams that depend on it.

2. Eliminates most of the need for of.

I usually just do concatMap(isCached => isCached ? [value] : fetchValue()) to avoid importing of. It's not really a drawback

[tomalaforge]

I prefer the current API. And for me, the pros are not really pros. I like the of() and the EMPTY to really see that we are working with observables and this is why we need a high order operator.

it easier to read and easier for rxjs beginners to understand.

Don't really like when things get too magical 😅

[varog-norman]

I also vote for the current API. I prefer explicitness - it's better to see what the code does instead of having to figure out why it works a certain way

[mpodwysocki]

I see no compelling argument for auto-unwrapping here as it causes more unexpected behavior than it solves. I would prefer if that the flatMap stay true to returning a sequence of sequences, and if you need to flatten that to a single stream, there's no reason you can't just use flat

[benlesh]

CORE TEAM: No for auto-unwrapping. But throwing when a string is returned might be a good idea.. Will open new discussion.