Understanding the React Hooks 'exhaustive-deps' lint rule
The main purpose of the exhaustive-deps
warning is to prevent the developers from missing dependencies inside their effect and lost some behavior.
Dan abramov – developer on Facebook core – strongly recommend to keep that rule enabled.
For the case of passing functions as dependencies, there is a dedicated chapter in the React FAQ:
https://reactjs.org/docs/hooks-faq.html#is-it-safe-to-omit-functions-from-the-list-of-dependencies
tl;dr
If you have to put a function inside your dependencies array:
- Put the function outside of your component, so you are sure that the reference won't be changed on each render.
- If you can, call the function outside of your effect, and just use the result as dependency.
- If the function must be declared in your component scope, you have to memoize the function reference by using the
useCallback
hook. The reference will be changed only if the dependencies of the callback function change.
The reason the linter rule wants onChange
to go into the useEffect
hook is because it's possible for onChange
to change between renders, and the lint rule is intended to prevent that sort of "stale data" reference.
For example:
const MyParentComponent = () => {
const onChange = (value) => { console.log(value); }
return <MyCustomComponent onChange={onChange} />
}
Every single render of MyParentComponent
will pass a different onChange
function to MyCustomComponent
.
In your specific case, you probably don't care: you only want to call onChange
when the value changes, not when the onChange
function changes. However, that's not clear from how you're using useEffect
.
The root here is that your useEffect
is somewhat unidiomatic.
useEffect
is best used for side-effects, but here you're using it as a sort of "subscription" concept, like: "do X when Y changes". That does sort of work functionally, due to the mechanics of the deps
array, (though in this case you're also calling onChange
on initial render, which is probably unwanted), but it's not the intended purpose.
Calling onChange
really isn't a side-effect here, it's just an effect of triggering the onChange
event for <input>
. So I do think your second version that calls both onChange
and setValue
together is more idiomatic.
If there were other ways of setting the value (e.g. a clear button), constantly having to remember to call onChange
might be tedious, so I might write this as:
const MyCustomComponent = ({onChange}) => {
const [value, _setValue] = useState('');
// Always call onChange when we set the new value
const setValue = (newVal) => {
onChange(newVal);
_setValue(newVal);
}
return (
<input value={value} type='text' onChange={e => setValue(e.target.value)}></input>
<button onClick={() => setValue("")}>Clear</button>
)
}
But at this point this is hair-splitting.