Gotcha with remove event listener

I'll drive right into it.

Consider two boxes out of which only one will be visible at any given time. The user can choose which box to see and each box will have an event listener attached to it. For the sake of simplicity, we'll have a click event that'll alert 'Hello World'. The code for adding the event handler looks like this:

function setClickEvent() {
  let el = document.getElementById(visibleElementId);
  function handleClick() {
    alert('Hello World');
  }
  function removeListener() {
    el.removeEventListener('click', handleClick);
  }
  el.addEventListener('click', handleClick);
  return removeListener;
}

The function adds a click event to the visible box and returns a function for removing the event listener.

If you switch between the boxes a couple of times, you would immediately find that the event is never removed and it gets piled on for every click. If you have already found the issue, kudos to you.

It took me quite a while to figure out the issue. When the function returned from setClickEvent is called, the removeEventListener would be called as expected, but the handleClick event inside would be different for every setClickEvent function since its scoped within the function.

When we switch from the red box to the blue box, then we would lose all reference to the handleClick function registered on the red box, which makes it impossible to remove the listener set on the red box on load. A simple way to overcome this problem is to move the handleClick function outside setClickEvent.

function handleClick() {
  alert('Hello World');
}
function setClickEvent() {
  let el = document.getElementById(visibleElementId);
  function removeListener() {
    el.removeEventListener('click', handleClick);
  }
  el.addEventListener('click', handleClick);
  return removeListener;
}

This does solve the issue to some extent. But, what if you couldn't move the handleClick outside?

A better way, in my opinion would be to remove the event listener before hiding the box .i.e., only one event listener would be present in the page and that would be registered on the visible box. This also ensures that we don't have any event listener in the wild which might lead to memory leak.

function showBox(id) {
  document.querySelectorAll('.box').forEach((box) => {
    cancelListener(); // Remove event listener
    box.classList.add('hidden');
  });
  let el = document.getElementById(id);
  el.classList.remove('hidden');
  visibleElementId = id;
  removeListener = setClickEvent();
}