mattkrick / redux-optimistic-ui

a reducer enhancer to enable type-agnostic optimistic updates
MIT License
693 stars 36 forks source link

Multiple optimistic reducers share global history #25

Closed bhj closed 7 years ago

bhj commented 7 years ago

Hi, related to #19, my root reducer looks something like

import queue from './modules/queue'
import user from './modules/user'
import { optimistic } from 'redux-optimistic-ui'
...
return combineReducers({
  queue: optimistic(queue),
  user
})

then the user reducer has only one optimistic property, so it imports optimistic() and wraps its special sub-reducer in optimistic() and calls it when necessary. This all works fine, however I notice when an optimistic action fires in either "domain" I can see the history stuff (BEGIN/COMMIT/etc) happening over in the other. This doesn't seem to have an immediately adverse affect since the state in the "other" domain remains unchanged through the cycle, but it seems undesirable, or am I missing something?

Thanks for all your work on this!

mattkrick commented 7 years ago

hi @bhj, would you mind trying #23 and letting me know how that works?

bhj commented 7 years ago

Thanks @mattkrick I tested with that patch as well as back on 0.5.0 and saw the same thing. Still not even sure if it's an "issue" :)

mattkrick commented 7 years ago

oh, just re-read your original piece. you're saying that the history in the queue includes actions that occur in user?

That's working as intended, as your user reducer could trigger middleware that could affect queue. your concern, if i understand you correctly, is that the user reducer is not idempotent. so let's look at an example:

queue = 0 user = 0;

action(REQUEST_QUEUE++) queue = 1 // optimistic user = 0

action(user++) queue = 1 user = 1

action(REQUEST_QUEUE_FAILED) queue = 0 user = 1 // internally, we reply ever action since the time of the request, so it gets set to 0, then replayed until present, so it turns to 1 again

bhj commented 7 years ago

@mattkrick Thanks for the explanation! OK, seeing that in redux devtools made me raise a brow but makes sense.