Fix for bug #24 DragDropMonitor failes to notify subscribeToStateChange subscribers in some cases

[mattiasgronlund]

Added stateId which is increased by one each time an action is dispatched to the store.

Make sure that SubscribeToState change always calls listner() if no notification was received with the previous state, as the areDirty(...) code will not work in that case.

[gaearon]

Can we refactor this part to something like

const canSkipListener = currentStateId === prevStateId || (
  currentStateId === prevStateId + 1 &&
  !areDirty(state.dirtyHandlerIds, handlerIds)  
)

if (!canSkipListener) {
  listener();
}

?

[mattiasgronlund]

Nice,

I have updated the PR with the changes above.

/Mattias

On Wed, Jan 27, 2016 at 1:17 AM, Dan Abramov [email protected]
wrote:

In src/DragDropMonitor.js
#25 (comment):

 const handleChange = () => {

•  if (areDirty(this.store.getState().dirtyHandlerIds, handlerIds)) {
  

•    listener();
  

•  const state = this.store.getState();
  

•  const currentStateId = state.stateId;
  

•  const expectedStateId = prevStateId + 1
  

•  try {
  

•    if (currentStateId === expectedStateId) {
  

•      if (areDirty(state.dirtyHandlerIds, handlerIds)) {
  

•        listener();
  

•      }
  

•    } else if (currentStateId !== prevStateId) {
  

•      // Redux do not guarantee that all state updates are sent to all subscribers.
  

•      // Here we missed at least one state update, so areDirty() will not work.
  

•      listener();
  

Can we refactor this part to something like

const canSkipListener = currentStateId === prevStateId || (
currentStateId === prevStateId + 1 &&
!areDirty(state.dirtyHandlerIds, handlerIds)
)
if (!canSkipListener) {
listener();
}

?

—
Reply to this email directly or view it on GitHub
https://github.com/gaearon/dnd-core/pull/25/files#r50924041.