Prevstate is being mutated inside of Reactor#dispatch

[colindresj]

Ran into an unexpected bug last week, which took me some time to dig through, but I think I've noticed a case where prevState is being mutated when state is transformed inside of a store handler.

Here's a screenshot of what's going on:

You can see that logging prevState before invoking the store handler shows one thing, and then logging it after the store handler is called, shows a new prevState.

The second result of prevState, coincides with what state should look like after currState is handled inside of the store, so this sounds like a pointer problem to me.

Eventually prevState === this.__state evaluates to true, so no change observers are notified and my app doesn't update despite an action being sent into the system.

For reference here's my handler:

const onFetchSuccess = (state, list, userId) => {
  const newList = Immutable.fromJS(list).toKeyedSeq().mapKeys((_, v) => {
    return createKey(v.get('type'), v.get('id'));
  }).toOrderedMap();

  return state.set(userId, newList);
};

As you can see no direct mutations going on. Adding an asImmutable call before returning state, however, prevents the bug:

const onFetchSuccess = (state, list, userId) => {
  const newList = ...
  return state.asImmutable().set(userId, newList);
};

So, somewhere along the way, state is being mutated, possibly inside of Store#handle, but haven't had enough time to really investigate.

Want to be upfront that I'm using a previous version of Nuclear because of #141

[jordangarcia]

Hm, that is interesting behavior indeed.

Can you reproduce this in the context of only ImmutableJS? I was under the impression that withMutations doesn't affect the current thing, but instead creates a mutable copy that can be returned.

[colindresj]

@jordangarcia that's my understanding of withMutations as well.

Unfortunately, was not able to repro just with Immutable. Here's a fiddle, which (I believe) works the same way Nuclear does: https://jsfiddle.net/6Lucnaj3/

[jordangarcia]

I will be releasing 1.2 in the next couple of days, it should fix this behavior as __notify will no longer be conditional on state equality, but rather if any store has changed since last notify.

[colindresj]

Cool, I'll keep an eye out

[bhamodi]

This should be fixed in v1.2.0 @colindresj! Feel free to reopen if it is still reproducible.