Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is it too expensive to add and remove event listeners on every call of my React custom hook? How to avoid it?

Here's my situation:

I've got a custom hook, called useClick, which gets an HTML element and a callback as input, attaches a click event listener to that element, and sets the callback as the event handler.

App.js

function App() {
  const buttonRef = useRef(null);
  const [myState, setMyState] = useState(0);

  function handleClick() {
    if (myState === 3) {
      console.log("I will only count until 3...");
      return;
    }
    setMyState(prevState => prevState + 1);
  }

  useClick(buttonRef, handleClick);

  return (
    <div>
      <button ref={buttonRef}>Update counter</button>
      {"Counter value is: " + myState}
    </div>
  );
}

useClick.js

import { useEffect } from "react";

function useClick(element, callback) {
  console.log("Inside useClick...");

  useEffect(() => {
    console.log("Inside useClick useEffect...");
    const button = element.current;

    if (button !== null) {
      console.log("Attaching event handler...");
      button.addEventListener("click", callback);
    }
    return () => {
      if (button !== null) {
        console.log("Removing event handler...");
        button.removeEventListener("click", callback);
      }
    };
  }, [element, callback]);
}

export default useClick;

Note that with the code above, I'll be adding and removing the event listener on every call of this hook.

And I would very much like to only add on mount and remove on dismount. Instead of adding and removing on every call.

Even though I'm using this:

useEffect(()=>{
  // Same code as above
},[element,callback]);  // ONLY RUN THIS WHEN 'element' OR 'callback' CHANGES

The problem with that is that even though element (ref) will remain the same across all renders, callback (which is the handleClick function from App) will change on every render. So I end up adding and removing the handlers on every render anyway.

I also can't transform handleCLick into a useCallback, because it depends on the state myState variable that changes on every render and my useCallback would still be recreated on every render (when myState changes) and the problem would continue.

const handleClick = useCallback(()=> {
  if (myState === 3) {
    console.log("I will only count until 3...");
    return;
  }
  setMyState(prevState => prevState + 1);
},[myState]); // THIS WILL CHANGE ON EVERY RENDER!

QUESTION:

Example on CodeSandbox

Should I worry about removing and attaching even listeners on every render? Or is this really not expensive at all and has no chance of hurting my performance? It there a way to avoid it?

like image 869
cbdeveloper Avatar asked May 15 '19 14:05

cbdeveloper


1 Answers

You could store the current callback in a ref and have a static callback for the event listener that only calls the current callback:

function useClick(element, callback) {
    console.log('Inside useClick...');

    const callbackRef = useRef(callback);

    useEffect(() => {
        callbackRef.current = callback;
    }, [callback]);

    const callbackWrapper = useCallback(props => callbackRef.current(props), []);

    useEffect(() => {
        console.log('Inside useClick useEffect...');
        const button = element.current;

        if (button !== null) {
            console.log('Attaching event handler...');
            button.addEventListener('click', callbackWrapper);
        }
        return () => {
            if (button !== null) {
                console.log('Removing event handler...');
                button.removeEventListener('click', callbackWrapper);
            }
        };
    }, [element, callbackWrapper]);
}

Working example:

Edit 7wxqxwx1rj

like image 118
trixn Avatar answered Oct 19 '22 06:10

trixn