javascript - React: weird closure bug caused by setInterval

Question

I have a component where there is a counter and a button to increment it. After the counter reaches to 5 we cannot increment it further. And there is a checkbox as well, when checked, the counter will increment by 1 every second.

The code is fairly straightforward

export default function App() {
  const [isActivated, setIsActivated] = useState(false);
  const [count, setCount] = useState(1);

  const incrementCount = () => {
    console.log("count inner", count);
    if (count < 5) {
      setCount((c) => c + 1);
    }
  };

  const timerRef = useRef(null);
  console.log("count outer", count);

  useEffect(() => {
    if (isActivated) {
      timerRef.current = setInterval(incrementCount, 1000);
    } else {
      clearInterval(timerRef.current);
    }
  }, [isActivated]);

  return (
    <div>
      <h1>{count}</h1>
      <input
        type="checkbox"
        onClick={(e) => {
          setIsActivated(e.target.checked);
        }}
      />
      <label>activate internal</label>
      <br />
      <br />
      <button onClick={incrementCount}>increment</button>
    </div>
  );
}

However, it has a bug: after you checked the checkbox, the counter will not stop at 5. Instead, it will keep incrementing. By putting console.log inside of incrementCount I found that the count state always stays at 1. I feel like it is a stale closure problem. But I cannot really pinpoint what exactly it is that is causing the problem.

Here is a live demo you can play with https://codesandbox.io/s/cranky-kepler-dedb6?file=/src/App.js

By the way, I know that I should have wrapped incrementCount inside of useCallback to make it stay referentially the same between renders. But that still doesn't answer the question that why is that the count variable in incrementCount is stale.

question from:https://stackoverflow.com/questions/65913640/react-weird-closure-bug-caused-by-setinterval

Answer 1

In your incrementCount, you are not just using the state closure to update the state. You are also reading straight from the count variable (if (count < 5)) which always has the stale value, unless you redefine the function on every re-render (which you do) and you use the re-defined function in setInterval which you don't.

Once incrementCount is run once, your component gets rerendered and you'll have a new instance of count, and also a new definition of incrementCount which is bound to the new count, but your previous definition of incrementCount bound to stale count is still in the memory and is being executed because of setInterval.

You can:

• make the count also a ref
• use useCallback for incrementCount and include it in the useEffect depdendencies. Also, provide a cleanup for setInterval in useEffect.
• move the conditional if (count < 5) inside the state closure, so you wouldn't rely on stale context. setCount((c) => (c < 5 ? c + 1 : c));