0

I am not clear how to deal with this, when the warning wants me to put a function as a dependency, since functions do not keep their identity across renders. What I've read is to put the function inside the effect directly, but that doesn't work in my case since I'd have to duplicate the function, as it's needed outside the effect as well. Here is a slimmed down version of my component:

import * as React from 'react';
import trapTabKey from '/trapTabKey';

const { useEffect, useRef, useState } = React;

export default function FullScreen(props) {
  const [fullScreen, setFullScreen] = useState(false);
  const divRef = useRef(null);

  const handleKeyPress = (e) => {
    const key = e.which || e.charCode || e.keyCode;
    // Escape
    if (key === 27) {
      document.body.classList.remove('full-screen-open');
      setFullScreen(false);
      // Tab
    } else if (key === 9 && fullScreen) {
      const domNode = divRef.current;
      trapTabKey(e, domNode);
    }
  };

  useEffect(() => {
    document.body.addEventListener('keydown', handleKeyPress);
    return () => {
      // I need to bind handleKeyPress to the body here
      document.body.removeEventListener('keydown', handleKeyPress);
      document.body.classList.remove('full-screen-open');
    };
    // Here I get the warning: react-hooks/exhaustive-deps
    // React Hook useEffect has a missing dependency: 'handleKeyPress'. Either include it or remove the dependency array.
  }, []);

  return (
    // I also need handleKeyPress to bind it to this DIV here
    <div      
      onKeyPress={handleKeyPress}
      ref={divRef}
    >
      Content....
    </div>
  );
}

Essentially my effect is supposed to be a version of componentDidMount, componentDidUnmount, and it works, but the warning is there and I'd like to make sure I am using hooks correctly. I could just put eslint disable, but I don't like that solution, there has to be a right way to do this.

Amir
  • 4,211
  • 4
  • 23
  • 41
  • @SuleymanSah ah this is perfect. I wish this would have popped up when I searched before posting my question! – Amir Jan 10 '20 at 14:14
  • Amir you can close this question I think. – SuleymanSah Jan 10 '20 at 14:15
  • Amir accepting an answer in a duplicate question is not good I think. – SuleymanSah Jan 10 '20 at 14:17
  • @SuleymanSah what am I supposed to do? The answer provided did help me, I upvoted the original too. I am not sure what the proper SO etiquette in a situation like this. – Amir Jan 10 '20 at 16:48
  • no problem, it is not a big matter. But if you found the answer in the suggested answer, it would make sense to close this question without accepting any answer. The answer below is just same as the suggested one. – SuleymanSah Jan 10 '20 at 16:50

1 Answers1

2

You should just do as the warning suggests - add handleKeyPress to the dependency array.

Like this:

// ...useEffect
}, [handleKeyPress]);

Since it's a function it shouldn't change, so you should be OK. But if it does change, you can wrap it in a useCallback, like this:

const handleKeyPress = useCallback((e) => {
  // code
}, []);

It works a bit like useEffect, where you can specify a second parameter as a dependency array.

  • Yes it looks like you are correct, I have to add it as a dependency AND wrap it in useCallback! Thank you! – Amir Jan 10 '20 at 14:15
  • You're welcome, I'm glad it worked! –  Jan 10 '20 at 14:18
  • I always found that eslint warning a bit strange. I've had use cases before where I need to use a function inside useEffect but dont need it added as a dependency. Wrapping the function in a useCallback adds further bulk thereofore I just add the // eslint-disable-next-line react-hooks/exhaustive-deps comment on the line above where the dependency array is defined. – P. Brew Jan 10 '20 at 14:30
  • 1
    @P.Brew Yea, doing further research into this, it does seem that there's quite a few use cases where you just have to add ignore to the warning, because it doesn't make sense to add it. Not in my example, but I've had others, like as you describe when you use an external library function for example, to initialize a datepicker or something. I know there are people who say there's probably better way to structure it but it seems like the problems hooks solve, open up new problems in other areas. – Amir Jan 10 '20 at 16:46