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

[raRaRa]

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!

[gaearon]

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: https://github.com/facebook/react/issues/14920#issuecomment-471070149

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

[raRaRa]

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.

[gaearon]

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: https://github.com/facebook/react/issues/14972#issuecomment-468280039. (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?

[WesCossick]

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?

[WesCossick]

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.

[raRaRa]

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?

[raRaRa]

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.

[SCasarotto]

@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.

[raRaRa]

@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.

[SCasarotto]

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?

[raRaRa]

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.

[WesCossick]

@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.

[bugzpodder]

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.

[gaearon]

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.

[gaearon]

Regarding the hideSelf example above. It’s a bit tricky to discuss because there is no wider context. (Why do you keep UI state in Redux? Why does a component “hide itself” but stays mounted? Why isn’t this managed by its parent which shows the current list of notifications and cleans it up?)

But in this specific example the idiomatic solution is to put hideSelf inside the effect. If it dispatches a Redux action then put this action as a dependency. As you said, you expect Redux actions to “never change” — therefore there should be nothing bad about declaring them as deps. And if in the future this prop starts changing, your code will handle that.

I’d be happy to look into this in more detail if there was a sample project with a complete example.

[gaearon]

By the way. I know some people have very strong negative reaction to suppressing lint rules. Maybe it’s cultural and depends on a project.

We also had ~60 suppressions when we added this lint rule to FB codebase. Arguably we’re the biggest user of Hooks yet as we started earliest and use them very actively. Out of those 60 suppressions, about half were either actual bugs or bugs waiting to happen. For most of the rest, there was usually a way to restructure the code in a more idiomatic way as to not trigger the warning and make the code itself more resilient. I think by now we have only about ~30 suppressions left and people rarely introduce new ones.

We think the tradeoff is worth it and we don’t feel bad about adding suppressions to:

1. Existing code you don’t have time to analyse or fix
2. Places where you really think you know what you’re doing

I know some teams might have zero tolerance for suppressions which IMO is counterproductive but I could see how in such a climate this strategy is harder to make work. However again, you always have an escape hatch of making your own Hook with explicitly different semantics.

[audiolion]

Could there be some semantic thing to the rule where specifying a literal like ['onlyOnMount'] signifies you want to opt-in to didMount behavior and doesn't throw a warning?

sandbox: https://codesandbox.io/s/4xowvm7nr9

example:

import React from "react";
import ReactDOM from "react-dom";

function App() {
  const [, setCount] = React.useState(0);
  React.useEffect(() => {
    setInterval(() => setCount(c => c + 1), 3000);
  }, [setCount]);
  return (
    <div className="App">
      <h1>Hello CodeSandbox</h1>
      <h2>Start editing to see some magic happen!</h2>
      <UseEffectDidMount />
    </div>
  );
}

function UseEffectDidMount() {
  React.useEffect(() => {
    window.alert("I only happen once (didMount)");
  }, ["onlyOnMount"]);
  return null;
}

const rootElement = document.getElementById("root");
ReactDOM.render(<App />, rootElement);

[WesCossick]

After going through one of our apps and restructuring everything to avoid errors, I do agree that it's appropriate to work around the warnings thrown for using Redux functionality in a Hook. For us, that meant destructuring the dispatch function (const { dispatch } = props;) and passing dispatch into the dependencies of the necessary useEffect Hooks. While this does end up producing redundant boilerplate code, it seems appropriate for Redux to adapt to React, rather than the other way around.

Even still, after the restructuring we did have to suppress the react-hooks/exhaustive-deps quite a few times, though, by placing // eslint-disable-line react-hooks/exhaustive-deps on offending lines. And maybe we should rethink the prevailing view that ESLint rule suppressions as 100% evil. You have a point @gaearon that they can be productive in select cases.

I did notice, though, that almost exclusively we were suppressing the rule for useEffect Hooks that ran on mount only. There's a variety of reasons why we need effects that run on mount only in our own apps, and the example I provided above is just one of those, but I think it's pretty safe to say that this is a common need for many apps.

Sure, we could roll our own solution and create a reusable useMount Hook, but I wonder if, since the need for this Hook is so (presumably) widespread, it'd be better if everyone wasn't reinventing the wheel each time and React provided this type of Hook built-in? Obviously that's not a Create React App concern, but the potential need is more apparent since the upgrade to CRA 3.0.

