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.)
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
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?
By default the eslint rule doesn’t complain on the set function from useState. Because it never changes
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.
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.
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.
Looks pretty close to codifying the API provided by ReasonReact some time ago, into best practices.
I’m not just talking about useEffect. useMemo and useCallback also take dependency arrays, and for them, the dependency array is essential.
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).
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.
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.
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.
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?
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?
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.
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.
Don’t need to care about it, it is just a code checking rule of eslint.
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.