Merge pull request #6913 from marmelab/fix-usegetmany-state

Update useGetMany loading/loaded state

Author: djhi

packages/ra-core/src/dataProvider/useGetMany.spec.tsx

@@ -5,6 +5,7 @@ import { renderWithRedux } from 'ra-test';
 import useGetMany from './useGetMany';
 import { DataProviderContext } from '../dataProvider';
 import { waitFor } from '@testing-library/react';
+import { useState } from 'react';
 
 const UseGetMany = ({
     resource,
@@ -18,6 +19,26 @@ const UseGetMany = ({
     return <div>hello</div>;
 };
 
+let updateState;
+
+const UseCustomGetMany = ({
+    resource,
+    ids,
+    options = {},
+    callback = null,
+    ...rest
+}) => {
+    const [stateIds, setStateIds] = useState(ids);
+    const hookValue = useGetMany(resource, stateIds, options);
+    if (callback) callback(hookValue);
+
+    updateState = newIds => {
+        setStateIds(newIds);
+    };
+
+    return <div>hello</div>;
+};
+
 describe('useGetMany', () => {
     it('should call the dataProvider with a GET_MANY on mount', async () => {
         const dataProvider = {
@@ -200,6 +221,133 @@ describe('useGetMany', () => {
         });
     });
 
+    it('should update loading state when ids change', async () => {
+        const dataProvider = {
+            getMany: jest.fn((resource, params) => {
+                if (params.ids.length === 1) {
+                    return Promise.resolve({
+                        data: [{ id: 1, title: 'foo' }],
+                    });
+                } else {
+                    return Promise.resolve({
+                        data: [
+                            { id: 1, title: 'foo' },
+                            { id: 2, title: 'bar' },
+                        ],
+                    });
+                }
+            }),
+        };
+
+        const hookValue = jest.fn();
+        renderWithRedux(
+            <DataProviderContext.Provider value={dataProvider}>
+                <UseCustomGetMany
+                    resource="posts"
+                    ids={[1]}
+                    callback={hookValue}
+                />
+            </DataProviderContext.Provider>,
+            {
+                admin: {
+                    resources: {
+                        posts: {
+                            data: {},
+                        },
+                    },
+                },
+            }
+        );
+
+        await waitFor(() => {
+            expect(dataProvider.getMany).toBeCalledTimes(1);
+        });
+
+        expect(hookValue.mock.calls[0][0]).toEqual({
+            data: [undefined],
+            error: null,
+            loaded: false,
+            loading: true,
+            refetch: expect.any(Function),
+        });
+        expect(hookValue.mock.calls[1][0]).toEqual({
+            data: [undefined],
+            error: null,
+            loaded: false,
+            loading: true,
+            refetch: expect.any(Function),
+        });
+        expect(hookValue.mock.calls[2][0]).toEqual({
+            data: [{ id: 1, title: 'foo' }],
+            error: null,
+            loaded: false,
+            loading: true,
+            refetch: expect.any(Function),
+        });
+        expect(hookValue.mock.calls[3][0]).toEqual({
+            data: [{ id: 1, title: 'foo' }],
+            error: null,
+            loaded: true,
+            loading: false,
+            refetch: expect.any(Function),
+        });
+
+        // Updating ids...
+        updateState([1, 2]);
+
+        await waitFor(() => {
+            expect(dataProvider.getMany).toBeCalledTimes(2);
+        });
+
+        expect(hookValue.mock.calls[4][0]).toEqual({
+            data: [{ id: 1, title: 'foo' }],
+            error: null,
+            loaded: true,
+            loading: false,
+            refetch: expect.any(Function),
+        });
+        expect(hookValue.mock.calls[5][0]).toEqual({
+            data: [{ id: 1, title: 'foo' }],
+            error: null,
+            loaded: true,
+            loading: true,
+            refetch: expect.any(Function),
+        });
+        expect(hookValue.mock.calls[6][0]).toEqual({
+            data: [{ id: 1, title: 'foo' }],
+            error: null,
+            loaded: true,
+            loading: true,
+            refetch: expect.any(Function),
+        });
+        expect(hookValue.mock.calls[7][0]).toEqual({
+            data: [
+                { id: 1, title: 'foo' },
+                {
+                    id: 2,
+                    title: 'bar',
+                },
+            ],
+            error: null,
+            loaded: true,
+            loading: true,
+            refetch: expect.any(Function),
+        });
+        expect(hookValue.mock.calls[8][0]).toEqual({
+            data: [
+                { id: 1, title: 'foo' },
+                {
+                    id: 2,
+                    title: 'bar',
+                },
+            ],
+            error: null,
+            loaded: true,
+            loading: false,
+            refetch: expect.any(Function),
+        });
+    });
+
     it('should retrieve results from redux state on mount', () => {
         const hookValue = jest.fn();
         renderWithRedux(

packages/ra-core/src/dataProvider/useGetMany.ts

@@ -1,4 +1,4 @@
-import { useCallback, useMemo } from 'react';
+import { useCallback, useMemo, useEffect } from 'react';
 import ReactDOM from 'react-dom';
 import { useSelector } from 'react-redux';
 import { createSelector } from 'reselect';
@@ -11,7 +11,6 @@ import { CRUD_GET_MANY } from '../actions/dataActions/crudGetMany';
 import { Identifier, Record, ReduxState, DataProviderProxy } from '../types';
 import { useSafeSetState } from '../util/hooks';
 import useDataProvider from './useDataProvider';
-import { useEffect } from 'react';
 import { useVersion } from '../controller';
 import { Refetch } from './useQueryWithStore';
 
@@ -119,10 +118,17 @@ const useGetMany = (
         refetch,
     });
     if (!isEqual(state.data, data)) {
-        setState({
+        const newState = {
             ...state,
-            data,
-        });
+            data: data?.includes(undefined) ? state.data : data,
+            loading:
+                state.data?.length !== 0 &&
+                (state.loading || data?.includes(undefined)),
+        };
+
+        if (!isEqual(state, newState)) {
+            setState(newState);
+        }
     }
     dataProvider = useDataProvider(); // not the best way to pass the dataProvider to a function outside the hook, but I couldn't find a better one
     useEffect(