165

I'm having a hard time understanding the 'exhaustive-deps' lint rule.

I already read this post and this post but I could not find an answer.

Here is a simple React component with the lint issue:

const MyCustomComponent = ({onChange}) => {
    const [value, setValue] = useState('');

    useEffect(() => {
        onChange(value);
    }, [value]);

    return (
        <input 
           value={value} 
           type='text' 
           onChange={(event) => setValue(event.target.value)}>
        </input>
    )
} 

It requires me to add onChange to the useEffect dependencies array. But in my understanding onChange will never change, so it should not be there.

Usually I manage it like this:

const MyCustomComponent = ({onChange}) => {
    const [value, setValue] = useState('');

    const handleChange = (event) => {
        setValue(event.target.value);
        onChange(event.target.value)
    }

    return (
        <input 
           value={value} 
           type='text'
           onChange={handleChange}>
        </input> ​
    )
} 

Why the lint? Any clear explanation about the lint rule for the first example?

Or should I not be using useEffect here? (I'm a noob with hooks)

spazm
  • 4,399
  • 31
  • 30
Logan Wlv
  • 3,274
  • 5
  • 32
  • 54
  • 1
    Yeah there's no reason to use an effect here, `useEffect` is very similar to a combination of `componentWillMount`, `componentDidMount`, and when you return a function from `useEffect` that function is treated as `componentWillUnmount`. All you're handling at the moment is a simple state change and the `useState` hook is enough to accomplish that – Michael Abeln Nov 14 '19 at 21:47
  • onChange won’t change, but value will. – Dave Newton Nov 14 '19 at 21:48
  • 1
    @MikeAbeln They’re not just changing state, they’re also calling the click handler passed in as a prop. – Dave Newton Nov 14 '19 at 21:49
  • 2
    @DaveNewton Good catch, that escaped me. Still, `useEffect` doesn't seem appropriate. The prop `onChange` can easily be moved to the body of the `onChange` method of the `input`. Although it should be renamed for clarity. Basically the second example OP gave in the question. – Michael Abeln Nov 14 '19 at 22:36
  • {onChange} is a callBack to the parent component so it gets updated with the input value on change. (in this example) – Logan Wlv Nov 14 '19 at 22:38
  • I've opened a feature request https://github.com/facebook/react/issues/22132 and a codesandbox demonstrating various approaches https://codesandbox.io/s/fancy-sea-zj5e4, I'd like to get your opinion. I think `useEffect` could benefit from an additional `triggers` parameter. – Eric Burel Aug 19 '21 at 08:17

2 Answers2

148

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.

Retsam
  • 30,909
  • 11
  • 68
  • 90
  • 18
    That was a great explanation. I was also using `useEffect` as a subscription artifact. Th only issue here which still confuses me is: What if I ALSO wanted to trigger a first `onChange` when the component loads? Would then, `useEffect` make sense with `[]` dependencies. But then, the lint would complain again. – zanona May 31 '20 at 12:38
  • @zanona would the `useCallback` trick work? Basically, you don't really want to call `onChange`, but you more specifically want to call *the onChange function that was passed during mount*. So you need a way to remember what was this function and call it afterward. I've also tried to add `[!!onChange]` as dependency, but it doesn't seem to work. – Eric Burel Jul 22 '21 at 09:48
  • It seems that `useCallback` will fail as well, but `useRef` is more appropriate: `const initialOnChangeRef = useRef(() => onChange(value)); useEffect(() => { initialOnChangeRef.current(); }, [initialOnChangeRef]);`. – Eric Burel Jul 22 '21 at 09:56
  • That makes more sense than `useEffect`, but the naming of the functions should be improved. Just call the function `handleChange` for example and don't use an underscore in `_setValue`. – Micros Oct 04 '22 at 08:42
  • 1
    Subscription vs side-effects part made this easier to understand. Anyone know what would be the good example of side-effect for useEffect if not getting triggered on some change? – Ivditi Gabeskiria Feb 22 '23 at 21:48
  • @zanona how do we then use subscription in react? – cikatomo Jul 01 '23 at 18:37
  • the eslint rule is actually an anti pattern. useEffect is essentially a subscriber - listening on what you put in that dependency array. What goes inside the callback should not necessariy be what is in the subscriber. – Steve Tomlin Jul 14 '23 at 13:53
84

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.
Freez
  • 7,208
  • 2
  • 19
  • 29
  • 1
    So for instance `const initialOnChange = useCallback(onChange, []); useEffect(() => initialOnChange(value), [initialOnChange])` should work for triggering the function onMount? Edit: no it doesn't :/ you need to pass the onChange function as a dependency of the `useCallback`, so it doesn't work as expected. – Eric Burel Jul 22 '21 at 09:50
  • Yes @EricBurel, it's a very similar behavior with useEffect. If you are calling an external function from inside useCallback, you will need to declare it in the dependencies array. If you implement the onChange directly without calling another function it would work normally. `const onChange = useCallback(() => { // implement here }, []);` – Freez Aug 18 '21 at 04:39
  • I feel like `useEffect` is mixing 2 concepts: dependencies, that defines the effect callback (so the callback change when a dependency change), and triggers, that actually trigger the effect callback. For many use cases, we would probably need a hook with an API like `useEffect(cb, deps, triggers)`. – Eric Burel Aug 19 '21 at 07:28
  • @EricBurel I think the trigger is exactly what useCallback is doing. The purpose of useEffect is to trigger automatically the callback when one of the dependencies change. – Freez Aug 19 '21 at 14:36
  • It seems like if you use "useCallback", then you must still omit the memoized callback from the dependencies. Otherwise it's not useful, because it's stricly equivalent to defining your callback within the effect and moving the "useCallback" dependencies into the "useEffect" dependencies. It will trigger the effect too often. – Eric Burel Aug 23 '21 at 08:08
  • 3
    Why couldn't we just add the function as a dependency without all of the useCallback overhead? – Isaac Pak Oct 14 '21 at 14:59
  • @IsaacPak If you define the function elsewhere you can! - If you define the function on the component, then it's not the "same" function render to render (in the triple equality sense) thus would then tell useEffect to update and cause infinite loop to my understanding. – Julix Nov 16 '21 at 16:07
  • 3
    @IsaacPak completely agree. React is overly complicated in this sense. – Brady Dowling Apr 25 '22 at 17:59
  • "If you can, call the function INSIDE* of your effect, and just use the result as a dependency." Original post says the opposite, linked docs and dan's blogs say the contrary (see above) – kevin May 23 '22 at 03:08
  • There is many times a deb should NOT be on useEffect, since you do not want that specific useEffect to be called with changes to that dependency. That should be the only explanation. Except we have to abide by rules that don't make sense for the specific problem we deal with, which is why programmers constantly argue against using lint rules. – user275564 Aug 13 '22 at 20:29