Search code examples
reactjseslintuse-effect

useEffect exhaustive-deps warning: what possible issues is this trying to avoid?


I realize there have been many questions about React's useEffect hook's dependency array, and the eslint warning that may arise from missing dependencies. Some other good discussions about this:

Although the causes & solutions for these warnings are very clear, one thing I'm struggling to grasp is the why. First, to quickly summarize, my understanding is that useEffect as three general purposes:

  • If its dependency array is omitted, it acts like both componentDidMount and componentDidUpdate: it runs the first time the component is rendered, and all subsequent renders.
  • If its dependency array is empty, it acts like componentDidMount: it can be used to run first-time initialization code (e.g. fetching initial data from the server, etc).
  • Otherwise, you can include in its dependency array any variables that you want it to 'watch'; it will act like componentDidMount, and then like componentDidUpdate when those variables change.
  • (For completeness: its return value can also be used to implement componentWillUnmount behavior, but that's beyond the scope of this issue).

I understand that eslint will complain if useEffect references any functions (or other variables) which are declared outside of useEffect. Per the above links, there are various ways to resolve these warnings: Move the function definition inside useEffect (if nothing outside useEffect needs to call it); add the function to the dependency array & memoize it with useCallback; disable the warning with // eslint-disable-next-line or // eslint-disable-line react-hooks/exhaustive-deps; etc. These all make sense. What I'm struggling to understand is WHY it even complains about this.

So, my specific questions:

  1. Practically speaking, I can't understand why you would ever want a reference to a local function in the dependency array. For example, if you have useEffect(() => { checkCurrentUser() }, []), and checkCurrentUser needs to be defined outside useEffect because it's called elsewhere, checkCurrentUser will be re-defined on every component render (which is why if we put it in the array, we should memoize it w/ useCallback). But to me, it makes no logical sense why one would ever want this in the dependency array at all. So why is this even a warning? What is this warning actually helping to prevent? Throughout all the times I've seen the warning, in every case, the application has behaved exactly as I expected/intended.

  2. If one of useEffect's primary purposes is to 'run once' like componentDidMount, and the way to achieve that behavior is to explicitly put an empty array, why does it hassle you if you specify an empty array? I can understand why it might make sense to warn if you have a dependency array that contains some references but not others - as in 'oops, you forgot some' - but the empty array is a specific, well-defined usage. To me, it doesn't seem to make sense that it would tell you 'an empty array is an issue,' as that's the only way to achieve componentDidMount-type initialization. It feels like the same thing as just warning you "don't use componentDidMount()." Why does it complain about an empty array, if that's supposedly one of the 3 main uses of useEffect?

Again, I do understand the circumstances in which the warning appears, and the ways to resolve it. I just struggle to understand the benefit of even having it, (1) in the case of functions (which we definitively know will change on every single render), and (2) in the case of an empty array (which are a specific usage).


Solution

  • (1) in the case of functions (which we definitively know will change on every single render)

    The reason for requiring to put functions in dependencies seems to be the same as the reason why they require you to put any other values in dependencies: what can happen is that a function you are using might be itself referencing some values from component scope, which can become stale.

    (2) in the case of an empty array (which are a specific usage).

    In this case I will reference answer cited by Dan Abramov from this comment:

    I think the biggest gotcha with class life cycle methods like componentDidMount is that we tend to think of it as an isolated method, but in fact it's part of a flow. If you reference something in componentDidMount you will most probably need to handle it in componentDidUpdate as well, or your component may get buggy. This is what the rule is trying to fix, you need to handle values over time....

    You may find more info on why they did it that way in that thread.