Why does eslint warn about useEffect having a missing dependency?

1

I tried to implement a simple data fetching example in ReScript-React, following the article How to fetch data with React Hooks, which I translated into ReScript as:

let (items, setItems) = React.useState(() => [])
React.useEffect0(() => {
  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
})

However, I get the following warning, which I’m surprised by because setItems is a setter function, not some data that could potentially change:

React Hook React.useEffect has a missing dependency: ‘setItems’. Either include it or remove the dependency array react-hooks/exhaustive-deps

Am I doing something incorrectly? What is the canonical/correct way to do fetch data using state and effect hooks in ReScript-React?

2

What’s the JS output? Maybe the useEffect function is not named the way that the eslint plugin recognises. If so, you could try adding the name to the list. Oh, wait, I got that wrong. Maybe the plugin doesn’t recognise the name (or the module) of the useState hook, so it doesn’t know that the setter is stable. There’s no list to add that to.

But of course, I wish some tool could do it for the ReScript code. There’s a reanalyze issue for that, by the by.

3

Here’s the JS output:

var match = React.useState(function () {
      return [];
    });
var setItems = match[1];
var items = match[0];
React.useEffect((function () {
        var __x = fetchData(undefined, undefined);
        __x.then(function (results) {
              if (results.TAG === /* Ok */0) {
                var data = results._0;
                Curry._1(setItems, (function (param) {
                        return data;
                      }));
                return Promise.resolve(undefined);
              }
              console.log("Fetching data failed with error: " + results._0);
              return Promise.resolve(undefined);
            });
        
      }), []);

Could the warning be because of the way the tuple destructor was translated? If my code is indeed correct, perhaps the way to fix this is to translate the line as follows?

const [items, setItems] = useState(...);
4

Well, you could try editing the JS and running eslint agains it. Just to check the hypothesis, that is.

5

Perhaps using the Uncurried submodule would help?

let (items, setItems) = React.Uncurried.useState(() => [])
React.useEffect0(() => {
  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
})

Could the warning be because of the way the tuple destructor was translated? If my code is indeed correct, perhaps the way to fix this is to translate the line as follows?

That could indeed be the case, I am not sure how the lint warning works.

6

Yes, editing the translated JS to look like the following does make the warning disappear:

const [items, setItems] = useState(...);

However, using Uncurried didn’t appear to help, it still generated the warning.

(Btw, I’m using create-react-app which uses webpack, which is the one showing these warnings.)

7

I think most ReScripters disable those checks on generated files anyway. Or they use something else than CRA, like Vite which does not force eslint on you.

See also: CRA eslint rules that are safe to disable with ReScript

8

Isn’t it because you’re using setItems inside the effect hook? So shouldn’t it be a dependency so the effect can run in case it changes?

9

By default the eslint rule doesn’t complain on the set function from useState. Because it never changes

1 Like
10

It’s because the lint rule doesn’t work unless you destructure the tuple returned from useState, and you can’t force the rescript compiler to output JS that does that.

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.

3 Likes
11

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.

3 Likes
12

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.

13

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

14

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

1 Like
15

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

3 Likes
16

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.

17

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

18

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.

19

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.

1 Like
20

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?