FWIW, we also make heavy use of a modified version of useEffect that runs on updates only, rather than updates and mounts. This would actually be a really great candidate for a built-in Hook, since by creating our own Hook we're missing out on the linting. In this case, the linting would actually be much appreciated. At present, we can't have linting without forgoing a custom Hook and writing heavily redundant code.

[gaearon]

I'm happy to consider individual cases, like I did in https://github.com/facebook/react/issues/14920. (I asked for feedback at that point — it's unfortunate that you didn't see that thread and didn't offer your examples earlier.)

Please let's keep the discussion grounded in concrete CodeSandbox cases because we've already learned discussing this in abstract doesn't work. I'd also prefer that these examples show real use cases and not abstract ones like https://github.com/facebook/create-react-app/issues/6880#issuecomment-486669856 does. You can use https://github.com/facebook/react/issues/14920#issuecomment-471070149 as examples of how to explain your use case in a way that is descriptive enough to offer concrete advise.

As I mentioned earlier, the case of a component "hiding itself" is dubious, and it would make more sense to me if a parent "notifications list" component managed and cleaned up "old" notifications in state instead. I'm happy to look at more concrete cases which need to be in CodeSandbox form.

[WesCossick]

Hey @gaearon, we weren't using React at our company back on Feb 21 when you posted facebook/react#14920, so we couldn't have offered any feedback at that time.

I'd be happy to offer concrete CodeSandbox cases based on real use cases. But before I do so, I'd like to make sure we're on the same page.

This discussion has come a long way from its original question. In my most recent comment, I pointed out a common need in our apps is to have useEffect Hooks that either run on mount only or update only. For the first, we use useEffect and often suppress the ESLint rule. For the second, to avoid redundant code, we created a custom Hook.

Suppressing the ESLint rule is not necessarily bad, but seems undesirable. Creating a custom Hook sidesteps the ESLint rule entirely, which is definitely not desirable. I believe we agree on these two things, but correct me if I'm wrong.

With all this in mind, and assuming many apps would need a way for useEffect to run on either mount only or update only, I proposed that built-in support for these behaviors might be the best solution. If you agree, we should move this discussion to the React repository.

However, your latest comment seems to imply that it's almost always possible to refactor a use case so that it doesn't require mount only or update only behavior. And therefore it wouldn't make sense to even consider built-in support for those behaviors.

If that's a correct assessment of your comment, I'd be happy to provide CodeSandbox cases that demonstrate where we felt we needed those behaviors. At that point, I assume we'll decide whether those are indeed reasonable use cases or if the designs need to be refactored.

If that's not what your comment implies, and you don't want to consider built-in support for those behaviors, please explain what you would like the CodeSandbox cases to show.

[gaearon]

In my experience many cases people in which people want to get around the rule have more idiomatic fixes that don’t break it. But people don’t often see these solutions immediately because it takes a bit of a mental shift. Another thing I’ve noticed is that many proposed scenarios where somebody thinks they want “mount only” behavior have legit reasons to run on updates too. Such as when people forget props change over time, or don’t take parent state into account. My goal is to collect as many of those concrete scenarios as possible and to think through them all so that we have a set of recommendations I can later point people to. My intuition is that most of these recommendations will have idiomatic solutions instead of suppressing the rule. But maybe I’m wrong and I’d like to collect more examples where that doesn’t make sense too. If there’s a sufficient number or those maybe we can make the rule less aggressive, for example, or offer an easier way to partially suppress it. However, I haven’t seen enough cases that are justifiable yet. Hope that helps!

[gaearon]

The notion that some things need to happen only the first time user sees something is true. However associating it with “mount” is often a mistake. Such as conceptually a page navigation is often considered a fresh “appearance” even if technically the same ProfilePage component gets reused with different props. So, for example, scroll logic that resets scroll “on mount” (as one would intuitively do) is incorrect. Similarly, there are opposite cases where something appears for the first time, but the render result relates to an update. Such as if you needed to measure layout and then do a cascading render. Technically it’s an update but conceptually your “only run once” logic should wait for it. So the distinction between “mount” and “update”, while very familiar, is often the wrong tool. But each individual case is different. This is why I’d like to see concrete examples where you want to break the rule, with explanations that are user-centric rather than mount/update-oriented.

[audiolion]

I went to go make a codesandbox to demonstrate mount-only behavior, but after writing it up, I realized that there was a different way I could write it that would make it more resilient to re-renders. I feel like the big idea that people are missing is that they can have an effect that runs on mount, and properly cleans up after itself, and re-runs when deps change.

