Potential bug in "official" useInterval example

useInterval

useInterval from this blog post by Dan Abramov (2019):

function useInterval(callback, delay) {
  const savedCallback = useRef();

  // Remember the latest callback.
  useEffect(() => {
    savedCallback.current = callback;
  }, [callback]);

  // Set up the interval.
  useEffect(() => {
    function tick() {
      savedCallback.current();
    }
    if (delay !== null) {
      let id = setInterval(tick, delay);
      return () => clearInterval(id);
    }
  }, [delay]);
}

A Potential Bug

The interval callback may be called between the render phase and the effects phase, causing the old (and hence not up to date) callback to be called. In other words, this may be the execution order:

1. Render phase - a new value for callback.
2. Interval callback - using savedCallback.current(), which is different than callback.
3. Effects executed - savedCallback.current = callback;

Demo

This bug is demonstrated in this codesandebox. To reproduce:

• Move the mouse over the grey div - this will lead to a new render with a new callback reference.
• Typically you'll see the error thrown in less than 2000 mouse moves.
• The interval is set to 50ms, so you need a bit of luck for it to fire right between the render and effect phases.

Use Case

The demo shows that the current callback value may differ from that in useEffect alright, but the real question is which one of them is the "correct" one?

Consider this code:

const [size, setSize] = React.useState();

const onInterval = () => {
  console.log(size)
}

useInterval(onInterval, 100);

onInterval is not wrapped with useCallback so the size value it will print is that of the last render.

If onInterval is called after render but before effects, it will print the wrong value.

This is the typical case we are trying to protect against.

Notes

• We are only considering a non-stable callback here (no useCallback or any other memoization). Sure, callback should be stable but useInterval cannot make such assumption and part of the point of the code is to support both cases.
• The crux of this question is to show/confirm:
  • Render and effects execution are asynchronous.
  • An interval callback may be called between them.
  • The "correct" callback in this case is the most recent one (the render argument).
  • The code is buggy.

Concurrent Mode

Why would the author update the callback ref in useEffect rather than on render? As some hinted in the comments, this is to protect from issues in concurrent mode. But there is still a possible bug in the code.

from Potential bug in "official" useInterval example