distolma / storeon-observable

Module for storeon which allows to create async actions
MIT License
5 stars 0 forks source link

Difference in order of execution compared to equivalent in redux-observable #14

Closed F-Bernkastel closed 4 years ago

F-Bernkastel commented 4 years ago

We have recently started experimenting with using storeon combined with storeon-observable to replace an existing redux + redux-observable state management system, however we ran into the interesting difference that storeon-observable executes epics first, then the listeners (where the actual state mutation occurs), which is counter-intuitive compared to the redux-observable order of reducers first, then the corresponding epics.

I've made a quick little pen to demonstrate it with both the ping-pong example given in the README, and a very simplified async data retrieving pattern that crops up a lot in our cases, you can find it here (Please check the console as it contains the relevant logs). It also illustrates where the problem lies, as we cannot have an event occur as a side effect of another event while using the state mutation of the first event.

I would like to know whether this is an intentional design decision or not. (the README mentions that the project is based of of redux-observable, hence we assumed that this behavior would also be the same).

Thanks in advance!

majo44 commented 4 years ago

Yep, I can confirm the problem.

In redux-observable we have such code https://github.com/redux-observable/redux-observable/blob/master/src/createEpicMiddleware.ts#L91 where even in comment we see that order is forced.

As storeon api do not provides any way to be notified "after" event dispatch and the @dispatch event is dispatch before event handlers, this problem will be not simple to solve.

The usage of setTimeout, Promise.then ect. is not a solution as we have to support synchronous scenarios. Without storeon api changes I see only solution which is using temporary event handler:

export const toEventObservable = <State, Events = any>(
  store: StoreonStore<State, Events>
): Observable<StoreonEvent<Events>> => {
  return new Observable<StoreonEvent<Events>>(subscriber => {
    subscriber.add(
      store.on('@dispatch', (_, event) => {
        const nextEvent = toEvent(event[0], event[1]) as any
        if (event[2]) {
          // if there are some events handlers for this particular event
          // we will add to the end of handlers stack temporary event handler
          // which after all other events handlers (which are able to change the state)
          // will notify observable
          const un = store.on(event[0], () => {
            un()
            subscriber.next(nextEvent)
          })
        } else {
          // if there is no events handlers for this particular event
          // we can safely notify observable, as state will not change
          subscriber.next(nextEvent)
        }
      })
    )
  })
}
distolma commented 4 years ago

Fixed in 1.0.1