Merge pull request #5577 from marmelab/Fix-logout-causes-error-in-use…

…GetList Fix logout causes error in useGetList
marmelab · Nov 30, 2020 · e2c4935 · e2c4935
2 parents 8646bab + d05c466
commit e2c4935
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 40 deletions.
diff --git a/packages/ra-core/src/dataProvider/useGetList.ts b/packages/ra-core/src/dataProvider/useGetList.ts
@@ -1,4 +1,4 @@
-import { useSelector, shallowEqual } from 'react-redux';
+import { useMemo } from 'react';
 import get from 'lodash/get';
 
 import {
@@ -11,6 +11,7 @@ import {
 } from '../types';
 import useQueryWithStore from './useQueryWithStore';
 
+const defaultIds = [];
 const defaultData = {};
 
 /**
@@ -67,16 +68,28 @@ const useGetList = <RecordType extends Record = Record>(
 } => {
     const requestSignature = JSON.stringify({ pagination, sort, filter });
 
-    const { data: ids, total, error, loading, loaded } = useQueryWithStore(
+    const {
+        data: { ids, allRecords },
+        total,
+        error,
+        loading,
+        loaded,
+    } = useQueryWithStore(
         { type: 'getList', resource, payload: { pagination, sort, filter } },
         options,
-        // data selector (may return [])
-        (state: ReduxState): Identifier[] =>
-            get(
+        // ids and data selector
+        (state: ReduxState): DataSelectorResult<RecordType> => ({
+            ids: get(
                 state.admin.resources,
                 [resource, 'list', 'cachedRequests', requestSignature, 'ids'],
-                []
+                null
             ),
+            allRecords: get(
+                state.admin.resources,
+                [resource, 'data'],
+                defaultData
+            ),
+        }),
         // total selector (may return undefined)
         (state: ReduxState): number =>
             get(state.admin.resources, [
@@ -85,26 +98,37 @@ const useGetList = <RecordType extends Record = Record>(
                 'cachedRequests',
                 requestSignature,
                 'total',
-            ])
+            ]),
+        (data: DataSelectorResult<RecordType>) => data.ids !== null
     );
 
-    const data = useSelector((state: ReduxState): RecordMap<RecordType> => {
-        if (!ids) return defaultData;
-        const allResourceData = get(
-            state.admin.resources,
-            [resource, 'data'],
-            defaultData
-        );
-        return ids
-            .map(id => allResourceData[id])
-            .reduce((acc, record) => {
-                if (!record) return acc;
-                acc[record.id] = record;
-                return acc;
-            }, {});
-    }, shallowEqual);
+    const data = useMemo(
+        () =>
+            ids === null
+                ? defaultData
+                : ids
+                      .map(id => allRecords[id])
+                      .reduce((acc, record) => {
+                          if (!record) return acc;
+                          acc[record.id] = record;
+                          return acc;
+                      }, {}),
+        [ids, allRecords]
+    );
 
-    return { data, ids, total, error, loading, loaded };
+    return {
+        data,
+        ids: ids === null ? defaultIds : ids,
+        total,
+        error,
+        loading,
+        loaded,
+    };
 };
 
+interface DataSelectorResult<RecordType extends Record = Record> {
+    ids: Identifier[];
+    allRecords: RecordMap<RecordType>;
+}
+
 export default useGetList;
diff --git a/packages/ra-core/src/dataProvider/useQueryWithStore.ts b/packages/ra-core/src/dataProvider/useQueryWithStore.ts
@@ -37,19 +37,6 @@ export type PartialQueryState = {
 
 const queriesThisTick: { [key: string]: Promise<PartialQueryState> } = {};
 
-/**
- * Lists of records are initialized to a particular object,
- * so detecting if the list is empty requires some work.
- *
- * @see src/reducer/admin/data.ts
- */
-const isEmptyList = data =>
-    Array.isArray(data)
-        ? data.length === 0
-        : data &&
-          Object.keys(data).length === 0 &&
-          data.hasOwnProperty('fetchedAt');
-
 /**
  * Default cache selector. Allows to cache responses by default.
  *
@@ -72,6 +59,8 @@ const defaultTotalSelector = query => (state: ReduxState) => {
         : null;
 };
 
+const defaultIsDataLoaded = (data: any): boolean => data !== undefined;
+
 /**
  * Fetch the data provider through Redux, return the value from the store.
  *
@@ -120,7 +109,8 @@ const useQueryWithStore = <State extends ReduxState = ReduxState>(
     query: Query,
     options: QueryOptions = { action: 'CUSTOM_QUERY' },
     dataSelector: (state: State) => any = defaultDataSelector(query),
-    totalSelector: (state: State) => number = defaultTotalSelector(query)
+    totalSelector: (state: State) => number = defaultTotalSelector(query),
+    isDataLoaded: (data: any) => boolean = defaultIsDataLoaded
 ): {
     data?: any;
     total?: number;
@@ -142,7 +132,7 @@ const useQueryWithStore = <State extends ReduxState = ReduxState>(
         total,
         error: null,
         loading: true,
-        loaded: data !== undefined && !isEmptyList(data),
+        loaded: isDataLoaded(data),
     });
 
     useEffect(() => {
@@ -154,7 +144,7 @@ const useQueryWithStore = <State extends ReduxState = ReduxState>(
                 total,
                 error: null,
                 loading: true,
-                loaded: data !== undefined && !isEmptyList(data),
+                loaded: isDataLoaded(data),
             });
         } else if (!isEqual(state.data, data) || state.total !== total) {
             // the dataProvider response arrived in the Redux store
@@ -171,7 +161,15 @@ const useQueryWithStore = <State extends ReduxState = ReduxState>(
                 }));
             }
         }
-    }, [data, requestSignature, setState, state.data, state.total, total]);
+    }, [
+        data,
+        requestSignature,
+        setState,
+        state.data,
+        state.total,
+        total,
+        isDataLoaded,
+    ]);
 
     const dataProvider = useDataProvider();
     useEffect(() => {