I see what you mean, and some of us are (slowly) getting it. While we want to map lifecycles to a hook equivalent mentally to better understand them, it isn't a perfect translation and we really do need to change our mental model around this to making our effects resilient to re-renders which helps avoid the bugs that you have been showcasing when props change over time, but the component doesn't remount.

I feel like most cases where you think you only want to run an effect once, you could run it again as deps change if you pass an appropriate cleanup function to the effect.

[raRaRa]

I think this all comes down to code readability & predictability. But I totally agree with @gaearon that it takes a bit of a mental shift to get this right.

I had an issue with useEffect where I was reading from the animation state and setting it again plus generating a style object from the updated animation, and If I had used animation as a dependency then it would have caused a recursion issue. So I fixed it by using the functional update form of setAnimation as explained here: https://reactjs.org/docs/hooks-faq.html#what-can-i-do-if-my-effect-dependencies-change-too-often

setAnimation((a) => {
    const updatedAnim = {
        ...a,
        depth: -355,
    }
    setStyleObj(generateStylesObj(updatedAnim))
    return updatedAnim
})

But is this safe? using the functional update form of setAnimation with another state setter inside.

And here's the code before the fix, which required the animation state to be dependency and caused recursion:

const updatedAnim = {
   ...animation,
   depth: -355,
}

setAnimation(updatedAnim)
setStyleObj(generateStyles(updatedAnim))

I'm basically generating a style object from the animation state. Another potential solution is to use useMemo that updates the style obj when the animation state changes:

const styleObj = React.useMemo(() => ({
    opacity: animation.opacity,
    WebkitTransform: `
        perspective(${animation.perspective}) 
        translate3d(${animation.offsetX}%, ${animation.offsetY}%, ${animation.depth}px)
        rotateZ(${animation.rotation}deg)`,
    transform: `
        perspective(${animation.perspective}) 
        translate3d(${animation.offsetX}%, ${animation.offsetY}%, ${animation.depth}px)
        rotateZ(${animation.rotation}deg)`,
}), [animation])

Would it be better/safer to use the useMemo trick rather than manually setting the style obj where animation changes?

To give context on how the styleObj is used:

<div style={styleObj} />

[WesCossick]

That makes sense, @gaearon. I'm relatively new to React, so I'm definitely open to rethinking how we approach design choices for our apps.

It seems most of the times we need the useEffect Hook to run on mount only or update only (vs. mount and update) is in the context of some type of state recovery upon navigation. Below is one example.

Recovering form state

As a user navigates around a single page app, the goal is to have form state recovered to what they left it as. This only applies to using the browser's forward and back buttons to navigate to points in history they've already visited.

Because it's a real world example, it's powered in-part by React Router and Formik.

Here's the CodeSandbox editor and here's just the rendered app. For browser history state to work properly, you cannot use the CodeSandbox editor's forward, back, or refresh buttons; you must use just the rendered app in your browser.

Recovery in action:

1. Open the rendered app
2. Fill out the first and/or last name fields
3. Click the "Some other page" link
4. Use your browser back button to return to the "Example form" page
5. Text should be recovered

