CODEWITHSUNDEEP
javascriptreactjsreact-hooks
mr.bjerre
mr.bjerre

Reputation: 2888

useEffect does not remove my event listeners

I have a custom hook that sets up multiple event listeners which works as intended. But I've noticed that my event listeners does not get removed on rerendering. What do I do wrong (event listeners in the last part of the code)? I.e. I've noticed it on the anchor tag events; handleLink.

export default function useCursorSelector(
  selectorRef: React.RefObject<HTMLElement>,
  onClickLink: (url: string) => void,
  onSelect: (text: string) => void
) {
  const classes = useStyles();

  const handleSelection = React.useCallback(
    (event: MouseEvent) => {
      const selection: Selection | null = window.getSelection();
      if (selection) {
        const text = fullStringSelection(selection);
        selection.removeAllRanges();
        onSelect(text);
      }
      event.stopPropagation();
    },
    [onSelect]
  );

  const handleLink = React.useCallback(
    (a: HTMLAnchorElement) => (e: MouseEvent) => {
      e.preventDefault();
      onClickLink(a.href);
    },
    [onClickLink]
  );

  React.useEffect(() => {
    if (selectorRef.current) {
      wrapInSpans(selectorRef.current, classes.highlight, onClickLink);
    }
  }, [classes.highlight, selectorRef, onClickLink]);

  React.useEffect(() => {
    selectorRef!
      .current!.querySelectorAll(`.${classes.highlight}`)
      .forEach((e: any) => {
        e.addEventListener("mouseup", handleSelection);
      });
    selectorRef!
      .current!.querySelectorAll(`a`)
      .forEach((a: HTMLAnchorElement) => {
        a.addEventListener("click", handleLink(a));
      });
    return () => {
      selectorRef!
        .current!.querySelectorAll(`.${classes.highlight}`)
        .forEach((e: any) => {
          e.removeEventListener("mouseup", handleSelection);
        });
      selectorRef!
        .current!.querySelectorAll(`a`)
        .forEach((a: HTMLAnchorElement) => {
          a.removeEventListener("click", handleLink(a));
        });
    };
  }, [handleSelection, handleLink, classes.highlight, selectorRef]);
}

Upvotes: 2

Views: 860

Answers (1)

Jordan Burnett
Jordan Burnett

Reputation: 1844

Each time you call handleLink(a) you are creating a new anonymous function to use as the callback for your event handler.

When you call removeEventListener you should be passing a reference to the same callback you used to attach the event, however each time you call handleLink you are creating a new anonymous function.

You could somehow store a reference to the callbacks you create, or rewrite handleLink so that you don't need to pass the element at all.

const handleLink = React.useCallback((e: MouseEvent) => {
    e.preventDefault();
    onClickLink(e.currentTarget.href);
}, [onClickLink]);

...

React.useEffect(() => {
  selectorRef!
    .current!.querySelectorAll(`.${classes.highlight}`)
    .forEach((e: any) => {
      e.addEventListener("mouseup", handleSelection);
    });
  selectorRef!
    .current!.querySelectorAll(`a`)
    .forEach((a: HTMLAnchorElement) => {
      a.addEventListener("click", handleLink);
    });
  return () => {
    selectorRef!
      .current!.querySelectorAll(`.${classes.highlight}`)
      .forEach((e: any) => {
        e.removeEventListener("mouseup", handleSelection);
      });
    selectorRef!
      .current!.querySelectorAll(`a`)
      .forEach((a: HTMLAnchorElement) => {
        a.removeEventListener("click", handleLink);
      });
  };
}, [handleSelection, handleLink, classes.highlight, selectorRef]);

Upvotes: 3

Related Questions