Guard setState with isMounted

[randallsquared]

Firing two actions close together, where one action changes state for a watched store, and the other action results in the removal of the watching component from the DOM, can produce "Error: Invariant Violation: replaceState(...): Can only update a mounted or mounting component." from React. The setState call therefore needs to be guarded even though componentWillUnmount removes the listener.

I haven't written a test for this, yet.

[BinaryMuse]

Thanks, @randallsquared. If you have a simple example that can consistently reproduce this error, I'll incorporate it into the test suite; otherwise I'll write a more naïve test case.

[randallsquared]

Disentangling from my app will take some time, so I don't have one offhand. I can have a test in later this week, if you wanted to wait for it.

[BinaryMuse]

That sounds fine to me. Thanks very much!

[gmarketer]

I have the same problem.
It seems that the problem caused by bug in events.
StoreWatchMixin.componentWillUnmount is called before change listener fires and listener removed normally as it should.
But later this listener called by EventEmitter like it was never removed.

[randallsquared]

So, I ended up doing quite a lot of surgery to that test, and I'm sure that it's not as clean as it might be. The two main causes of the extraneous changes were avoiding React's triggering of jsdom's "Wrong Document" error [1] and using FluxChildMixin and FluxMixin. Probably this could be refactored to remove those last two, since the reason I reintroduced them was to try to avoid Wrong Documents, but it turned out that clearing the require cache was what was really needed.

[1] https://groups.google.com/forum/#!topic/reactjs/5UlF-mBsG2o

[BinaryMuse]

Okay, I did a bit of digging into this issue. Here's what's happening to cause the bug:

1. Parent component willMount hook called
   1. Parent component registers change event handler on Store1, Store2
2. Child component willMount hook called
   1. Child component registers change event handler on Store1
3. Action is dispatched
4. Store1 handles action, fires change event
   1. EventEmitter gets an array of all the event handlers — note that this is a copy of the internal handlers list
   2. EventEmitter starts calling handlers in a loop
5. Parent component handles event, sets state from getStateFromFlux and re-renders due to new state — Child is no longer part of the render tree
6. Child component willUnmount hook called
   1. Child component removes change event handler on Store1 — but it doesn't matter, because the list of handlers has already been built in step 4
7. Second event handler from the step 4 array is called
8. Since the Child was unmounted, the call to setState throws

Based on nodejs/node-v0.x-archive#7872, this is the intended behavior in EventEmitter, so guarding against this in Fluxxor is necessary.

I'm going to merge this as is, and work on cleaning up the tests some other time. Thanks so much @randallsquared for the code!

[BinaryMuse]

Landed as 212f3a9 and e402faf