1
votes

I have this component, and I am using useRef and useEffect to handle a click outside a popup so I can close it.

I have added the two dependencies the useEffect needs but I get this error:

The 'handleClickOutside' function makes the dependencies of useEffect Hook (at line 117) change on every render. Move it inside the useEffect callback. Alternatively, wrap the 'handleClickOutside' definition into its own useCallback()

As you can see here, I am adding both dependencies, but it still throws this error/warning:

  useEffect(() => {
    if (isOverlayOpen) {
      document.addEventListener("mousedown", handleClickOutside);
    } else {
      document.removeEventListener("mousedown", handleClickOutside);
    }
    return () => {
      document.removeEventListener("mousedown", handleClickOutside);
    };
  }, [isOverlayOpen, handleClickOutside]);

Any ideas how to fix this?

Here i the codesandbox: https://codesandbox.io/s/laughing-newton-1gcme?fontsize=14&hidenavigation=1&theme=dark The problem is in src/components/typeahead line 100

And here the component code:

function ResultsOverlay({
  isOpen,
  items,
  selectItem,
  highlightedOption,
  setIsOverlayOpen,
  isOverlayOpen
}) {
  const node = useRef();
  const handleClickOutside = e => {
    if (node.current.contains(e.target)) {
      return;
    }
    setIsOverlayOpen(false);
  };

  useEffect(() => {
    if (isOverlayOpen) {
      document.addEventListener("mousedown", handleClickOutside);
    } else {
      document.removeEventListener("mousedown", handleClickOutside);
    }
    return () => {
      document.removeEventListener("mousedown", handleClickOutside);
    };
  }, [isOverlayOpen]);

  function matchedOptionsClass(index) {
    if (highlightedOption === index) {
      return "ph4 list f3 sesame-red list-item pointer";
    }
    return "ph4 list sesame-blue list-item pointer";
  }

  if (isOpen) {
    return (
      <div className="absolute" ref={node}>
        <ul className="w5 mt0 pa0 h5 overflow-scroll shadow-5 dib">
          {items &&
            items.map((item, index) => (
              <li
                onClick={() => selectItem(item)}
                className={matchedOptionsClass(index)}
              >
                {item}
              </li>
            ))}
        </ul>
      </div>
    );
  } else {
    return null;
  }
}
1

1 Answers

1
votes

Two problems:

First, do what the linst is telling you and move your function definition inside your effect

  useEffect(() => {
      const handleClickOutside = e => {
          if (node.current.contains(e.target)) {
              return;
          }
          setIsOverlayOpen(false);
       };
    if (isOverlayOpen) {
      document.addEventListener("mousedown", handleClickOutside);
    } else {
      document.removeEventListener("mousedown", handleClickOutside);
    }
    return () => {
      document.removeEventListener("mousedown", handleClickOutside);
    };
  }, [isOverlayOpen]);

Second, setIsOverlayOpen is a callback provided via props, so it doesn't have a stable signature and triggers the effect on each render.

Assuming that setIsOverlayOpen is a setter from useState and doesn't need to change it's signature you can workaround this by wrapping your handler in an aditional dependency check layer by using useCallback

  const stableHandler = useCallback(setIsOverlayOpen, [])      

  useEffect(() => {
      const handleClickOutside = e => {
          if (node.current.contains(e.target)) {
              return;
          }
          stableHandler(false);
       };
    if (isOverlayOpen) {
      document.addEventListener("mousedown", handleClickOutside);
    } else {
      document.removeEventListener("mousedown", handleClickOutside);
    }
    return () => {
      document.removeEventListener("mousedown", handleClickOutside);
    };
  }, [isOverlayOpen, stableHandler]);