React useEffect dependency of useCallback always triggers render

Question

I have a mystery. Consider the following custom React hook that fetches data by time period and stores the results in a Map:

export function useDataByPeriod(dateRanges: PeriodFilter[]) {
    const isMounted = useMountedState();

    const [data, setData] = useState(
        new Map(
            dateRanges.map(dateRange => [
                dateRange,
                makeAsyncIsLoading({ isLoading: false }) as AsyncState<MyData[]>
            ])
        )
    );

    const updateData = useCallback(
        (period: PeriodFilter, asyncState: AsyncState<MyData[]>) => {
            const isSafeToSetData = isMounted === undefined || (isMounted !== undefined && isMounted());
            if (isSafeToSetData) {
                setData(new Map(data.set(period, asyncState)));
            }
        },
        [setData, data, isMounted]
    );

    useEffect(() => {
        if (dateRanges.length === 0) {
            return;
        }

        const loadData = () => {
            const client = makeClient();
            dateRanges.map(dateRange => {
                updateData(dateRange, makeAsyncIsLoading({ isLoading: true }));

                return client
                    .getData(dateRange.dateFrom, dateRange.dateTo)
                    .then(periodData => {
                        updateData(dateRange, makeAsyncData(periodData));
                    })
                    .catch(error => {
                        const errorString = `Problem fetching ${dateRange.displayPeriod} (${dateRange.dateFrom} - ${dateRange.dateTo})`;
                        console.error(errorString, error);
                        updateData(dateRange, makeAsyncError(errorString));
                    });
            });
        };

        loadData();
        // eslint-disable-next-line react-hooks/exhaustive-deps
    }, [dateRanges /*, updateData - for some reason when included this triggers infinite renders */]);

    return data;
}

The useEffect is being repeatedly triggered when updateData is added as a dependency. If I exclude it as a dependency then everything works / behaves as expected but eslint complains I'm violating react-hooks/exhaustive-deps.

Given updateData has been useCallback-ed I'm at a loss to understand why it should repeatedly trigger renders. Can anyone shed any light please?

Answer 1

The problem lies in the useCallback/useEffect used in combination. One has to be careful with dependency arrays in both useCallback and useEffect, as the change in  the useCallback dependency array will trigger the useEffect to run. 

The “data” variable is used inside useCallback dependency array, and when the setData is called react will rerun function component with new value for data variable and that triggers a chain of calls. 

Call stack would look something like this:

1. useEffect run
2. updateData called
3. setState called
4. component re-renders with new state data
5. new value for data triggers useCallback
6. updateData changed
7. triggers useEffect again

To solve the problem you would need to remove the “data” variable from the useCallback dependency array. I find it to be a good practice to not include a component state in the dependency arrays whenever possible.

If you need to change component state from the useEffect or useCallback and the new state is a function of the previous state, you can pass the function that receives a current state as parameter and returns a new state.

const updateData = useCallback(
    (period: PeriodFilter, asyncState: AsyncState<MyData[]>) => {
        const isSafeToSetData = isMounted === undefined || (isMounted !== undefined && isMounted());
        if (isSafeToSetData) {
            setData(existingData => new Map(existingData.set(period, asyncState)));
        }
    },
    [setData, isMounted]
);

In your example you need the current state only to calculate next state so that should work.

Answer 2

This is what I now have based on @jure's comment above:

I think the problem is that the "data" variable is included in the dependency array of useCallback. Every time you setData, the data variable is changed that triggers useCallback to provide new updateData and that triggers useEffect. Try to implement updateData without a dependecy on the data variable. you can do something like setData(d=>new Map(d.set(period, asyncState)) to avoid passing "data" variable to useCallback

I adjusted my code in the manners suggested and it worked. Thanks!

export function useDataByPeriod(dateRanges: PeriodFilter[]) {
    const isMounted = useMountedState();

    const [data, setData] = useState(
        new Map(
            dateRanges.map(dateRange => [
                dateRange,
                makeAsyncIsLoading({ isLoading: false }) as AsyncState<MyData[]>
            ])
        )
    );

    const updateData = useCallback(
        (period: PeriodFilter, asyncState: AsyncState<MyData[]>) => {
            const isSafeToSetData = isMounted === undefined || (isMounted !== undefined && isMounted());
            if (isSafeToSetData) {
                setData(existingData => new Map(existingData.set(period, asyncState)));
            }
        },
        [setData, isMounted]
    );

    useEffect(() => {
        if (dateRanges.length === 0) {
            return;
        }

        const loadData = () => {
            const client = makeClient();
            dateRanges.map(dateRange => {
                updateData(dateRange, makeAsyncIsLoading({ isLoading: true }));

                return client
                    .getData(dateRange.dateFrom, dateRange.dateTo)
                    .then(traffic => {
                        updateData(dateRange, makeAsyncData(traffic));
                    })
                    .catch(error => {
                        const errorString = `Problem fetching ${dateRange.displayPeriod} (${dateRange.dateFrom} - ${dateRange.dateTo})`;
                        console.error(errorString, error);
                        updateData(dateRange, makeAsyncError(errorString));
                    });
            });
        };

        loadData();
    }, [dateRanges , updateData]);

    return data;
}