The form-persist.js file shows a useEffect Hook that runs when the form first appears and never again (i.e. "mount only," but we're avoiding those terms). This is responsible for recovering Formik's state from the browser's history. It breaks the react-hooks/exhaustive-deps rule intentionally.

That file also uses a custom useUpdateOnlyEffect Hook, which is a version of useEffect that only runs when its dependencies update. This is responsible for saving updates to the browser's history. It breaks the react-hooks/exhaustive-deps rule unintentionally, but we wouldn't know that because the linter doesn't work for custom Hooks.

If the useEffect Hook is given [props.formik] as its dependencies, it results in an endless loop (see console output). If the useUpdateOnlyEffect is turned into a regular useEffect Hook, the initial values are saved into the browser state before it's recovered, so the recovery ends up pulling the initial values and not what the user actually typed.

Possible alternative designs for the useUpdateOnlyEffect Hook

You might initially think to change the order of the two Hooks in the form-persist.jsfile. If the useUpdateOnlyEffect Hook comes second, it can be converted to a standard useEffect Hook and the linter will catch the unintentional rule break. However, this is fragile code. Correct me if I'm wrong, but as far as I'm aware, React does not guarantee that the order in which it executes Hooks won't change from one version of React to the next. What if in a future version React decides to execute Hooks in a different order? Or what if simply something about your own code causes the two Hooks to be executed out-of-order? The two Hooks would be at the mercy of the order they're executed in, and the order isn't something that's documented or guaranteed to be permanent.

Another alternative would be to debounce or otherwise delay the saving of Formik's state. But how long do you decide to delay it? 100ms? 500ms? What if the user's computer is really slow? Timing becomes a significant concern, as the recovery and saving could still occur out-of-order.

Finally, you could simply not use a custom Hook, and instead write the logic with refs everywhere you need a Hook that runs on updates only. But that creates an unnecessary amount of redundant boilerplate code. Hence, a custom Hook.

Possible alternative designs for the useEffect Hook

Unfortunately, I'm not aware of any design changes that could be made to the Hook that recovers the state such that it doesn't intentionally break the ESLint rule.

---

I'd love to hear your thoughts, @gaearon, on this concrete example. Is there a way we could approach this differently?

[gaearon]

@raRaRa Do you mind posting a full example for animation case that I could run? Usually when you start reaching for the functional setState, the code can be further simplified with useReducer.

[gaearon]

@WesCossick

Thanks for a concrete example. I’m on my phone so can’t test it in depth. But I’m curious — is there any particular reason you want to fill in the initial state from an effect, as opposed to actually using the initial state for this? (In case of Formik, I imagine it has some prop to set initial values.) I think that would remove your need for a “first render only” effect to fill the state.

[gaearon]

Curiously your desire to break the rule here does uncover the problem in an app. I think that if you open it in two tabs, diverging edits from those would overwrite one another. So one of them wouldn’t get restored. While not likely, it can be a problem. To solve it you could store some kind of revision ID in the storage too. In that case you would want to run the effect on update too. Instead of always replacing the state, it would offer to either resolve conflict (if unexpected revision is in the store), or ignore the update (if revision belongs to this tab). That could let you keep tabs in sync.

https://overreacted.io/writing-resilient-components/#principle-3-no-component-is-a-singleton is a relevant principle.

Of course it might be a higher bar than most apps care about. But I’m just pointing out that even here, the lint rule points out a deeper flaw in the design and forces you to confront it. (Or hack around it.)

[WesCossick]

@gaearon Good question. With Formik, you need to define the initial values as a prop on the <Formik /> component, which means you'd have to determine them outside of that component. But the logic for remembering changes to props.formik.values needs to occur inside the <Formik /> component, with help from Formik's connect() HoC, otherwise it doesn't have access to the values. So to go the route of using the initial values for recovery, the code for storing + recovering would be split.

The design we've implemented keeps the persistence logic completely encapsulated in its own component that can easily be included in any Formik form simply by adding <FormPerist />. It's very intuitive and flexible.

Also, to address your second comment, the design stores and recovers the form state from the browser's history API. Each page view maintains its own isolated state, which isn't shared across tabs, even across different points in the browser history within the same tab.

[mattwoodco]

This has been a valuable discussion, now that I'm seeing this lint warning all throughout my codebase.

I understand the mental shift needed, and the risk in not defining dependencies, but I also feel the pain others have mentioned regarding the added complexity.

Like others, I would prefer to have simple mechanism to fetch some stuff on mount.

useEffect(() => {
  authedFetch(fetchParameter);
}, [fetchParameter]);

But in order to make this safe, I need to do the following...

const memoizedFetch = useRef();

useEffect(() => {
  memoizedFetch.current = authedFetch;
}, [authedFetch]);

useEffect(() => {
  memoizedFetch.current(fetchParameter);
}, [fetchParameter]);

Is there a more concise workaround? The solution to this particular issue feels like it offsets a few of the benefits we gained when we started simplifying our code with Hooks.

[estaub]

@mattwoodnyc I may just having a case of the stupids, but your example doesn't make sense to me. Did you, perhaps, intend to write

useEffect(() => {
  memoizedFetch.current = authedFetch;
}, [])  // or perhaps [authedFetch]

You may want to consider useCallback(), but it's hard to guess without understanding the life of authedFetch.

[mattwoodco]

@estaub. I've updated the code above. authedFetch was a contrived example of a custom wrapped fetch with some extra metadata in the request header

So using useCallback, am I correct in assuming that the cleanest solution is as follows?....

const memoizedFetch = useCallback(authedFetch,[authedFetch])

useEffect(() => {
  memoizedFetch(fetchParameter);
}, [fetchParameter]);

[bugzpodder]

Why is

useEffect(() => {
  authedFetch(fetchParameter);
}, [fetchParameter]);

not considered safe?

Are you talking about eslint complaining about authedFetch? How about:

useEffect(() => {
  authedFetch(fetchParameter);
}, [fetchParameter, authedFetch]);

[mattwoodco]

Yes, @bugzpodder, I was referring to the eslint complaining.

The problem I was having was that, when adding the function as a dependency, the effect kept running . And the only solution was the above.

[bugzpodder]

Can you show me how authedFetch is defined? If its an inline function, then yes you need to wrap useCallback. Personally I don't see the need to do useRef here.

[mattwoodco]

@bugzpodder, @estaub, the authedFetch might be something like the following:

const genericAuthedFetch = method = async (endpoint, obj) => { 
  const authedRequestObj = {
    method,
    headers: {
      // TOKEN, ETC.
    }
  };
  try {
    if(obj) {
      authedRequestObj.body = JSON.stringify(obj);
    }
    const response = await fetch(endpoint, authedRequestObj);
    const data  = await response.json();
    return data;
  } catch (error) {
    return Promise.reject(error);
  }
}

const authedFetch = {
  get: genericAuthedFetch("GET"),
  post: genericAuthedFetch("POST"),
  put: genericAuthedFetch("PUT"),
  delete: genericAuthedFetch("DELETE")
}

So, yes, the more I'm working with it, I'm seeing that useRef was overkill, and useCallback is the way to go...

const memoizedFetch = useCallback(authedFetch.get,[authedFetch.get]);

useEffect(() => {
  memoizedFetch(fetchParameter);
}, [fetchParameter]);

[estaub]

@mattwoodnyc authedFetch is not a function, so useCallback is clearly wrong. Are genericAuthedFetch and authedFetch statically defined (outside of any function or class), or are they defined inside the function component? If defined inside the component, are they dependent on anything else defined inside the function?

[mattwoodco]

@estaub — My mistake... authedFetch was missing the method property...

I've edited the code above

const memoizedFetch = useCallback(authedFetch.get,[authedFetch.get]);

useEffect(() => {
  memoizedFetch(fetchParameter);
}, [fetchParameter]);

I'm currently defining genericAuthedFetch and authedFetch as functions inside another custom hook, and then I'm passing authedFetch around using a context provider at the app's root.

[bugzpodder]

const memoizedFetch = useCallback(authedFetch.get,[authedFetch.get]);

This doesn't seem right to me. memoizedFetch changes whenever authedFetch.get changes, so I don't see a benefit of defining it.

authedFetch.get is not something that would change, so the code

useEffect(() => {
  authedFetch.get(fetchParameter);
}, [fetchParameter, authedFetch.get]);

should work fine.

I wasn't asking as much for the definition of authedFetch.get, but rather where you construct the authedFetch object. If you do something like:

const Component = (props) => {
  const authedFetch = {
    get: genericAuthedFetch("GET"),
    post: genericAuthedFetch("POST"),
    put: genericAuthedFetch("PUT"),
    delete: genericAuthedFetch("DELETE")
  }
  useEffect(() => {
    authedFetch.get(fetchParameter);
  }, [fetchParameter, authedFetch.get]);
}

That wouldn't work well because authedFetch is a new object each time. However if you do something like:

import { authedFetch } from "./api";
const Component = (props) => {
  useEffect(() => {
    authedFetch.get(fetchParameter);
  }, [fetchParameter, authedFetch.get]);
}

This would work well and you don't need to useCallback. useCallback helps if you define your fetcher inside your component.

[gaearon]

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

So I wanted to touch on Redux use case a little bit. I think the "action creator as a prop" pattern is a major hindrance to using Hooks and I'd recommend to avoid it if you want Hooks to feel more natural.

🔴 For example, I don't recommend writing code like this:

function Component() {
  const actions = ... // Somehow bind them or get them from props
  useEffect(() => {
    actions.foo()
    actions.bar()
    doSomething().then(() => {
      actions.baz()
    })
  }, [actions.foo, actions.bar, actions.baz]); // 😔 So many dependencies
  // ...
}

✅ Instead I recommend to use dispatch directly:

import {foo, bar, baz} from './actions';

function Component() {
  const dispatch = ... // Somehow get it from the store
  useEffect(() => {
    dispatch(foo())
    dispatch(bar())
    doSomething().then(() => {
      dispatch(baz())
    })
  }, [dispatch]); // 🙌 Minimal dependencies
  // ...
}

Since action creators themselves are at top level and there's no "binding", there's no need to include any of them as the dependencies. The intermediate actions object in component scope is just an unnecessary indirection that doesn't serve any real purpose. So avoid it.

So how do you get dispatch? With old connect(), that's what you get in props if you omit second argument. So it's super easy to obtain.

With new alpha Redux Hooks, useDispatch will hand it to you.

The same principle applies to useReducer(). Also remember you can often skip action creators altogether, especially if you have a good type system that catches typos and wrong action shapes.

Hope this helps.

[gaearon]

@bugzpodder

Regarding your last example:

import { authedFetch } from "./api";
const Component = (props) => {
  useEffect(() => {
    authedFetch.get(fetchParameter);
  }, [fetchParameter, authedFetch.get]);
}

In this example, there is absolutely no need to have authedFetch.get as a dependency because it's not defined inside the component render.

Just this would work fine:

import { authedFetch } from "./api";
const Component = (props) => {
  useEffect(() => {
    authedFetch.get(fetchParameter);
  }, [fetchParameter]);
}

[WesCossick]

Hey @gaearon, the discussion has carried on quite a bit, but I wanted to see if you had any further thoughts on the state recovery example I provided? Is it possible to refactor that example?

[sekoyo]

Regardless it would be good if we could turn off this rule in eslint for us people who want to live on the edge. It has good intentions but I don't want to unnecessarily memoize all the functions I use in an effect, i.e.

const authed = useStore(state => state.user.authed);
const setAuthModalOpen = useActions(actions => actions.app.setAuthModalOpen);

useEffect(() => {
  setAuthModalOpen(!authed);
}, [authed]);

I regularly call methods created in the render function, would you guys recommend memoizing them all? I think this would increase code complexity in this case? Or else it would be handy to choose to consciously ignore them (but not everything):

useEffect(() => {
  setAuthModalOpen(!authed);
}, [authed]); // ignore: setAuthModalOpen, fn2, ...

So far I've found I'm looking for variable changes, not functions.

[bugzpodder]

The lint rule is able to catch most subtle bugs so overall I think it's a net win.

[bovas85]

I'm using firebase and the db dependancy is failing eslint, but I can't use that as dependancy because it updates every .2s (it's a realtime database).

Any ideas?

useEffect(() => {
    const updateStore = async () => {
      const goalsRef = db.collection(user.uid).doc('goals')
      const res = await goalsRef.get()
      const data = await res.data()
      const goalsArray = data && await data.goals

      if (goalsArray) {
        setGoals(goalsArray)
        return
      }

      await db
        .collection(user.uid)
        .doc('goals')
        .set({
          goals: [newGoal]
        })
      setGoals(goalsArray)
    }
    updateStore()
  }, [user]) // here [db] is missing, but adding it causes an infinite loop of the effect as db keeps updating

[SCasarotto]

I use firebase a lot. I just never store a reference to db I just always firebase.firestore()

[bugzpodder]

Move declaration of db inside useEffect may help. You haven't shown how you defined db.

[bovas85]

@SCasarotto can you show an example of how you would query and update it with hooks? Moving the code above outside the component also works but I need some props to be sent to the method

[SCasarotto]

From one of my production apps I recently built with hooks:

import { useState, useEffect } from "react";
import firebase from "firebase/app";
import "firebase/firestore";

import { collectionToDataArray } from "../../../helpers";

export const useOrganizations = () => {
  const [organizationArray, setOrganizationArray] = useState([]);

  useEffect(() => {
    const usersWatcher = firebase
      .firestore()
      .collection("Organization")
      .onSnapshot(
        snapshot => {
          setOrganizationArray(collectionToDataArray(snapshot));
        },
        e => {
          console.error(e);
        }
      );
    return () => {
      usersWatcher();
    };
  }, []);

  return organizationArray;
};

As for "query and update" see something like:

useEffect(() => {
  const userWatcher = firebase
    .firestore()
    .doc(`Coordinator/${uid}`)
    .onSnapshot(
      snapshot => {
        dispatch({ type: "fetch", payload: docToDataObject(snapshot) });
      },
      e => {
        console.error(e);
      }
    );
  return () => {
    userWatcher();
  };
}, [uid]);

[bovas85]

Interesting @SCasarotto, why do you return the disposer to run the watcher? Suppose I have an input that I change in client, how would you then update the firestore?