baconjs/bacon.js — pull request #361 by skozin (2014-04-20 22:54:29+00:00)
Title: Improve reentrance of the side effects dispatching code (v.1)

Consider this code:

```coffee
bus = new Bacon.Bus
a = bus.map 'a'
b = bus.map 'b'

c = Bacon.combineAsArray a, b
d = Bacon.combineAsArray c, b

sibling = new Bacon.Bus

c.onValue ->
console.log '---> c'
sibling.push 123
console.log '<--- c'

d.onValue -> console.log '- d'
sibling.onValue -> console.log '- sibling'

bus.push 'test'
```

... which outputs the following:

```text
---> c
- d
- sibling
<--- c
```

While it is great that the side effects assigned to `sibling` can be observed immediately after the push, I don't like the invocation of the side effects of `d` in the middle of a `c.onValue` handler.

This happens because, in the current implementation, there is a single queue of "afters" (side effects), and it is flushed immediately and completely as a result of each new root event.

This PR improves the described behavior by introducing a stack of afters. At the beginning of each transaction, a new frame (empty array) is pushed onto the stack, and all afters generated during the transaction are collected in that frame. When the transaction ends, (only) the afters from it's frame are processed. The output of the above code will change to this:

```text
---> c
- sibling
<--- c
- d
```

But this change breaks [one test](https://github.com/baconjs/bacon.js/blob/master/spec/BaconSpec.coffee#L1690-L1700) from the spec (slightly edited for clarity):

```coffee
describe "When an Event triggers another one in the same stream, while dispatching", ->
it "Delivers triggered events correctly", ->
bus = new Bacon.Bus
values = []
bus.take(2).onValue (v) ->
bus.push v + "A"
bus.push v + "B"
bus.onValue (v) ->
values.push(v)
bus.push "x"
bus.push "y"
expect(values).to.deep.equal(["x", "xA", "xAA", "xAB", "xB", "y"])
```

In this branch, the order of invocations of the second `bus.onValue` handler is different:

```coffee
["xAA", "xAB", "xA", "xB", "x", "y"]
```

Does the order documented in the test makes sence? More specifically, given the following code:

```coffee
bus = new Bacon.Bus
f = (v) -> bus.push(v + '|f')
g = (v) ->
bus.take(2).onValue(f)
bus.onValue(g)
bus.push('x')
```

... why do we need `g('x')` to happen before `f('x|f')`? Can you give an example when this is desired?

In the case it is really important, here is another PR: PR2. In addition to introducing the stack, it implements flushing of all stack frames created by Observables that depend on the Observable that generated a new root event. Thus, when a side effect generates an event that will cause (synchronous) recursion, the old behavior is preserved. Compared to this PR, the code is more complex and includes dependency checking.

The performance of the common case (no events generated by side effects) doesn't seem to be affected by either of the PRs.

Together with #356, this PR improves Bacon's reliability regarding the order of side effects. I understand that most of the time you shouldn't rely on their order, but sometimes it is really important. Especially when you have no control over their internal logic.