search

kefirjs / kefir

A Reactive Programming library for JavaScript
https://kefirjs.github.io/kefir/
MIT License
1.87k stars 97 forks source link

Incorrect erros handling in .combine #54

Closed rpominov closed 9 years ago

rpominov commented 9 years ago

Consider this example:

A:                ----1-----e-----2-----
B:                -------3-----4-----5--
combine(A + B):   -------4--e--5--6--7--

The 5 value after the error combined as 1 + 4. The 1 is the latest value from A, and combine uses it ignoring the fact that there was a error after it.

I think this behavior is not correct, combine should continue to emit errors on new values from B until there is a value in A. In other words third value should be combined from e and 4 which result in e. So the correct output should be:

combine(A + B):   -------4--e--e--6--7--

The .sampledBy method also affected by this:

A:                ----1-----e-------2----
B:                -------•-----•--•---•--
A sampled by B:   -------1--e--1--1---2--

should be:        -------1-----e--e---2--

I think we should fix this, but here is a question "is the current behavior useful for anybody?", because when it will be fixed it will be tricky to create current behavior.

rpominov commented 9 years ago

Another question is what to do if we have more than one error, for instance:

A:                ----1-----e-----2-----
                            E1
B:                -------3-----e-----5--
                               E2
combine(A + B):   -------4--e--?--e--7--
                           E1     E2

What should be in place of question mark? Options are:

1) E2 as the latest error 2) Array [E1, E2] (in this case should we wrap other errors to arrays too, so it will be [E1] -- [E1, E2] -- [E2]?)

I like second option more (with all errors wrapped to arrays).

rpominov commented 9 years ago

After a discussion in Bacon's issue https://github.com/baconjs/bacon.js/issues/548#issuecomment-90743234 I've decided to go with 1) option for now.