Why does eslint warn about useEffect having a missing dependency?

[benadamstyles]

By the way: in a large project, with enough people working on it, it is practically impossible to get all your dependency arrays correct without help from the linter. I think of myself as a very careful coder, and I still surprise myself by missing out dependencies quite often, which is only caught by eslint.

Writing React code without this lint rule is risky, and the kinds of bugs you can end up with if you miss out dependencies are the worst – those ones that are only discovered by users, when they do something weird like go back and forth between 2 views multiple times or click something too quickly.

I’d love to have some other way to check this, but right now all we have is running eslint against the generated JS, and that’s a lot better than trying to get the arrays right without any help from a computer.

[DZakh]

Sounds like you use useEffect in a wrong way, because dependency array should be needed only for optimisations. I recommend the talk https://youtu.be/Ck-e3hd3pKw?t=4002 to get an idea how it intended to be.

[cristianoc]

Looks pretty close to codifying the API provided by ReasonReact some time ago, into best practices.

[benadamstyles]

I’m not just talking about useEffect. useMemo and useCallback also take dependency arrays, and for them, the dependency array is essential.

[yawaramin]

@DZakh

dependency array should be needed only for optimisations.

My understanding was that it’s needed to tell React to run the effect specifically when a variable value changes. Not really an optimization. More an essential input to the system. Getting it wrong will lead to UI bugs (sounds like it’s happened to everybody).

[DZakh]

My point is that it’s an error prone usage not intended by react team (see strict mode). If you can’t remove all dependency arrays without breaking your application, then something is wrong.

[kirabo]

@DZakh

Sounds like you use useEffect in a wrong way

@DZakh how would you write my example from the opening post in ReScript?

[kirabo]

@benadamstyles

Our solution where I work is to always put setState functions in the dependency array anyway. Does no harm, and means you don’t have to remember which things to put in the array and which not to.

I like this, but then it’s not easy to see from the code that this effect is meant to run only once on mount because you have to use useEffect1 instead of useEffect0:

let (items, setItems) = React.useState(() => [])
React.useEffect1(() => {
  fetchData()->Js.Promise.then_(results => {
    switch results {
    | Belt.Result.Ok(data) =>
      setItems(_ => data)
      Js.Promise.resolve()
    | Belt.Result.Error(msg) =>
      Js.Console.log("Fetching data failed with error: " ++ msg)
      Js.Promise.resolve()
    }
  }, _)->ignore

  None
}, [setItems])

Why can’t the compiler destructure the output of useState automatically? That feels like the nicest solution.

[benadamstyles]

@kirabo

this effect is meant to run only once on mount

This is actually a bit misleading, especially in React’s new concurrent mode. There isn’t really such a thing as “on mount”, because components can be mounted multiple times and aborted, then mounted again. Effects are bits of imperative logic that run based on inputs; if you’re thinking about them in terms of component lifecycles, then it’s probably a good thing that your code is forcing you to think about them a bit differently.

[kirabo]

Aha, interesting, I didn’t know about React’s new concurrent mode. So what would be the way to fetch remote data in an app only once at the begininng?

[cristianoc]

Some form of analysis might be able to help with this. Hard to say precisely without fully understanding the problem.
To aid understanding: is this a react-specific idiosyncrasy or is it common to all UI frameworks?

[benadamstyles]

Well we’re getting into abstract territory here – but what does “only once at the beginning” mean, when you are using a framework (React) that tries to remove temporal concerns from your UI logic? In other words, in React, your UI is a function of state, and it is conceptually completely recreated every time your state changes. (In reality, it diffs between what’s already on screen and only updates what actually needs to be updated, but the point is that you shouldn’t ever have to think about “what was on screen before”).

In concurrent mode this becomes even more important because components can be “mounted” but then discarded, so whether or not you fetch data can no longer purely rely on the lifecycle of a component. It should instead rely on what you need to do with that data – for example, if you need to put that data into some state, then it’s right to rely on a setState function to decide whether to fetch the data.

This is all very abstract but it’s important to understand it when you get into some of the more complex questions when you’re writing React code (and you will always get into them eventually). What you’re already doing is correct, but you may need to just slightly shift your thinking about effects. Using an empty dependency array (or useEffect0) to signify “on mount” is cognitively pretty weird and obscure – and that’s for a good reason: it doesn’t really mean that at all.

[hoichi]

I think it’s React-specific (and maybe preact-specific too). For instance, SolidJS has similar hooks API, but as a compiled reactive framework, it uses an entirely different mechanism to track dependencies.

The problem that the react-hooks/exhaustive-deps rule is trying to solve is that the component functions are run over and over, and some of the values inside the function scopes change. But for some hooks that take callbacks as parameters, those callbacks’ closures can get out of sync with the values from the latest run of the component function (the stale closure problem). This is by design: you do not want to re-run, say, the useMemo callback on every rerender of the component. But neither do you want to not rerun whenever some value captured in the callback actually changes, so, for the lack of a better tracking mechanism, you have to be careful to explicitly list all those values in the hook’s dependency array.

[Mng12345]

Don’t need to care about it, it is just a code checking rule of eslint.

[cristianoc]

Maybe useMemo is a good example to look at in isolation.
I’ve done a fair amount of memoization, in the rare case where it is needed, over many years.
But can’t remember a single time where something like this would be the central focus of something to worry about when programming.
So now I’m puzzled as to what is different here.

[hoichi]

First of all, useMemo is normally used to memoize values, not functions. The function passed to useMemo is nullary and is supposed to simply produce values. For memoizing actual functions (event handlers and whatnot), there’s useCallback. But I think this is beside the point.

