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 perf improvements #3572

Merged
merged 9 commits into from
Aug 22, 2019
2 changes: 1 addition & 1 deletion examples/simple/src/posts/PostList.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ const PostList = props => {
}
/>
) : (
<Datagrid rowClick={rowClick} expand={<PostPanel />}>
<Datagrid rowClick={rowClick} expand={PostPanel}>
<TextField source="id" />
<TextField source="title" cellClassName={classes.title} />
<DateField
Expand Down
54 changes: 31 additions & 23 deletions packages/ra-core/src/dataProvider/useGetMany.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { useMemo } from 'react';
import ReactDOM from 'react-dom';
import { useSelector } from 'react-redux';
import { createSelector } from 'reselect';
import debounce from 'lodash/debounce';
Expand Down Expand Up @@ -81,8 +82,10 @@ const useGetMany = (resource: string, ids: Identifier[], options: any = {}) => {
const [state, setState] = useSafeSetState({
data,
error: null,
loading: true,
loaded: data.length !== 0 && !data.includes(undefined),
loading: ids.length !== 0,
loaded:
ids.length === 0 ||
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

avoids showing the loader if there is nothing to load

(data.length !== 0 && !data.includes(undefined)),
});
if (!isEqual(state.data, data)) {
setState({
Expand Down Expand Up @@ -173,27 +176,32 @@ const callQueries = debounce(() => {
{ ids: accumulatedIds },
{ action: CRUD_GET_MANY }
)
.then(response => {
queries.forEach(({ ids, setState, onSuccess }) => {
setState(prevState => ({
...prevState,
loading: false,
loaded: true,
}));
if (onSuccess) {
const subData = ids.map(id =>
response.data.find(datum => datum.id == id)
);
onSuccess({ data: subData });
}
});
})
.catch(error => {
queries.forEach(({ setState, onFailure }) => {
setState({ error, loading: false, loaded: false });
onFailure && onFailure(error);
});
});
.then(response =>
// Forces batching, see https://stackoverflow.com/questions/48563650/does-react-keep-the-order-for-state-updates/48610973#48610973
ReactDOM.unstable_batchedUpdates(() =>
queries.forEach(({ ids, setState, onSuccess }) => {
setState(prevState => ({
...prevState,
loading: false,
loaded: true,
}));
if (onSuccess) {
const subData = ids.map(id =>
response.data.find(datum => datum.id == id)
);
onSuccess({ data: subData });
}
})
)
)
.catch(error =>
ReactDOM.unstable_batchedUpdates(() =>
queries.forEach(({ setState, onFailure }) => {
setState({ error, loading: false, loaded: false });
onFailure && onFailure(error);
})
)
);
delete queriesToCall[resource];
});
});
Expand Down
8 changes: 7 additions & 1 deletion packages/ra-core/src/reducer/admin/resource/data.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Reducer } from 'redux';
import isEqual from 'lodash/isEqual';
import { FETCH_END } from '../../../actions/fetchActions';
import {
CREATE,
Expand Down Expand Up @@ -75,7 +76,12 @@ export const addRecords = (

const records = { fetchedAt: newFetchedAt };
Object.keys(newFetchedAt).forEach(
id => (records[id] = newRecordsById[id] || oldRecords[id])
id =>
(records[id] = newRecordsById[id]
? isEqual(newRecordsById[id], oldRecords[id])
? oldRecords[id] // do not change the record to avoid a redraw
: newRecordsById[id]
: oldRecords[id])
);

return hideFetchedAt(records);
Expand Down
44 changes: 9 additions & 35 deletions packages/ra-ui-materialui/src/layout/Menu.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
import React from 'react';
import PropTypes from 'prop-types';
import { connect } from 'react-redux';
import { shallowEqual, useSelector } from 'react-redux';
import inflection from 'inflection';
import compose from 'recompose/compose';
import { withStyles, createStyles, useMediaQuery } from '@material-ui/core';
import { makeStyles, useMediaQuery } from '@material-ui/core';
import DefaultIcon from '@material-ui/icons/ViewList';
import classnames from 'classnames';
import { getResources, useTranslate } from 'ra-core';

import DashboardMenuItem from './DashboardMenuItem';
import MenuItemLink from './MenuItemLink';

const styles = createStyles({
const useStyles = makeStyles({
main: {
display: 'flex',
flexDirection: 'column',
Expand All @@ -32,19 +31,20 @@ const translatedResourceName = (resource, translate) =>
});

const Menu = ({
classes,
classes: classesOverride,
className,
dense,
hasDashboard,
onMenuClick,
open,
pathname,
resources,
logout,
...rest
}) => {
const translate = useTranslate();
const classes = useStyles({ classes: classesOverride });
const isXSmall = useMediaQuery(theme => theme.breakpoints.down('xs'));
const open = useSelector(state => state.admin.ui.sidebarOpen);
const resources = useSelector(getResources, shallowEqual);
useSelector(state => state.router.location.pathname); // used to force redraw on navigation

return (
<div className={classnames(classes.main, className)} {...rest}>
Expand Down Expand Up @@ -81,36 +81,10 @@ Menu.propTypes = {
hasDashboard: PropTypes.bool,
logout: PropTypes.element,
onMenuClick: PropTypes.func,
open: PropTypes.bool,
pathname: PropTypes.string,
resources: PropTypes.array.isRequired,
};

Menu.defaultProps = {
onMenuClick: () => null,
};

const mapStateToProps = state => ({
open: state.admin.ui.sidebarOpen,
resources: getResources(state),
pathname: state.router.location.pathname, // used to force redraw on navigation
});

const enhance = compose(
connect(
mapStateToProps,
{}, // Avoid connect passing dispatch in props,
null,
{
areStatePropsEqual: (prev, next) =>
prev.resources.every(
(value, index) => value === next.resources[index] // shallow compare resources
) &&
prev.pathname === next.pathname &&
prev.open === next.open,
}
),
withStyles(styles)
);

export default enhance(Menu);
export default Menu;
9 changes: 3 additions & 6 deletions packages/ra-ui-materialui/src/list/Datagrid.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,11 @@ const styles = theme =>
padding: 0,
width: theme.spacing(6),
},
expandButton: {
padding: theme.spacing(1),
},
expandIconCell: {
width: theme.spacing(6),
},
expandIcon: {
padding: theme.spacing(1),
transform: 'rotate(-90deg)',
transition: theme.transitions.create('transform', {
duration: theme.transitions.duration.shortest,
Expand Down Expand Up @@ -173,7 +171,7 @@ class Datagrid extends Component {
* displaying the table header with zero data rows,
* the datagrid displays nothing in this case.
*/
if (!isLoading && (ids.length === 0 || total === 0)) {
if (loaded && (ids.length === 0 || total === 0)) {
return null;
}

Expand Down Expand Up @@ -245,7 +243,6 @@ class Datagrid extends Component {
hasBulkActions,
hover,
ids,
isLoading,
onToggleItem,
resource,
rowStyle,
Expand All @@ -270,7 +267,7 @@ Datagrid.propTypes = {
order: PropTypes.string,
}),
data: PropTypes.object.isRequired,
expand: PropTypes.element,
expand: PropTypes.oneOfType([PropTypes.element, PropTypes.elementType]),
hasBulkActions: PropTypes.bool.isRequired,
hover: PropTypes.bool,
ids: PropTypes.arrayOf(PropTypes.any).isRequired,
Expand Down
6 changes: 2 additions & 4 deletions packages/ra-ui-materialui/src/list/DatagridBody.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ const DatagridBody = ({
hasBulkActions,
hover,
ids,
isLoading,
onToggleItem,
resource,
row,
Expand Down Expand Up @@ -62,7 +61,7 @@ const DatagridBody = ({
)}
</TableBody>
),
[version, isLoading, data, selectedIds, JSON.stringify(ids)] // eslint-disable-line
[version, data, selectedIds, JSON.stringify(ids)] // eslint-disable-line
);

DatagridBody.propTypes = {
Expand All @@ -71,11 +70,10 @@ DatagridBody.propTypes = {
className: PropTypes.string,
children: PropTypes.node,
data: PropTypes.object.isRequired,
expand: PropTypes.element,
expand: PropTypes.oneOfType([PropTypes.element, PropTypes.elementType]),
hasBulkActions: PropTypes.bool.isRequired,
hover: PropTypes.bool,
ids: PropTypes.arrayOf(PropTypes.any).isRequired,
isLoading: PropTypes.bool,
onToggleItem: PropTypes.func,
resource: PropTypes.string,
row: PropTypes.element,
Expand Down
6 changes: 4 additions & 2 deletions packages/ra-ui-materialui/src/list/DatagridLoading.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from 'react';
import React, { memo } from 'react';
import Table from '@material-ui/core/Table';
import TableCell from '@material-ui/core/TableCell';
import TableHead from '@material-ui/core/TableHead';
Expand All @@ -25,7 +25,7 @@ const Placeholder = ({ classes: classesOverride }) => {
const times = (nbChildren, fn) =>
Array.from({ length: nbChildren }, (_, key) => fn(key));

export default ({
const DatagridLoading = ({
classes,
className,
expand,
Expand Down Expand Up @@ -113,3 +113,5 @@ export default ({
</TableBody>
</Table>
);

export default memo(DatagridLoading);
Loading