CODEWITHSUNDEEP
reactjsreact-hooks
Nissan Murano
Nissan Murano

Reputation: 71

Does the dependency array in React hooks really need to be exhaustive?

According to the React docs:

every value referenced inside the effect function should also appear in the dependencies array

If my effect function references a few variables from the outer scope but I only want it to be executed when one of them changes, why do I need to specify all the other variables in the dependency array? Yes, the closure will become stale if the other variables change but I don't care because I don't need the function to be called yet. When the variable that I care about changes then the new closure with the values at that moment can be called. What am I missing?

Here's a working example (to my knowledge) where the useEffect dependency arrays are not exhaustive:

import React, { useEffect, useState } from "react";

const allCars = {
    toyota: ["camry", "corolla", "mirai"],
    ford: ["mustang", "cortina", "model T"],
    nissan: ["murano", "micra", "maxima"],
};

function CarList() {
    const [cars, setCars] = useState([]);
    const [brand, setBrand] = useState("toyota");
    const [filterKey, setFilterKey] = useState("");

    useEffect(() => {
        // I don't want to run this effect when filterKey changes because I wanna wrap that case in a timeout to throttle it.
        setCars(allCars[brand].filter(model => model.startsWith(filterKey)));
    }, [brand]);

    useEffect(() => {
        // This effect is only called when filterKey changes but still picks up the current value of 'brand' at the time the function is called.
        const timeoutId = setTimeout(() => {
            setCars(allCars[brand].filter(model => model.startsWith(filterKey)));
        }, 500);
        return () => clearTimeout(timeoutId);
    }, [filterKey]);

    const handleChangeFilterKey = event => {
        setFilterKey(event.target.value);
    };

    return (
        <div>
            {`${brand} cars`}
            <div>Select brand</div>
            <input type="radio" value="toyota" checked={brand === "toyota"} onChange={() => setBrand("toyota")} />
            <input type="radio" value="ford" checked={brand === "ford"} onChange={() => setBrand("ford")} />
            <input type="radio" value="nissan" checked={brand === "nissan"} onChange={() => setBrand("nissan")} />
            <div>Filter</div>
            <input label="search" value={filterKey} onChange={handleChangeFilterKey} />
            <ul>
                {cars.map(car => (
                    <li>{car}</li>
                ))}
            </ul>
        </div>
    );
}

Are there any pitfalls to the above example?

Upvotes: 7

Views: 4151

Answers (2)

Nikita Balikhin
Nikita Balikhin

Reputation: 16

In your case there is no side-effect, it's just data derived from state, so you should use different tools instead of useEffect.

First filter could be implemented with just getting value by key:

const carsByBrand = allCars[brand];

Complexity of this operation is constant, so memoization will be too expensive.

And second filter is debounced, but you might want to memoize it because of filtering with linear complexity. For these purposes you can use for example useDebouncedMemo hook:

const filteredCars = useDebouncedMemo(() => {
   return carsByBrand.filter(model => model.startsWith(filterKey)
}, [filterKey, carsByBrand], 500)

In this case dependencies of hook are exhaustive and logic is much more declarative.

Upvotes: 0

nolan
nolan

Reputation: 467

Yes, you should follow this rule all the time, when you find your code break with following it, that means good practice is not followed. That is the meaning of this rule, make sure you design code well.

I imagine in your case the code looks like this:

  const Test = () => {
   const [wantToSync] = useState(0)
   const [notWantToSync] = useState(0) // notWantToSync also might come from props, i'll use state as example here
   useEffect(() => {
    fetch('google.com', {
      body: JSON.stringify({wantToSync, notWantToSync})
    }).then(result => {
      // ...
    })
   }, [wantToSync]) // component is supposed to be reactive to notWantToSync, missing notWantToSync in dep is dangerous
 }

If notWantToSync was defined as a state of the component, the component is supposed to be reactive to it, including useEffect. If that is not what you want, notWantToSync shouldn't be state from start.

const Test = () => {
  const [wantToSync] = useState(0)
  const notWantToSyncRef = useRef(0) // hey I don't want notWantToSync to be reactive, so i put it in useRef
  useEffect(() => {
    fetch('google.com', {
      body: JSON.stringify({wantToSync, notWantToSync: notWantToSyncRef.current})
    }).then(result => {
      // ...
    })
  }, [wantToSync, notWantToSyncRef]) // yeah, now eslint doesn't bother me, and notWantToSync doesn't trigger useEffect anymore
}

Normally you don't need to do if else in useEffect to stop re-rendering, there're other approaches like useMemo or useCallback to do similar things in addition to useRef although they have different using context.

I see your struggle in the new example, so you want to do a throttle, and if filterKey is added to the first useEffect dep, the throttle will be broken. My opinion is that when you find yourself in a situation like this, that often means there's better practice(eslint exhaustive help to identify it), for example, extract throttle logic to a hook: https://github.com/streamich/react-use/blob/master/src/useThrottle.ts.

Exhaustive deps is not something vital to follow, it's just a good practice to make sure your code is designed well. The above example proves it that

Upvotes: 3

Related Questions