The differences to normal memoizing functions are:

1. Free variables
2. Where the cache resides
3. The stability of the returned function

1. Free and Bound Variables

Normally, the memoization helpers only care about bound variables. The arguments passed to the memoized version of the function are used as cache keys, and the original function is only called if there’s a cache miss. If the value in the cache was calculated using some free variables, and values of those has changed, the value won’t be re-evaluated.

So it’s a good practice to only memoize functions that don’t use any free values (or at least use some that don’t change during the app lifetime). But that advice is not very useful for functions created inside React components, because those functions often use values from the component function’s scope. And with that usage pattern, it’s easy to get stale values, so you have to take care of the free variables, and so (the React solution) you have to list them as explicit dependencies.

I think this is the difference most relevant to exhaustive dependencies, but let me go on anyway, in case you could benefit from more context

2. The Cache

With traditional memoization, to my knowledge, the cache is created either

1. inside the scope created by the memoize call,
2. inside the ES module from which the memoize function is imported, or
3. the cache instance is provided by the consumer.

Variant 1 could be OK with class components: you could memoize in the constructor once and be done with it. But with function components, where the functions are re-run repeatedly, that would just create a new empty cache every time, and there’d be no memoization, effectively. Well, there would if you’d use your value way more often that the component re-renders (i.e., the component function re-runs), but that’s not an awfully frequent case.

Variant 2 would mean all the instances of all the component share their cache, and variant 3 means some manual labour, and you’d have to do some non-trivial work to separate cache for instances of the same component.

So the React solution is that useMemo is stateful, in the same way as useState/useReducer: it tracks some internal state for every hook usage in every component instance. So there’s a separate cache for every (first) call of a useMemo hook inside every createElement call, and the consumers don’t have to do anything extra, other than abide the rules of hooks.

3. Stability

One concern that traditional memoization, to my knowledge, doesn’t have at all, but that is very important for modern React, is the stability of the references. And since it’s directly related to the hooks’ dependencies, I think it’s worth going into some detail.

Why stability matters

First of all, in React a stable value is a value that doesn’t change on re-renders. And when it comes to non-scalar values, it means that the reference doesn’t change. So, for clarity, creating a new value of [a, b] on every render is still unstable, even if both a and b haven’t changed.

Why does it matter? React uses diffing all the time, for the hooks’ dependencies and for the components’ props. Diffing is done shallowly, i.e., every property of a props object and every element of a dependency array is compared using Object.is. So if the value is an object (arrays and functions are objects in JS too), it’s compared by reference.

Comparison by reference blends very well with immutable patterns. There, new value === new reference. So React can compare values cheaply and then bail out of re-rendering (if it’s props) or out of running computations/effects (if it’s hook dependencies).

Also, React actually stabilises functions returned by useState or useReducer, so you can pass them as props or use them inside effects, and you never have to worry about handling the changes of their values, because those functions literally never change. And that’s why the eslint plugin ignores those functions: it knows you don’t have to list them as dependencies.

Where it breaks

The problems begins when you create values on the fly. Every function you create inside the component function, every array or object where you combine some values, and for that matter, every value created using a variant constructor with payload means a different value on every re-render.

Sure enough, if the value changes over time, you do want the reference to change. But only when the value actually changes. IOW, you want to enforce the new value === new reference invariant. Otherwise, if you get new refs without new values, you create false positives when diffing and your components can go into infinite loops. It happens all the time, and I don’t know any middle or senior React dev who doesn’t occasionally get bitten by that.

So this is something React docs tell about useCallback but don’t tell about useMemo: as often as not, it’s used not for “expensive computations”, but simply to stabilise the references between the actual changes in the values.

But, using useMemo, it’s easy to break the invariant the other way: if you forget to list a dependency, the hook fails to produce a new reference pointing to a new value. Now your reference is too stable.

And this is what the exhaustive deps part of the plugin tries to solve: it prevents you from overstabilising your values (or skipping your effects when you shouldn’t).

[cristianoc]

Thanks for the extensive explanation, this is really useful.

Can’t avoid wondering how it is that we ended up there.
The corresponding “manual” for memoization outside of React is: if you call this function again with the same value, the original result will be returned. Not much else to say.

[benadamstyles]

I think discussing memoization is a red herring. We’re complicating the issue, and actually a large proportion of dependency arrays in React code have nothing to do with memoization, but in fact have to do with when you run an effect. In my own experience, most usages of useMemo are in order to stabilize a dependency of a subsequent useEffect.

useEffect hooks need dependency arrays because of all the reasons explained in this thread – in brief, because you probably only want them to run when certain other things happen. These “other things” are usually represented as free values in the hook, as @hoichi explains. It’s very hard for humans to make sure that they put all of the free values used in the hook, into the dependency array. So, we use the eslint rule to check that for us.

To maybe move the conversation forward slightly: I’ve just installed the latest version of the VS Code extension (amazing work by the way ) and it has struck me how similar reanalyze is to eslint, from the DX perspective. I would love to see if we could make reanalyze do this kind of custom code checking for us. I’m imagining a plugin system for reanalyze…

[hoichi]

@benadamstyles

I think discussing memoization is a red herring. We’re complicating the issue, and actually a large proportion of dependency arrays in React code have nothing to do with memoization, but in fact have to do with when you run an effect. In my own experience, most usages of useMemo are in order to stabilize a dependency of a subsequent useEffect.

I agree about the typical usage of useMemo, and maybe part of the confusion is the naming. Maybe the hook should have been called useStableValue or something like that.

[DZakh]

Totally agree, eslint is the tool I miss the most, especially the no-restricted-syntax - ESLint - Pluggable JavaScript Linter