UseEffect not removing event listeners when code is referenced-CodePudding

I am using event listeners with react and html canvas to build a technical analysis website which the user will be drawing trendlines, shapes, free drawing etc. I have a very basic prototype of just the user being able to click a button to activate free draw and then click it again to disable it.

The issue I am running into is that all the code for the free draw function needs to be placed under the useEffect hook of the canvas component otherwise I will not be able to remove the event listeners.

And the reason I do not want to continue putting code in the useEffect is just for organizational purposes as I plan to add many other functions for all the usual features on a technical analysis website similar to tradingview.

The following is my code that works: Canvas.js:

const Canvas = ({ height, width, draw }) => {
  const canvas = React.useRef();
  const isDrawing = useSelector((state) => state.freeDrawBoolean.value);

  React.useEffect(() => {
    const context = canvas.current.getContext("2d");

    let coord = { x: 0, y: 0 };


    function reposition(event) {
      coord.x = event.clientX - context.canvas.offsetLeft;
      coord.y = event.clientY - context.canvas.offsetTop;
    }

    function draw1(event) {
      context.beginPath();
      context.lineWidth = 5;
      context.lineCap = "round";
      context.strokeStyle = "#ACD3ED";
      context.moveTo(coord.x, coord.y);
      reposition(event);
      context.lineTo(coord.x, coord.y);
      context.stroke();
    }

    function start(event) {
      document.addEventListener("mousemove", draw1);
      reposition(event);
    }

    function stop() {
      document.removeEventListener("mousemove", draw1);
    }

    if (isDrawing == true) {
      document.addEventListener("mousedown", start);
      document.addEventListener("mouseup", stop);
    }
    if (isDrawing == false) {
      document.removeEventListener("mousedown", start);
      document.removeEventListener("mouseup", stop);
    }

    return () => {
      document.removeEventListener("mousedown", start);
      document.removeEventListener("mouseup", stop);
    };
  }, [isDrawing]);

  return (
      <canvas
        ref={canvas}
        height={height}
        width={width}
        style={{ border: "1px solid red" }}
      />
  );
};


export default Canvas;

App.js:

function App() {
  const isDrawing = useSelector((state) => state.freeDrawBoolean.value);
  const dispatch = useDispatch();

  return (
    <>

      <Canvas height={500} width={500} />
      <button onClick={() => dispatch(changeBool())}>changeBool</button>
      <span>value: {isDrawing ? "true" : "false"}</span>
    </>
  );
}

export default App;

So the above code works, and works by clicking on changeBool button and changing the draw function to be on or off. As you can see the useEffect under Canvas has a lot now and I do not want to continue to add code there, I would rather organize it like the following:

Canvas.js:

const Canvas = ({ height, width, draw }) => {
  const canvas = React.useRef();
  const isDrawing = useSelector((state) => state.freeDrawBoolean.value);

  React.useEffect(() => {
    const context = canvas.current.getContext("2d");

    draw(context, isDrawing);

    //futureFeature(context, isDrawing2)
  }, [isDrawing]);

  return (
      <canvas
        ref={canvas}
        height={height}
        width={width}
        style={{ border: "1px solid red" }}
     />
  );
};


export default Canvas;

App.js:

const draw = (context, isDrawing) => {
  let coord = { x: 0, y: 0 };


  function reposition(event) {
    coord.x = event.clientX - context.canvas.offsetLeft;
    coord.y = event.clientY - context.canvas.offsetTop;
  }

  function draw1(event) {
    context.beginPath();
    context.lineWidth = 5;
    context.lineCap = "round";
    context.strokeStyle = "#ACD3ED";
    context.moveTo(coord.x, coord.y);
    reposition(event);
    context.lineTo(coord.x, coord.y);
    context.stroke();
  }

  function start(event) {
    document.addEventListener("mousemove", draw1);
    reposition(event);
  }

  function stop() {
    document.removeEventListener("mousemove", draw1);
  }

  if (isDrawing == true) {
    document.addEventListener("mousedown", start);
    document.addEventListener("mouseup", stop);
  }
  if (isDrawing == false) {
    document.removeEventListener("mousedown", start);
    document.removeEventListener("mouseup", stop);
  }

  return () => {
    document.removeEventListener("mousedown", start);
    document.removeEventListener("mouseup", stop);
  };
};

function App() {
  const isDrawing = useSelector((state) => state.freeDrawBoolean.value);
  const dispatch = useDispatch();

  return (
    <>
      <Canvas height={500} width={500} draw={draw} />
      <button onClick={() => dispatch(changeBool())}>changeBool</button>
      <span>value: {isDrawing ? "true" : "false"}</span>
    </>
  );
}

export default App;

Now this second iteration of code works to the extent that originally isDrawing == false thus you are unable to draw and when you click changeBool button then you can draw, however when you then go to turn it off again changeBool becomes false but you are still able to draw, so the issue is the event listeners are not being removed when I try to organize the code this way. I cannot think of a way of fixing this without changing back to my first iteration of code and bringing back the same organizational issues I was trying to avoid.

CodePudding user response：

This is likely because you arent returning the callback in the useEffect

   React.useEffect(() => {
    const context = canvas.current.getContext("2d");

    return draw(context, isDrawing);

    //futureFeature(context, isDrawing2)
  }, [isDrawing]);