Proposal: enforce immutability for hook dependencies instead of exhaustive deps
Hi, so there is a long discussion for and against CRA's react-hooks/exhaustive-deps lint rules in the issue #6880.
Without going to further analyze that, it's safe to assume that the exhaustive-deps rule causes some unwanted busywork and even introduction of undesired side-effects which the developers might have not intended.
To illustrate what I mean: if I have a regular functional component with a getTodos() API call in the useEffect hook, as of now exhaustive-deps requires me to add the getTodos() to the dependency array. Yet me, as the developer, have no intention ever changing the function thus it should be immutable during the whole execution of the component. Moreover, it would be a bug if it ever did so which would make adding it as a dependency contradictory to my original aim of only running the hook once per mount. The correct behavior in such cases would be to throw an error, not doing a very strange recomputation of the useEffect hook. And this is notwithstanding the annoying overhead of having to add every single thing imaginable to the dependency array.
I hope my argumentation makes sense and the participants of the original thread, including @gaearon, can attest that this is an issue worth debating about.
The solution to this matter, the enforcement of immutability of the props/variables, is another issue entirely and I do not have any idea how such thing could be implemented or even checked with a linter. Perhaps this problem can only be solved with adding additional immutable functionality or error checking to React, which seems a bit of a long-shot.
In the meantime, I'll try to figure my own approach of dealing with the issue. Perhaps I'll just disable the rule and trust that I remember to add only the variables that can and should change during the lifecycle of a component. But for the sake of my sanity I won't be bothering to waste my time on adding every variable possible to the dependency array, when they should never change in the first place. If React could throw errors when those values did change unnecessarily, that would already be a pretty big improvement.
I'm having a similar issue, and I can break it down very simply.
When using useState, CRA correctly identifies that the second element in the returned array (the setter) does NOT need to be included in any dependency arrays. See the following:
import React, {useCallback, useState} from 'react';
export default () => {
const [counter, setCounter] = useState(0);
const onButtonClick = useCallback(() => {
setCounter(oldCounterValue => oldCounterValue + 1);
}, []);
return <>
<span>{counter}</span>
<span><button type="button" onClick={onButtonClick}>Increment</button></span>
</>;
}
No warning gets generated from the above code. However, if a new hook is created that is simply a wrapper for useState, warnings get generated. See the following:
import React, {useCallback, useState} from 'react';
const useCounter = initialValue => {
const [counter, setCounter] = useState(initialValue);
const incrementCounter = useCallback(() => {
setCounter(oldCounterValue => oldCounterValue + 1);
}, []);
return [counter, incrementCounter];
}
export default () => {
const [counter, incrementCounter] = useCounter(0);
const onButtonClick = useCallback(() => {
incrementCounter();
}, []);
return <>
<span>{counter}</span>
<span><button type="button" onClick={onButtonClick}>Increment</button></span>
</>;
}
This code generates the warning Line 18:8: React Hook useCallback has a missing dependency: 'incrementCounter'. Either include it or remove the dependency array react-hooks/exhaustive-deps.
Analyzing the second code block by hand it is obvious that the second array item returned by the useCounter hook is just as immutable as the setter returned by the useState hook. Is there a way we can tell the linter to not generate warnings in cases like this?
It is also warns about any 3rd party library. Say you use Redux and use dispatch in your useEffect. Or Actions. Really anything and it says it needs to be in the dependency array. It isn't that big of a deal but it causes issues with developers adding everything into the dependency array when they really aren't needed since they never change.