How to disable the rule react-hooks/exhaustive-deps?

I'm trying to disable the rule react-hooks/exhaustive-deps. I've tried adding "react-hooks/exhaustive-deps": "off", to .eslintrc without any luck.

How do I disable this rule? Thanks!

You can put // eslint-disable-next-line react-hooks/exhaustive-deps before the violating line if there is a good reason. Usually disabling it is a mistake and will significantly bite you later. (Most cases where people think it's safe to disable are not safe and lead to stale closures.)

Some references for how to write code with correctly declared dependencies:

https://reactjs.org/docs/hooks-faq.html#is-it-safe-to-omit-functions-from-the-list-of-dependencies
https://reactjs.org/docs/hooks-faq.html#what-can-i-do-if-my-effect-dependencies-change-too-often
Concrete cases where people thought it's safe but it wasn't, with correct solutions: [ESLint] Feedback for 'exhaustive-deps' lint rule react#14920 (comment)

If you post a complete example I can try to look at how to fix your case.

@gaearon

Thanks for the quick response @gaearon. I've been through those links you mentioned.

I've got about 60+ of these warnings throughout my app. Most of the cases involve a dependency that doesn't update the effect, e.g. when calling redux actions, props.someAction(). Can you explain why I need the prop functions in the dep list, is it to make sure I get the latest/correct ref to the function, as it might change?

I have a lot of CSS animation logic related to when a slide enters or leaves a slideshow, where I only need to know if one or two dependencies changed (isEntering, isLeaving). Is there any good reason why I need to list deps that I know aren't needed? If I list the rest of the deps then those might mess up the animation logic unless I add more checks in the effect which makes it a lot more complex than it needs to be. (at least in comparison with class components)

I could list a few examples if you would like, but like I explained above it's mostly prop functions causing these issues for me.

Can you explain why I need the prop functions in the dep list, is it to make sure I get the latest/correct ref to the function, as it might change?

