Merge pull request #3320 from marmelab/pref-optimization-3-0

[RFR] List performance optimizations
marmelab · Jun 8, 2019 · 3c2e7e1 · 3c2e7e1
2 parents bddfce6 + f672d37
commit 3c2e7e1
Show file tree

Hide file tree

Showing 10 changed files with 185 additions and 113 deletions.
diff --git a/examples/demo/src/reviews/ReviewList.js b/examples/demo/src/reviews/ReviewList.js
@@ -37,6 +37,9 @@ const styles = theme =>
         listWithDrawer: {
             marginRight: 400,
         },
+        drawerPaper: {
+            zIndex: 100,
+        },
     });
 
 class ReviewList extends Component {

diff --git a/packages/ra-core/src/controller/ListController.tsx b/packages/ra-core/src/controller/ListController.tsx
@@ -1,4 +1,4 @@
-import { isValidElement, ReactNode, ReactElement } from 'react';
+import { isValidElement, ReactNode, ReactElement, useMemo } from 'react';
 import inflection from 'inflection';
 
 import { SORT_ASC } from '../reducer/admin/resource/list/queryReducer';
@@ -65,6 +65,10 @@ interface Props {
     [key: string]: any;
 }
 
+const defaultSort = {
+    field: 'id',
+    order: SORT_ASC,
+};
 /**
  * List page component
  *
@@ -125,10 +129,7 @@ const ListController = (props: Props) => {
         hasCreate,
         location,
         filterDefaultValues,
-        sort = {
-            field: 'id',
-            order: SORT_ASC,
-        },
+        sort = defaultSort,
         perPage = 10,
         filter,
         debounce = 500,
@@ -172,6 +173,14 @@ const ListController = (props: Props) => {
         queryModifiers.setPage(query.page - 1);
     }
 
+    const currentSort = useMemo(
+        () => ({
+            field: query.sort,
+            order: query.order,
+        }),
+        [query.sort, query.order]
+    );
+
     const resourceName = translate(`resources.${resource}.name`, {
         smart_count: 2,
         _: inflection.humanize(inflection.pluralize(resource)),
@@ -182,10 +191,7 @@ const ListController = (props: Props) => {
 
     return children({
         basePath,
-        currentSort: {
-            field: query.sort,
-            order: query.order,
-        },
+        currentSort,
         data,
         defaultTitle,
         displayedFilters: query.displayedFilters,

diff --git a/packages/ra-core/src/controller/useListParams.ts b/packages/ra-core/src/controller/useListParams.ts
@@ -1,4 +1,4 @@
-import { useCallback, useState } from 'react';
+import { useCallback, useState, useMemo } from 'react';
 // @ts-ignore
 import { useSelector, useDispatch } from 'react-redux';
 import { parse, stringify } from 'query-string';
@@ -47,6 +47,13 @@ interface Modifiers {
     showFilter: (filterName: string, defaultValue: any) => void;
 }
 
+const emptyObject = {};
+
+const defaultSort = {
+    field: 'id',
+    order: SORT_ASC,
+};
+
 /**
  * Get the list parameters (page, sort, filters) and modifiers.
  *
@@ -100,10 +107,7 @@ const useListParams = ({
     resource,
     location,
     filterDefaultValues,
-    sort = {
-        field: 'id',
-        order: SORT_ASC,
-    },
+    sort = defaultSort,
     perPage = 10,
     debounce = 500,
 }: ListParamsOptions): [Parameters, Modifiers] => {
@@ -115,15 +119,26 @@ const useListParams = ({
         [resource]
     );
 
-    const query = getQuery({
-        location,
+    const requestSignature = [
+        location.search,
+        resource,
         params,
         filterDefaultValues,
-        sort,
+        JSON.stringify(sort),
         perPage,
-    });
+    ];
 
-    const requestSignature = [resource, JSON.stringify(query)];
+    const query = useMemo(
+        () =>
+            getQuery({
+                location,
+                params,
+                filterDefaultValues,
+                sort,
+                perPage,
+            }),
+        requestSignature
+    );
 
     const changeParams = useCallback(action => {
         const newParams = queryReducer(query, action);
@@ -153,7 +168,7 @@ const useListParams = ({
         requestSignature
     );
 
-    const filterValues = query.filter || {};
+    const filterValues = query.filter || emptyObject;
 
     const setFilters = useCallback(
         lodashDebounce(filters => {

diff --git a/packages/ra-core/src/fetch/useGetList.ts b/packages/ra-core/src/fetch/useGetList.ts
@@ -50,22 +50,22 @@ const useGetList = (
     filter: object,
     options?: any
 ) => {
-    const { data, total, error, loading, loaded } = useQueryWithStore(
+    const { data: ids, total, error, loading, loaded } = useQueryWithStore(
         { type: GET_LIST, resource, payload: { pagination, sort, filter } },
         { ...options, action: CRUD_GET_LIST },
         (state: ReduxState) =>
             state.admin.resources[resource]
-                ? state.admin.resources[resource].data
+                ? state.admin.resources[resource].list.ids
                 : null,
         (state: ReduxState) =>
             state.admin.resources[resource]
                 ? state.admin.resources[resource].list.total
                 : null
     );
-    const ids = useSelector(
+    const data = useSelector(
         (state: ReduxState) =>
             state.admin.resources[resource]
-                ? state.admin.resources[resource].list.ids
+                ? state.admin.resources[resource].data
                 : null,
         [resource]
     );

diff --git a/packages/ra-core/src/fetch/useQueryWithStore.ts b/packages/ra-core/src/fetch/useQueryWithStore.ts
@@ -25,7 +25,11 @@ export interface QueryOptions {
  * @see src/reducer/admin/data.ts
  */
 const isEmptyList = data =>
-    data && Object.keys(data).length === 0 && data.hasOwnProperty('fetchedAt');
+    Array.isArray(data)
+        ? data.length === 0
+        : data &&
+          Object.keys(data).length === 0 &&
+          data.hasOwnProperty('fetchedAt');
 
 /**
  * Default cache selector. Allows to cache responses by default.

diff --git a/packages/ra-core/src/util/index.ts b/packages/ra-core/src/util/index.ts
@@ -10,6 +10,7 @@ import resolveRedirectTo from './resolveRedirectTo';
 import TestContext from './TestContext';
 import renderWithRedux from './renderWithRedux';
 import warning from './warning';
+import useWhyDidYouUpdate from './useWhyDidYouUpdate';
 
 export {
     downloadCSV,
@@ -24,4 +25,5 @@ export {
     TestContext,
     renderWithRedux,
     warning,
+    useWhyDidYouUpdate,
 };
diff --git a/packages/ra-core/src/util/useWhyDidYouUpdate.ts b/packages/ra-core/src/util/useWhyDidYouUpdate.ts
@@ -0,0 +1,46 @@
+import { useRef, useEffect } from 'react';
+
+/**
+ * Debug hook showing which props updated between two renders
+ * @example
+ *
+ * const MyComponent = React.memo(props => {
+ *   useWhyDidYouUpdate('MyComponent', props);
+ *   return <div...;
+ * });
+ *
+ * @link https://usehooks.com/useWhyDidYouUpdate/
+ */
+export default function useWhyDidYouUpdate(name, props) {
+    // Get a mutable ref object where we can store props ...
+    // ... for comparison next time this hook runs.
+    const previousProps = useRef() as any;
+
+    useEffect(() => {
+        if (previousProps.current) {
+            // Get all keys from previous and current props
+            const allKeys = Object.keys({ ...previousProps.current, ...props });
+            // Use this object to keep track of changed props
+            const changesObj = {};
+            // Iterate through keys
+            allKeys.forEach(key => {
+                // If previous is different from current
+                if (previousProps.current[key] !== props[key]) {
+                    // Add to changesObj
+                    changesObj[key] = {
+                        from: previousProps.current[key],
+                        to: props[key],
+                    };
+                }
+            });
+
+            // If changesObj not empty then output to console
+            if (Object.keys(changesObj).length) {
+                console.log('[why-did-you-update]', name, changesObj);
+            }
+        }
+
+        // Finally update previousProps with current props for next hook call
+        previousProps.current = props;
+    });
+}
diff --git a/packages/ra-ui-materialui/src/list/DatagridBody.js b/packages/ra-ui-materialui/src/list/DatagridBody.js
@@ -1,6 +1,5 @@
-import React, { cloneElement } from 'react';
+import React, { cloneElement, useMemo } from 'react';
 import PropTypes from 'prop-types';
-import shouldUpdate from 'recompose/shouldUpdate';
 import TableBody from '@material-ui/core/TableBody';
 import classnames from 'classnames';
 
@@ -26,36 +25,45 @@ const DatagridBody = ({
     styles,
     version,
     ...rest
-}) => (
-    <TableBody className={classnames('datagrid-body', className)} {...rest}>
-        {ids.map((id, rowIndex) =>
-            cloneElement(
-                row,
-                {
-                    basePath,
-                    classes,
-                    className: classnames(classes.row, {
-                        [classes.rowEven]: rowIndex % 2 === 0,
-                        [classes.rowOdd]: rowIndex % 2 !== 0,
-                        [classes.clickableRow]: rowClick,
-                    }),
-                    expand,
-                    hasBulkActions,
-                    hover,
-                    id,
-                    key: id,
-                    onToggleItem,
-                    record: data[id],
-                    resource,
-                    rowClick,
-                    selected: selectedIds.includes(id),
-                    style: rowStyle ? rowStyle(data[id], rowIndex) : null,
-                },
-                children
-            )
-        )}
-    </TableBody>
-);
+}) =>
+    useMemo(
+        () => (
+            <TableBody
+                className={classnames('datagrid-body', className)}
+                {...rest}
+            >
+                {ids.map((id, rowIndex) =>
+                    cloneElement(
+                        row,
+                        {
+                            basePath,
+                            classes,
+                            className: classnames(classes.row, {
+                                [classes.rowEven]: rowIndex % 2 === 0,
+                                [classes.rowOdd]: rowIndex % 2 !== 0,
+                                [classes.clickableRow]: rowClick,
+                            }),
+                            expand,
+                            hasBulkActions,
+                            hover,
+                            id,
+                            key: id,
+                            onToggleItem,
+                            record: data[id],
+                            resource,
+                            rowClick,
+                            selected: selectedIds.includes(id),
+                            style: rowStyle
+                                ? rowStyle(data[id], rowIndex)
+                                : null,
+                        },
+                        children
+                    )
+                )}
+            </TableBody>
+        ),
+        [version, isLoading, data, selectedIds, JSON.stringify(ids)]
+    );
 
 DatagridBody.propTypes = {
     basePath: PropTypes.string,
@@ -73,7 +81,7 @@ DatagridBody.propTypes = {
     row: PropTypes.element,
     rowClick: PropTypes.oneOfType([PropTypes.string, PropTypes.func]),
     rowStyle: PropTypes.func,
-    selectedIds: PropTypes.arrayOf(PropTypes.any).isRequired,
+    selectedIds: PropTypes.arrayOf(PropTypes.any),
     styles: PropTypes.object,
     version: PropTypes.number,
 };
@@ -85,19 +93,8 @@ DatagridBody.defaultProps = {
     row: <DatagridRow />,
 };
 
-const areArraysEqual = (arr1, arr2) =>
-    arr1.length == arr2.length && arr1.every((v, i) => v === arr2[i]);
-
-const PureDatagridBody = shouldUpdate(
-    (props, nextProps) =>
-        props.version !== nextProps.version ||
-        nextProps.isLoading === false ||
-        !areArraysEqual(props.ids, nextProps.ids) ||
-        props.data !== nextProps.data
-)(DatagridBody);
-
 // trick material-ui Table into thinking this is one of the child type it supports
 // @ts-ignore
-PureDatagridBody.muiName = 'TableBody';
+DatagridBody.muiName = 'TableBody';
 
-export default PureDatagridBody;
+export default DatagridBody;