Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFR] List performance optimizations #3320

Merged
merged 12 commits into from
Jun 8, 2019
3 changes: 3 additions & 0 deletions examples/demo/src/reviews/ReviewList.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ const styles = theme =>
listWithDrawer: {
marginRight: 400,
},
drawerPaper: {
zIndex: 100,
},
});

class ReviewList extends Component {
Expand Down
24 changes: 15 additions & 9 deletions packages/ra-core/src/controller/ListController.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -65,6 +65,10 @@ interface Props {
[key: string]: any;
}

const defaultSort = {
field: 'id',
order: SORT_ASC,
};
/**
* List page component
*
Expand Down Expand Up @@ -125,10 +129,7 @@ const ListController = (props: Props) => {
hasCreate,
location,
filterDefaultValues,
sort = {
field: 'id',
order: SORT_ASC,
},
sort = defaultSort,
perPage = 10,
filter,
debounce = 500,
Expand Down Expand Up @@ -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)),
Expand All @@ -182,10 +191,7 @@ const ListController = (props: Props) => {

return children({
basePath,
currentSort: {
field: query.sort,
order: query.order,
},
currentSort,
data,
defaultTitle,
displayedFilters: query.displayedFilters,
Expand Down
37 changes: 26 additions & 11 deletions packages/ra-core/src/controller/useListParams.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -100,10 +107,7 @@ const useListParams = ({
resource,
location,
filterDefaultValues,
sort = {
field: 'id',
order: SORT_ASC,
},
sort = defaultSort,
perPage = 10,
debounce = 500,
}: ListParamsOptions): [Parameters, Modifiers] => {
Expand All @@ -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);
Expand Down Expand Up @@ -153,7 +168,7 @@ const useListParams = ({
requestSignature
);

const filterValues = query.filter || {};
const filterValues = query.filter || emptyObject;

const setFilters = useCallback(
lodashDebounce(filters => {
Expand Down
2 changes: 2 additions & 0 deletions packages/ra-core/src/util/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -24,4 +25,5 @@ export {
TestContext,
renderWithRedux,
warning,
useWhyDidYouUpdate,
};
46 changes: 46 additions & 0 deletions packages/ra-core/src/util/useWhyDidYouUpdate.ts
Original file line number Diff line number Diff line change
@@ -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;
});
}
22 changes: 11 additions & 11 deletions packages/ra-ui-materialui/src/layout/Responsive.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
import React from 'react';
import PropTypes from 'prop-types';
import withWidth from '@material-ui/core/withWidth';
import { useTheme } from '@material-ui/styles';
import useMediaQuery from '@material-ui/core/useMediaQuery';

export const Responsive = ({
xsmall,
small,
medium,
large,
width,
...rest
}) => {
export const Responsive = ({ xsmall, small, medium, large, ...rest }) => {
const theme = useTheme();
const width =
[...theme.breakpoints.keys].reverse().reduce((output, key) => {
// eslint-disable-next-line react-hooks/rules-of-hooks
const matches = useMediaQuery(theme.breakpoints.only(key));
return !output && matches ? key : output;
}, null) || 'xs';
let element;
switch (width) {
case 'xs':
Expand Down Expand Up @@ -59,7 +60,6 @@ Responsive.propTypes = {
small: PropTypes.element,
medium: PropTypes.element,
large: PropTypes.element,
width: PropTypes.string,
};

export default withWidth()(Responsive);
export default Responsive;
87 changes: 42 additions & 45 deletions packages/ra-ui-materialui/src/list/DatagridBody.js
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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, JSON.stringify(ids)]
);

DatagridBody.propTypes = {
basePath: PropTypes.string,
Expand Down Expand Up @@ -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;
Loading