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?

Gent

Did you try removing setData from the dependency array in the useCallback? – Becki

Given setData is a dependency isn't that just creating the react-hooks/exhaustive-deps issue in a different place? – Gent

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 – Borg

Also are you sure that useMountedState returns same ref ? Besides that I think @jure is right – Gonocyte

@jure you were exactly right! Thanks! Do you want to post this as an answer and I'll accept it? No sweat if you can't be bothered but I want to give credit where it's due. I'll write up what I have now - but please do leave your answer and I'll accept that instead. Thanks again! – Gent

@JohnReilly glad it worked. I tried to compose an answer. not so easy to explain the behavior with words :) guess this is a bit confusing part of react hooks. – Borg

You did an excellent job of explaining it! – Gent

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.

Borg

This should be upvoted more! I hadn't previously thought of solving it be using dependency injection (passing in the old data as an argument). Thanks!!! – Dihydric

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;
}

Gent