Avoiding endless event listeners that are added when a state changes in React

Question

I have a state-variable items that's just a collection (object) of items with an ID and a ref (which points to an HTMLDivElement), and a state-variable attachments which is another collection, but has the opacity key that I'll be changing if an item is ever dragged close to it.

In short, when item.ref's mousemove is close to an attachment, update attachment's opacity key to 1 (from 0). Here's the code for how I'm achieving this:

const items = getFromStore('items');
const attachments = getFromStore('attachments');

useEffect(() => {
  for(const id in items) {
    const element = items[id].ref;
    element.addEventListener('mousedown', (e) => handleMouseDown(e, element);
  }
}, [items, attachments]);

and the handleMouseDown function:

const handleMouseDown = useCallback((e, element) => {
  ..
  ..
  const mouseMove = (e) => {
    ..
    ..
    updateAttachment(id, { opacity: 1 })
  };

  window.addEventListener('mousemove', mouseMove);
}, [attachments] /* notice the dependency */);

The call to updateAttachment happens when we move/drag our item. The problem here is that, because my useEffect also depends on attachments, so that it's able to rebuild the event listener function, I now get thousands of event listeners every time the user moves their mouse, because the handleMouseDown function updates the attachments, then, because attachments have updated, the effect is ran again, and so on.

What am I doing wrong?

Answer 1

You need to return a cleanup function from your effect, to unsubscribe from the events when the effect is rerun.

useEffect(() => {
  const listeners = {};
  for (const id in items) {
    const element = items[id].ref;
    listeners[id] = (e) => handleMouseDown(e, element)
    element.addEventListener('mousedown', listeners[id]);
  }

  return () => {
    for (const id in items) {
      const element = items[id].ref;
      element.removeEventListener('mousedown', listeners[id]);
    }
  }
}, [items, attachments]);

Additionally, if handleMouseDown is only used inside the effect i recommend moving it in there. This won't eliminate the need for a cleanup function, but will make it obvious that it's all part of the same code and make it impossible for them to get out of sync by messing up dependency arrays.

useEffect(() => {
  const handleMouseDown = (e, element) => {
    // ...
  }

  const listeners = {};
  for (const id in items) {
    const element = items[id].ref;
    listeners[id] = (e) => handleMouseDown(e, element)
    element.addEventListener('mousedown', listeners[id]);
  }

  return () => {
    for (const id in items) {
      const element = items[id].ref;
      element.removeEventListener('mousedown', listeners[id]);
    }
  }
}, [items, attachments]);