1

I'm using useEffect in a React function component like so:

useEffect(() => {
  const handleRouteChangeComplete = () => {
    window.scrollTo(0, 0);
    userInterface.dispatch(closeNavigation());
  };

  Router.events.on('routeChangeComplete', handleRouteChangeComplete);
}, []);

The second (array) option on useEffect gives me the following linting error:

React Hook useEffect has a missing dependency: 'userInterface'. Either include it or remove the dependency array. eslint(react-hooks/exhaustive-deps)

I don't think either of the options in the linting error are correct.

  • I only want to initialise the event listener once.
  • If I remove the array option, useEffect will behave like a componentDidUpdate and could execute the code more than once
  • If I add userInterface as a dependency on useEffect it would also execute the code more than once if that context updates

Am I missing something here or is the linting forcing me to write incorrect logic?

I would disable the rule in this one instance but I have 4 other instances of a similar error in my app.

CaribouCode
  • 13,998
  • 28
  • 102
  • 174
  • Are you going to replace the current `userInterface` with another instance? If you're not going to replace it, using it as a dependency would not call the `useEffect` after the 1st time. – Ori Drori May 02 '20 at 15:27
  • 1
    @OriDrori `userInterface.state` may change but `userInterface.dispatch` should not. But the way my current code is written, adding `userInterface` as a dependency to the `useEffect` will cause the code to execute again if `userInterface.state` changes. – CaribouCode May 02 '20 at 15:45

1 Answers1

2

There's a reason for the eslint error and that's to prevent hard to notice bugs due to stale values. Essentially the lint error is just telling you that your userInterface variable is going to be stale within the effect.

It isn't necessarily a good idea to mute that error because then if you add more dependencies you might not realize why things aren't updating like you expect.

Another post about the same thing:

useEffect dependency array and ESLint exhaustive-deps rule

https://github.com/facebook/create-react-app/issues/6880

One major thing you should keep in mind is making sure you clean-up your actions regardless of if it's only being done on mount, because sometimes the effects can live beyond the life of the component if you don't clean it up.

You have several different options for how to fix this, if dispatch is stable (which based on the name, it usually is), then you can pull dispatch off of the userInterface variable and then add it to the dependency array.

const { dispatch } = userInterface;

useEffect(() => {
  const handleRouteChangeComplete = () => {
    window.scrollTo(0, 0);
    dispatch(closeNavigation());
  };

  Router.events.on('routeChangeComplete', handleRouteChangeComplete);
  () => {
    Router.events.off('routeChangeComplete', handleRouteChangeComplete);
  };
}, [dispatch]);

If pulling out the dispatch value isn't an option, then you can use a ref to make sure you keep the latest version of userInterface in a stable manner. This is a common enough task at times that you might want to extract the logic to a custom hook to get a ref of a value.

const userInterfaceRef = useRef(userInterface);
useEffect(() => {
  userInterfaceRef.current = userInterface;
}, [userInterface]);

useEffect(() => {
  const handleRouteChangeComplete = () => {
    window.scrollTo(0, 0);
    userInterfaceRef.current.dispatch(closeNavigation());
  };

  Router.events.on('routeChangeComplete', handleRouteChangeComplete);
  () => {
    Router.events.off('routeChangeComplete', handleRouteChangeComplete);
  };
}, []);

The reason for the seemingly extra useEffect here is because unless you know for sure that the userInterface will never change, then you need to keep it up to date or else the userInterfaceRef will be stale. The reason I made the ref of userInterface instead of the dispatch function is because this way you can use other properties of the userInterface within the effect without any issues.

If you need to have dependencies in your effect that you don't want to restart the effect, use the ref option I described to ensure you have the latest value without the need for re-running the effect every time they change.

If you are adding an on handler to something imperatively in an effect, you should make sure to clean it up. It's a good habit to get into.

Zachary Haber
  • 10,376
  • 1
  • 17
  • 31
  • Hey, thanks for the reply! Couple of follow up questions though. In your ref example, since dispatch should never change, isn't the first `useEffect` to keep it in sync with the ref unnecessary? Secondly, does this linting rule exist because there's a good reason to declare all dependencies for a `useEffect` even if you don't want to re-execute the code when certain ones change? Also I do have the clean up on my `componentDidMount` but removed for the sake of brevity in this question. Thanks for pointing it out though! – CaribouCode May 02 '20 at 15:43
  • React functional components are such a bodged implementation by Facebook. They create more problems than they solve. – Organic Oct 22 '21 at 14:06