staltz / xstream

An extremely intuitive, small, and fast functional reactive stream library for JavaScript
http://staltz.github.io/xstream/
MIT License
2.38k stars 136 forks source link

Support map(fromArray).flatten() [edited title] #184

Open shfrmn opened 7 years ago

shfrmn commented 7 years ago
const s1$ = xs.of("meow")
const s2$ = xs.fromArray([1,2,3])

xs.combine(s1$, s2$).debug()
// ["meow", 1]
// ["meow", 2]
// ["meow", 3]

xs.combine(s2$, s1$).debug()
// [3, "meow"]

I apologize if the issue has already been raised before.

shfrmn commented 7 years ago

I understand why it may work this way, but I believe it shouldn't. Consider a more realistic example:

const batch$: xs<T[]> // from the network
    ,  item$: xs<T> = batch$.map(arr => xs.fromArray(arr)).flatten()

xs.combine(item$, xs.periodic(1000).take(3)).debug()
// [lastItem, 0]
// [lastItem, 1]
// [lastItem, 2]

I believe synchronously emitted values should be still passed into combine as well as async. Any reasons why not?

TylorS commented 7 years ago

This discussion has been had before, https://github.com/cujojs/most/issues/414 may be of use to you.

shfrmn commented 7 years ago

Well, I understand the concern that stream libraries will be used instead of standard array methods. However, what's the point of having .fromArray then (as well as allowing more than one argument to .of)?

shfrmn commented 7 years ago

@TylorS After a day of looking at it I believe the actual problem is bigger and not going to be solved by just fixing or removing .fromArray. Nothing is ever going to stop the user from receiving an array of data from the server as I showed in my last example. It means the user is fundamentally stuck with the ugliness of such batch streams. As far as I know currently there is no possible way of switching from Stream<T[]> to Stream<T>. Let's assume for a second .fromArray was fixed and is working properly. It immediately becomes apparent that the way I was trying to tackle the transformation using .map, .fromArray and then .flatten is still very ugly.

I really appreciate you trying to keep the API minimal, however I would like to suggest that the solution is some kind of .flattenSync operator. It would transform a stream of arrays into a stream of their items. This in combination with removing .fromArray and limiting the number of .of arguments will perhaps eliminate the root of the problem.

Have a nice day!

staltz commented 7 years ago

Hi :) Thanks for letting us know. Yes, it is complicated as @TylorS explained it's not easily supported in stream libraries (xstream, most.js, rxjs). The only one I know which supports this is RxJS v4 which uses a trampoline scheduler, but it's a big tax on performance, debuggability and is arguably on the border of the use case for push/reactive streams. We can find a way of converting Stream<T[]> to Stream<T>, but the issue with afterwards combine will probably persist. In the meanwhile, I recommend looking at ES6 Iterables and functional collections like IxJS, which more properly handle arrays and such.