events from fromEvent + scan out of order

[Risto-Stevcev]

I have the following code:

var createStore = function(name, reducer, initialState) {
  var storeName = 'store:'+ name

  var stream = most.fromEvent(storeName, document)
    .scan(function(state, event) {
      return reducer(state, event.detail)
    }, initialState)

  var subscribe = function(callback) {
    stream.subscribe({
      next: callback
    })
  }

  var dispatch = function(action) {
    document.dispatchEvent(new CustomEvent(storeName, { detail: action }))
  }

  return {
    stream: stream,
    subscribe: subscribe,
    dispatch: dispatch
  }
}

var store = createStore('foo', function(state, action) {
  switch (action.type) {
    case 'SET_FOOBAR':
      return { foobar: state.foobar + action.foobar }
    default:
      return state
  }
}, { foobar: 1 })

store.subscribe(console.log)
store.dispatch({ type: 'SET_FOOBAR', foobar: 3 })
store.dispatch({ type: 'SET_FOOBAR', foobar: 5 })

which strangely enough prints the initial state as the last thing to the console:

Object {foobar: 4}
Object {foobar: 9}
Object {foobar: 1}

I would expect it to print the inital state as the first thing, like this:

Object {foobar: 1}
Object {foobar: 4}
Object {foobar: 9}

is this a bug? I'm trying this on 1.7.0. I've tried this code with rxjs and xstream and they both work like I expected it to

[Risto-Stevcev]

I found out that if I move it inside a setTimeout then it works like I would expect it to:

setTimeout(function() {
  store.dispatch({ type: 'SET_FOOBAR', foobar: 3 })
  store.dispatch({ type: 'SET_FOOBAR', foobar: 5 })
}, 0)

I guess there's something going on with how it queues up events in the event loop

[zkochan]

I have experienced this issue when using most.from with an observable created by zen-push. Moving pushStream.next('foo'); into a setTimeout() fixed the issue in my case as well.

[davidchase]

@Risto-Stevcev document.dispatchEvent dispatches synchronously (mdn: the event handlers run on a nested callstack: they block the caller until they complete)

so that's why you get the unexpected result until you wrap your dispatching into a setTimeout.

[zkochan]

Would be better to throw some error in cases like this. It is a silent error and really hard to track down.

[Frikki]

It is a silent error ...

It is not an error. In fact, it behaves as expected. It would be wrong to “throw some error in cases like this.”

Within a function f, a failure is an error if and only if it prevents f from meeting any of its callee’s preconditions, achieving any of f’s own postconditions, or reestablishing any invariant that f shares responsibility for maintaining.

There are three different kinds of errors:

• a condition that prevents the function from meeting a precondition (e.g., a parameter restriction) of another function that must be called;
• a condition that prevents the function from establishing one of its own postconditions (e.g., producing a valid return value is a postcondition); and
• a condition that prevents the function from re-establishing an invariant that it is responsible for maintaining. This is a special kind of postcondition that applies particularly to member functions. An essential postcondition of every non-private member function is that it must re-establish its class’s invariants.

Any other condition is not an error and should not be reported as an error.

[Risto-Stevcev]

There's something odd about this behavior though. I've tried doing similar things with fromEvent equivalents from xstream and rxjs and they don't have this issue.

[TylorS]

Most basically uses setTimeout or Promise.resolve(task).then(runTask) to schedule the default value from scan at the time of subscription. Which gives exactly one event loop to synchronously call dispatchEvent and emit before the default value has the chance to emit.

Whereas other libraries will emit the default scan value at the time of subscription - ensuring it occurs before any others could possibly make it through.

This may seem odd at first, but this guarantee that events do not occur in the same call stack as subscription makes your applications less prone to errors that occur due to the order in which you subscribe to your streams.

[zkochan]

I still think this is very confusing. Maybe it is obvious to you but for such a beginner in FRP like myself, it was really hard. I didn't believe my eyes when I've seen that the initial value happened first.

It should be at least mentioned in the API docs, with bold font and examples.

[Risto-Stevcev]

@TylorS

This may seem odd at first, but this guarantee that events do not occur in the same call stack as subscription makes your applications less prone to errors that occur due to the order in which you subscribe to your streams.

Can you be more specific (maybe an example) on how this prevents errors? Emitting the default scan value at subscription seems like the correct design decision to me

[Risto-Stevcev]

@Frikki Not sure I would call it an error either, but scan definitely behaves differently in this particular circumstance than any other circumstance that it's used, so it's not conforming to the API spec

[axefrog]

I can't see exactly why this is happening in this case, but I've experienced the initial scan value arriving out of order when the time value of the initial inbound event is generated before the scan source is run. The scheduler schedules the initial value based on the current time, and if the inbound value is delivered to the scan sink in the same call stack, you end up with two scheduled tasks, the second having an earlier schedule time than the first, which causes the scheduler to arrange for the second to be delivered before the first.

[Frikki]

As I see it, @axefrog, the culprit is what @davidchase wrote here, and is aligning with your comment, too.

@Risto-Stevcev, I still think that most.scan() does what it should, but the dispatchEvent() call blocks the caller and messes up the order:

... the event handlers run on a nested callstack: they block the caller until they complete ...

If we, as @TylorS mentions in his comment, didn’t ensure that events occur on a different callstack than the subscription, we would see race condition issues in many cases.

This relates to this issue, and the solution chosen was to develop @most/core and get rid of these not-really-true-event-streams (see @TylorS’s and @briancavalier’s comments towards the end of the issue).

[Risto-Stevcev]

Is there a downside to modifying the implementation of scan so that it applies setTimeout(..., 0) to the initial value of a scan if it's coming from an event?

That way none of this would occur and would ensure that all events occur on a different callstack than the subscription

[ryanlaws]

@Risto-Stevcev, in my testing,setTimeout(fn, 0) delays the call to fn by up to 12 milliseconds (usually about 5ms), even in the absence of anything else in the window, so that implementation isn't safe for all purposes.

[Risto-Stevcev]

@ryanlaws DOMHighResTimeStamp is in ms not mcs, could also be that you included the function declaration by accident:

const f = () => 123
const timeout = () => { performance.mark('fn-start'); setTimeout(f, 0); performance.mark('fn-end') }
timeout()
performance.measure('fn', 'fn-start', 'fn-end')
performance.getEntriesByName('fn')[0].duration
// vs
timeout2 = () => { performance.mark('fn2-start'); f(); performance.mark('fn2-end') } 
timeout2()
performance.measure('fn2', 'fn2-start', 'fn2-end')
performance.getEntriesByName('fn2')[0].duration

On my machine the difference is about 0.06-0.08 ms, which is 60-80 microseconds. That's very negligible

[ryanlaws]

@Risto-Stevcev Thanks for the test snippet. It's showing a difference of just shy of .1ms on my machine. It's very possible that my test was flawed - I believe I was directly calling performance.now() and subtracting instead of performance.mark() so the former may account for the extra milliseconds. I ran it months ago and I don't have my test code handy, so I trust yours over mine for sure.