Using React Hooks, why are my event handlers firing with the incorrect state?

Question

I'm trying to create a copy of this spinning div example using react hooks. https://codesandbox.io/s/XDjY28XoV

Here's my code so far

import React, { useState, useEffect, useCallback } from 'react';

const App = () => {
  const [box, setBox] = useState(null);

  const [isActive, setIsActive] = useState(false);
  const [angle, setAngle] = useState(0);
  const [startAngle, setStartAngle] = useState(0);
  const [currentAngle, setCurrentAngle] = useState(0);
  const [boxCenterPoint, setBoxCenterPoint] = useState({});

  const setBoxCallback = useCallback(node => {
    if (node !== null) {
      setBox(node)
    }
  }, [])

  // to avoid unwanted behaviour, deselect all text
  const deselectAll = () => {
    if (document.selection) {
      document.selection.empty();
    } else if (window.getSelection) {
      window.getSelection().removeAllRanges();
    }
  }

  // method to get the positionof the pointer event relative to the center of the box
  const getPositionFromCenter = e => {
    const fromBoxCenter = {
      x: e.clientX - boxCenterPoint.x,
      y: -(e.clientY - boxCenterPoint.y)
    };
    return fromBoxCenter;
  }

  const mouseDownHandler = e => {
    e.stopPropagation();
    const fromBoxCenter = getPositionFromCenter(e);
    const newStartAngle =
      90 - Math.atan2(fromBoxCenter.y, fromBoxCenter.x) * (180 / Math.PI);
    setStartAngle(newStartAngle);
    setIsActive(true);
  }

  const mouseUpHandler = e => {
    deselectAll();
    e.stopPropagation();
    if (isActive) {
      const newCurrentAngle = currentAngle + (angle - startAngle);
      setIsActive(false);
      setCurrentAngle(newCurrentAngle);
    }
  }

  const mouseMoveHandler = e => {
    if (isActive) {
      const fromBoxCenter = getPositionFromCenter(e);
      const newAngle =
        90 - Math.atan2(fromBoxCenter.y, fromBoxCenter.x) * (180 / Math.PI);
      box.style.transform =
        "rotate(" +
        (currentAngle + (newAngle - (startAngle ? startAngle : 0))) +
        "deg)";
      setAngle(newAngle)
    }
  }

  useEffect(() => {
    if (box) {
      const boxPosition = box.getBoundingClientRect();
      // get the current center point
      const boxCenterX = boxPosition.left + boxPosition.width / 2;
      const boxCenterY = boxPosition.top + boxPosition.height / 2;

      // update the state
      setBoxCenterPoint({ x: boxCenterX, y: boxCenterY });
    }

    // in case the event ends outside the box
    window.onmouseup = mouseUpHandler;
    window.onmousemove = mouseMoveHandler;
  }, [ box ])

  return (
    <div className="box-container">
      <div
        className="box"
        onMouseDown={mouseDownHandler}
        onMouseUp={mouseUpHandler}
        ref={setBoxCallback}
      >
        Rotate
      </div>
    </div>
  );
}

export default App;

Currently mouseMoveHandler is called with a state of isActive = false even though the state is actually true. How can I get this event handler to fire with the correct state?

Also, the console is logging the warning:

React Hook useEffect has missing dependencies: 'mouseMoveHandler' and 'mouseUpHandler'. Either include them or remove the dependency array  react-hooks/exhaustive-deps

Why do I have to include component methods in the useEffect dependency array? I've never had to do this for other simpler component using React Hooks.

Thank you

Answer 1

The Problem

Why is isActive false?

const mouseMoveHandler = e => {
   if(isActive) {
       // ...
   }
};

(Note for convenience I'm only talking about mouseMoveHandler, but everything here applies to mouseUpHandler as well)

When the above code runs, a function instance is created, which pulls in the isActive variable via function closure. That variable is a constant, so if isActive is false when the function is defined, then it's always going to be false as long that function instance exists.

useEffect also takes a function, and that function has a constant reference to your moveMouseHandler function instance - so as long as that useEffect callback exists, it references a copy of moveMouseHandler where isActive is false.

When isActive changes, the component rerenders, and a new instance of moveMouseHandler will be created in which isActive is true. However, useEffect only reruns its function if the dependencies have changed - in this case, the dependencies ([box]) have not changed, so the useEffect does not re-run and the version of moveMouseHandler where isActive is false is still attached to the window, regardless of the current state.

This is why the "exhaustive-deps" hook is warning you about useEffect - some of its dependencies can change, without causing the hook to rerun and update those dependencies.

---

Fixing it

Since the hook indirectly depends on isActive, you could fix this by adding isActive to the deps array for useEffect:

// Works, but not the best solution
useEffect(() => {
    //...
}, [box, isActive])

However, this isn't very clean: if you change mouseMoveHandler so that it depends on more state, you'll have the same bug, unless you remember to come and add it to the deps array as well. (Also the linter won't like this)

The useEffect function indirectly depends on isActive because it directly depends on mouseMoveHandler; so instead you can add that to the dependencies:

useEffect(() => {
    //...
}, [box, mouseMoveHandler])

With this change, the useEffect will re-run with new versions of mouseMoveHandler which means it'll respect isActive. However it's going to run too often - it'll run every time mouseMoveHandler becomes a new function instance... which is every single render, since a new function is created every render.

We don't really need to create a new function every render, only when isActive has changed: React provides the useCallback hook for that use-case. You can define your mouseMoveHandler as

const mouseMoveHandler = useCallback(e => {
   if(isActive) {
       // ...
   }
}, [isActive])

and now a new function instance is only created when isActive changes, which will then trigger useEffect to run at the appropriate moment, and you can change the definition of mouseMoveHandler (e.g. adding more state) without breaking your useEffect hook.

---

This likely still introduces a problem with your useEffect hook: it's going to rerun every time isActive changes, which means it'll set the box center point every time isActive changes, which is probably unwanted. You should split your effect into two separate effects to avoid this issue:

useEffect(() => {
    // update box center
}, [box])

useEffect(() => {
   // expose window methods
}, [mouseMoveHandler, mouseUpHandler]);

---

End Result

Ultimately your code should look like this:

const mouseMoveHandler = useCallback(e => {
    /* ... */
}, [isActive]);

const mouseUpHandler = useCallback(e => {
    /* ... */
}, [isActive]);

useEffect(() => {
   /* update box center */
}, [box]);

useEffect(() => {
   /* expose callback methods */
}, [mouseUpHandler, mouseMoveHandler])

More info:

Dan Abramov, one of the React authors goes into a lot more detail in his Complete Guide to useEffect blogpost.

Answer 2

React Hooks useState+useEffect+event gives stale state. seems you are having similar problems. basic issue is that "it gets its value from the closure where it was defined"

try that Solution 2 "Use a ref". in your scenario

Add below useRef, and useEffect

let refIsActive = useRef(isActive);
useEffect(() => {
    refIsActive.current = isActive;
});

then inside mouseMoveHandler , use that ref

 const mouseMoveHandler = (e) => {    
  console.log('isActive',refIsActive.current);
    if (refIsActive.current) {