Yes. Functions capture state and props inside of them. It just so happens that React Redux currently uses classes (or does it even anymore?) with methods that have stable identity. (And in that case adding them to the array doesn't hurt either.) But in general, you shouldn't expect that calling an arbitrary function from an effect is safe without declaring that function as a dependencies. Here's an example where "skipping" a function dependency produces a bug: react/react#14972 (comment). (This particular example is about memo but useEffect would have the same issue.)

I have a lot of CSS animation logic related to when a slide enters or leaves a slideshow, where I only need to know if one or two dependencies changed (isEntering, isLeaving). Is there any good reason why I need to list deps that I know aren't needed?

I'd be happy to look at a concrete case if you can provide one. Maybe the lint rule itself has some false positives that we can fix?

For us, one of the primary places we're receiving this warning is for useEffect hooks that we only want to run when the component mounts, and never again for that component.

As a concrete example, the app we're working on can display a small notification in the top right corner that automatically disappears after 15 seconds. Here's a super trimmed down version of what that might look like:

// Imports
import React, { useEffect } from 'react';


// Export component
export default (props) => {
    // Handle hiding this notification
    const hideSelf = () => {
        // In our case, this simply dispatches a Redux action
    };
    
    
    // Automatically hide the notification
    useEffect(() => {
        setTimeout(() => {
            hideSelf();
        }, 15000);
    }, []);
    
    
    // Return JSX
    return (
        <div className='notification'>
            Just an example <button onClick={hideSelf}>Close button</button>
        </div>
    );
};

This complains because hideSelf is not a dependency listed in the useEffect hook. But that's intentional; we only want that hook to run when this component mounts.

Is there a better way to design this? Or would this be a false positive?

I suppose we could wrap hideSelf in useCallback and then place the memoized function in the dependencies list, but that seems a bit misleading to someone reading the code in the future. The empty brackets very clearly indicate that the effect only runs when the component mounts. But placing a dependency in the array would imply otherwise, even if the effect never actually re-runs. Plus, it would actually be undesirable for the effect to run a second time, so we'd want the code to not allow that possibility in the first place.

@gaearon

The empty brackets very clearly indicate that the effect only runs when the component mounts

This is one of the reasons I don't want to add "unnecessary" things to my deps, as it makes the code look more complex and confusing. I totally agree that empty brackets tell you that it's CDM. There are many cases in my app where I just want to fetch once when the component mounts, but now I need to choose whether I want to add bunch of // eslint-disable-next-line react-hooks/exhaustive-deps lines or add the props as deps, but then I need to make sure the effect doesn't run any code when one of the deps changes. @gaearon I believe this is the main problem with this rule or hook. Perhaps we just need more hooks or options?

I understand that we need to re-think how we write our components using hooks, instead of trying to figure out how class lifecycle methods (CDM, CDU, etc.) correspond to hooks. But I must admit that this rule has made me feel less happy about hooks, mostly due to the complexity around simple things such as CDM. Like discussed above, I just want to use empty brackets in the deps knowing that the logic inside the effect will run once during mount. But now with the eslint rule I can't do that unless I start adding the // eslint-disable-line all over the place. I'm confident that I'm not the only one running into this problem, more people will probably realize this as they upgrade to CRA 3.0.

I've looked at many libraries using hooks and most of them violate this rule. When I use the useEffect hook I think about what dependencies I want to trigger the effect. But this rule forces me to add all the dependencies, making the code less readable/predictable, forcing me to add more logic inside the effect to make sure it doesn't run again (CDM, etc.). I'm sure some people disagree, but how would you write a simple CDM and make it obvious to programmers on your team what the effect is doing? In my case empty brackets would tell me that immediately. The "right" way is probably to re-think this w/o thinking about the class lifecycle methods, by utilizing useMemo so that the deps don't change, etc. but IMO that could make it harder to understand. I want readable code over many things and class components gave us that. I was getting pretty comfortable with hooks until now.

This just doesn't feel right. I'm now forced to add // eslint-disable-line to over 60 lines, because I want to understand my code.

My suggestion would be to add new hooks or an option that would disable this rule, before more people start running into this same problem. We are used to the class lifecycle methods, perhaps we just need more official hooks from the React team that correspond to them. E.g. useEffectOnce that doesn't need deps.

@WesCossick

@WesCossick Is there a reason you can't move the hideSelf inside the useEffect? If you can then it wont ask for it to be in the dependency list.

@WesCossick

@WesCossick Is there a reason you can't move the hideSelf inside the useEffect? If you can then it wont ask for it to be in the dependency list.

But then he would need to add the Redux action to the dependency list, since the hideSelf func dispatches a Redux action.

@raRaRa

Good point @raRaRa. So in this post and example @gaearon uses a useRef to store a callback and doesn't pass it into the dependency list.

https://overreacted.io/making-setinterval-declarative-with-react-hooks/
https://codesandbox.io/s/3499qqr565

So could @WesCossick do the same?

Yeah he could do the same by storing the Redux action in a useRef.

For me the main problem here is the complexity around achieving simple lifecycle methods such as componentDidMount without having to do all these tricks. I can't imagine going through this as a beginner in React. Another potential problem is code readability.

Imagine going through this code as a beginner:

Alright here's an useEffect, ah it has empty brackets so it's doing something like componentDidMount, hmm it's calling something within an useRef, ah the Redux action is stored in the useRef, etc.

I wouldn't want to be in his position.

@SCasarotto

@SCasarotto, in addition to what @raRaRa stated, it wouldn't be possible to move hideSelf into useEffect because it's also referenced by the onClick prop in the JSX. It would no longer be visible to that scope.

In regards to using useRef to store those functions, while that does appear to suppress the errors in this particular situation, it seems a bit... hacky. It's a misleading design wherein useRef is being used for something it's not intended for. According to the docs, useRef is "handy for keeping any mutable value around." The key word being mutable—which hideSelf is not. Nor is props.dispatch, from Redux.

Further, this and the previously mentioned useCallback method would both add a significant amount of boilerplate to nearly every component, especially ones that use Redux, thereby defeating one of the primary benefits of hooks.

I'd argue that the way it's designed presently, static code analysis for the react-hooks/exhaustive-deps rule simply isn't intelligent enough to make proper warnings (except for maybe the most basic uses of the useEffect hook). I'd argue that it takes human understanding of the nuances of the code to truly decide which dependencies should or should not be included.

I converted a large project to hooks and ran into the same issues as everyone else (200 class components). While initially I was skeptical about this rule, I ended up fixing all the warnings and it felt pretty good at the end. I think Dan's point that if you just have a useReducer with an empty dependency array, you are probably writting buggy code. In reality if you use redux and passing in some dispatch action for example, it felt unnecessary to list it as a dependency. While I agree with this sentiment, it does make you get into the habit of doing the right thing.

There were some issues, like I had a helper function that took in props, and if I just use the auto-fix feature it would pass in props as a dependency and cause an infinite mounting bug. So I ended up disabling auto-fixing for this rule in lint-staged. Also, refactoring these types of code felt pretty good but they can be very difficult to do if you have huge cDM and cDU functions.

So far in our experience the issues caused by missing dependencies have been significantly worse than losing the quick “[] means mount” visual shortcut.

One thing I’ve noticed is that it’s often Redux users who complain about this. I think it’s likely because React Redux encourages the “action creator” pattern where you get a bunch of methods and then call them individually. Of course, declaring every method and thinking about them can get confusing if you have dozens.

You wouldn’t have this problem if you weren’t using the “action creator” pattern. For example, with useReducer our recommendation is to put dispatch itself on its own context. Then any effect that needs to dispatch something just needs [dispatch] as dependencies which is easy to read and remember it never changes in practice. This is not currently idiomatic in React Redux — but Hooks were primarily designed to be idiomatic in React itself. React Redux adjusted to past React idioms, and I ancitipate it can adjust to this too.

Finally, please remember nothing prevents you from creating your own useMount Hook that does what you want, and then calling it. The lint rule won’t check it. In that case you’re explicitly choosing to forgo checks because you consider the verbosity of how your chosen idioms (like React Redux) combines with Hooks worse than the stale props or state.

@gryn

@gryn please provide a minimal repro using codesandbox.io rather than a code snippet.

useEffect(() => {
getResults();
// React Hook useEffect has a missing dependency: 'getResults'. Either include it or remove the dependency array react-hooks/exhaustive-deps
}, [query]);

https://codesandbox.io/s/vdevr

@samuelmataraso in that case you need to add that function as dependancy or declare it either outside of the component or inside the use effect and then call it

@bovas85

@samuelmataraso as @bovas85 said, you can do this:

  useEffect(() => {
    const getResults = async () => {
      const response = await axios.get(
        `http://hn.algolia.com/api/v1/search?query=${query}`
      );
      setResults(response.data.hits);
    };
    getResults();
  }, [query]);

@bugzpodder

@bugzpodder I'm not sure how complete you would want this example. I have left a large comment explaining the purpose of the effect: https://codesandbox.io/s/infallible-hellman-mdech

@gryn

Hi @gryn, the codesandbox should be a working app demonstrating real app behavior. Without it, we can't really play around with it and can only guess in terms of what the actual fix to be. If the codesandbox is just a stub, then it defeats the purpose of making it.

@bugzpodder

@bugzpodder I have updated the example to be "fully functional" (I still have not opted to include redux, but have stored the state in the top level component as a substitute). I have added documentation to the entire file, including what you should do to exercise the critical path.

MobX is a pretty good example of where injecting dependencies isn't necessary, but we get the annoying linting warnings with no way to turn them off.

Important realization here is that you will never need to specify dependencies for useEffect because there are simply none. Or to be more precise it does depend on store variable, but that's the same case as for React.useRef which value you shouldn't specify in dependencies also.

@bugzpodder

@bugzpodder ^

@gryn

@gryn thanks for producing a working example. here is my fork of your sandbox: https://codesandbox.io/s/elegant-austin-qmqob
Here are the changes I made:

checking location !== previousLocation in ScrollToTop doesn't make sense to me. It should always be different? (maybe this is not the case in your app)
scrollPosition should not be stored in redux and passed via props. The reason is that if it changes, the component re-renders because this prop changed.
What I did was changing scrollPosition to scrollRef from useRef. You can check out Dan's post on why this makes sense: https://overreacted.io/making-setinterval-declarative-with-react-hooks/

@bluepeter I don't have experience into mobx or why their documentation recommends an empty dependency array. In my experience with redux, the dispatch function never changes but I put it in the dependency array anyway. Does it work if you put the store in the dependency array (fully realizing that the docs did not recommend this approach)

@bugzpodder

@bugzpodder passing the stores into the dependency does seem to work. It seems somewhat inelegant, though.

@FredyC can probably articulate better than I can why it's not recommended to pass in MobX store dependencies into useEffect hooks?

@bluepeter

MobX "store" has stable referential integrity, it will never change unless doing that on purpose. It does work to specify it in dependencies, but as @bluepeter says, it's highly inelegant as it just clutters the code without any benefit.

I still haven't got into using ESLint with CRA, TSLint is good enough for now and because of these false positives I am probably going to avoid it for a long time. Similarly, @jaredpalmer had issues where it was forcing him to useCallback for unnecessary reasons. So thanks, but no thanks :)

@bugzpodder

@bugzpodder Jack, thank you for your feedback. I agree that the code is not excellent the way it is! I am working within a large application and sometimes we don't have the luxury of fixing what we find. However, I can fix the issue with the way the scroll saving works (we will still need to store it in redux, since the component recording the position will unmount when the page changes, but I can pass a ref down to the wrapped component, instead of a prop).

My take away from this is that you can use references for "consumable" information, and props for "actable" information. That is, "actable" means it either directly effects rendering, or can directly cause action. "Consumable", on the otherhand, is useful only during an action, and is not the cause of one.

Perhaps there are better words for it, and perhaps I am still missing some subtlety. In anycase, you've helped me move forward and hopefully improved my understanding. Thank you.

Since my comment about action creators got "buried" in the collapsed thread, I'll post it again here: #6880 (comment). TLDR: Hooks don't mesh very well with the "action creator" pattern. The recommendation is to useDispatch() directly. If you insist on splitting "presentational" and "container" components, you can always split them in two layers yourself, and put the useDispatch calls in the outer ones.

Navigation Menu

Repository navigation

How to disable the rule react-hooks/exhaustive-deps? #6880

41 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

How to disable the rule react-hooks/exhaustive-deps? #6880

Description

Activity

gaearon commented on Apr 23, 2019

raRaRa commented on Apr 23, 2019

gaearon commented on Apr 23, 2019

WesCossick commented on Apr 23, 2019

WesCossick commented on Apr 23, 2019

raRaRa commented on Apr 23, 2019

raRaRa commented on Apr 24, 2019

SCasarotto commented on Apr 24, 2019

raRaRa commented on Apr 24, 2019

SCasarotto commented on Apr 24, 2019

raRaRa commented on Apr 24, 2019

WesCossick commented on Apr 24, 2019

bugzpodder commented on Apr 24, 2019

gaearon commented on Apr 25, 2019

41 remaining items

bugzpodder commented on May 23, 2019

samuelematias commented on May 26, 2019

bovas85 commented on May 27, 2019

bugzpodder commented on May 27, 2019

gryn commented on May 28, 2019

bugzpodder commented on May 28, 2019

gryn commented on May 30, 2019

bluepeter commented on May 30, 2019

bluepeter commented on May 30, 2019

bugzpodder commented on May 31, 2019

bluepeter commented on May 31, 2019

danielkcz commented on May 31, 2019

gryn commented on May 31, 2019

gaearon commented on Aug 16, 2019

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions