Should setting state inside discrete events cause cleanup to run?

[gaearon]

This bug is pretty confusing:

https://twitter.com/kentcdodds/status/1102659818660102145

I think it happens because fn scheduled by setInterval(fn, 0) jumps in front of the [running] effect cleanup caused by setRunning(false). So the interval still fires, overwriting setLapse(0) that happened during the event with its setLapse(someValue).

This reminds me of the issue described in #14750 (comment), or at least a part of it:

In fact, this problem exists even for regular React keystrokes (and other “discrete” events). The solution to that would be to flush passive effects before we get a discrete event.

But here, it seems like this wouldn’t be sufficient because the effect flips as a result of the click, not before it. So should setState inside a discrete event also flush passive effect? Seems like not. (That would defeat the purpose of delaying them.)

So this is working as designed, and the fix is just useLayoutEffect when the timing matters? Or the rAF solution?

[sebmarkbage]

The problem is that the timer itself isn't a discrete event. Discrete events are only guaranteed in relation to other discrete events. I don't think that's what you want here.

Stopping the timer is an asynchronous operation since all set states are async. So something else can come in before it is flushed.

This is a case where useLayoutEffect doesn't actually fully fix the problem. Certainly not in concurrent mode, but this is also sketchy in sync mode. If it wasn't a timer but let's say a focus event, then that can fire within a batch before this is flushed which would have the same problem.

setRunning(false); // I would like to add a stop of this timer to the queue to be performed later
setLapse(0); // I would like to add an operation to set lapse to zero later
// lots of random stuff that can happen before the batch flushes
// This might also queue an operation to set lapse to something else
// actual rendering
// If concurrent mode, lots of other random stuff that can happen while rendering
// This might also queue an operation to set lapse to something else
// Actually do all that work in order

It's all about what order things are added to the queue.

[sebmarkbage]

One approach is to dispatch the reset only once the timer actually stops, i.e. in the effect. If you want it to always display zero in the same frame, then you can always display zero when running is false.

However, as almost always, this is better modeled as a reducer. The reducer can easily refuse updates to lapse when it thinks it is in a stopped state and also perform the logic to actually reset it.

[gaearon]

I get it now. Thanks.

https://mobile.twitter.com/dan_abramov/status/1104069236132057089
https://codesandbox.io/s/23m1659rlp