Search code examples
javascriptreactjsreact-hooksevent-loop

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 invoked between the commit phase and the useEffect invocation, 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. Commit phase - state committed to DOM.
  3. useLayoutEffect
  4. Interval callback - using savedCallback.current(), which is different than callback.
  5. useEffect - savedCallback.current = callback;

React's Life Cycle

To further illustrate this, here's a diagram showing React's Life Cycle with hooks:

React lifecycle for functional components

Dashed lines mean async flow (event loop released) and you can have an interval callback invocation at these points.

Note however, that the dashed line between Render and React updates DOM (commit phase) is most likely a mistake. As this codesandbox demonstrates, you can only have an interval callback invoked after useLayoutEffect or useEffect (but not after the render phase).

So you can set the callback in 3 places:

  • Render - Incorrect because state changes have not yet been committed to the DOM.
  • useLayoutEffect - correct because state changes have been committed to the DOM.
  • useEffect - incorrect because the old interval callback may fire before that (after layout effects) .

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.

A screenshot of a code sandbox showing an exception thrown when the effect callback is not up to date

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);

If onInterval is invoked after the commit phase but before useEffect, it will print the wrong value.


Solution

  • This does not look like a bug to me, although I understand the discussion.

    The answer above that suggests updating the ref during render would be a side effect, which should be avoided because it will cause problems.

    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?

    I believe the "correct" one is the one that has been committed. For one reason, committed effects are the only ones that are guaranteed to have cleanup phase later. (The interval in this question doesn't need a cleanup effect, but other things might.)

    Another more compelling reason in this case, perhaps, is that React may pre-render things (either at lower priority, or because they're "offscreen" and not yet visible, or in the future b'c of animation APIs). Pre-rendering work like this should never modify a ref, because the modification would be arbitrary. (Consider a future animation API that pre-renders multiple possible future visual states to make transitions faster in response to user interaction. You wouldn't want the one that happened to render last to just mutate a ref that's used by your currently visible/committed view.)


    Edit 1 This discussion mostly seems to be pointing out that when JavaScript isn't synchronous (blocking), when it yields between rendering, there's a chance for other things to happen in between (like a timer/interval that was previously scheduled). That's true, but I don't think it's a bug if this happens during render (before an update is "committed").

    If the main concern is that the callback might execute after the UI has been committed and mismatch what's on the screen, then you might want to consider useLayoutEffect instead. This effect type is called during the commit phase, after React has modified the DOM but before React yields back to the browser (aka so no intervals or timers can run in between).


    Edit 2 I believe the reason Dan originally suggested using a ref and an effect for this (rather than just an effect) was because updates to the callback wouldn't reset the interval. (If you called clearInterval and setInterval each time the callback changed, the overall timing would be interrupted.)