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.

Sign up for GitHub

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

[DataGridPro] Fetch new rows only once when multiple models are changed in one cycle #16101

Merged
merged 23 commits into from
Jan 29, 2025
Merged

[DataGridPro] Fetch new rows only once when multiple models are changed in one cycle #16101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
1d0f269
Debounced rows fetch
arminmeh Jan 8, 2025
f989245
Fix test. Fix debounce in the lazy loader
arminmeh Jan 8, 2025
f08bd29
Prevent certain event while rows are stale
arminmeh Jan 9, 2025
9477136
Cleanup
arminmeh Jan 9, 2025
29b479c
Performance improvement
arminmeh Jan 9, 2025
76f76ae
Fix tests
arminmeh Jan 9, 2025
1a66aae
Use debounced fetch
arminmeh Jan 9, 2025
5cdef95
Use apiRef to update filter model
arminmeh Jan 9, 2025
c30bf94
Lint
arminmeh Jan 9, 2025
eedefc1
Fix test
arminmeh Jan 10, 2025
fa6f22c
Avoid using setProps with debounce
arminmeh Jan 10, 2025
262eef3
Set page with different API
arminmeh Jan 10, 2025
c095e23
avoid creating new spy on component rerender
cherniavskii Jan 10, 2025
c650661
Revert "Avoid using setProps with debounce"
cherniavskii Jan 10, 2025
545e5bb
Revert test
arminmeh Jan 23, 2025
883d1fa
Fix treeData test
arminmeh Jan 24, 2025
4e87d1e
Fix test
arminmeh Jan 28, 2025
00b8051
Improve tests
arminmeh Jan 28, 2025
ed10845
Fix test
arminmeh Jan 28, 2025
7ff7d1c
Revert 'only'
arminmeh Jan 28, 2025
852c44d
Fix lazy loading tests
arminmeh Jan 28, 2025
1994bb9
Fix and align data source tests
arminmeh Jan 29, 2025
65195eb
lint
arminmeh Jan 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions packages/x-data-grid-premium/src/tests/dataSourceAggregation.DataGridPremium.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,18 +99,12 @@ describe('<DataGridPremium /> - Data source aggregation', () => {

it('should show aggregation option in the column menu', async () => {
const { user } = render(<TestDataSourceAggregation />);
await waitFor(() => {
arminmeh marked this conversation as resolved.
Show resolved Hide resolved
expect(getRowsSpy.callCount).to.be.greaterThan(0);
});
await user.click(within(getColumnHeaderCell(0)).getByLabelText('Menu'));
expect(screen.queryByLabelText('Aggregation')).not.to.equal(null);
});

it('should not show aggregation option in the column menu when no aggregation function is defined', async () => {
const { user } = render(<TestDataSourceAggregation aggregationFunctions={{}} />);
await waitFor(() => {
expect(getRowsSpy.callCount).to.be.greaterThan(0);
});
await user.click(within(getColumnHeaderCell(0)).getByLabelText('Menu'));
expect(screen.queryByLabelText('Aggregation')).to.equal(null);
});
Expand All @@ -123,9 +117,6 @@ describe('<DataGridPremium /> - Data source aggregation', () => {
}}
/>,
);
await waitFor(() => {
expect(getRowsSpy.callCount).to.be.greaterThan(0);
});
expect(getRowsSpy.args[0][0].aggregationModel).to.deep.equal({ id: 'size' });
});

Expand Down
12 changes: 6 additions & 6 deletions packages/x-data-grid-pro/src/hooks/features/dataSource/useGridDataSourceBase.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
GridDataSourceCache,
runIf,
} from '@mui/x-data-grid/internals';
import { unstable_debounce as debounce } from '@mui/utils';
import { GridPrivateApiPro } from '../../../models/gridApiPro';
import { DataGridProProcessedProps } from '../../../models/dataGridProProps';
import { gridGetRowsParamsSelector, gridDataSourceErrorsSelector } from './gridDataSourceSelector';
Expand Down Expand Up @@ -64,9 +65,6 @@ export const useGridDataSourceBase = <Api extends GridPrivateApiPro>(
| 'unstable_dataSource'
| 'unstable_dataSourceCache'
| 'unstable_onDataSourceError'
| 'sortingMode'
| 'filterMode'
| 'paginationMode'
| 'pageSizeOptions'
| 'treeData'
| 'unstable_lazyLoading'
Expand Down Expand Up @@ -324,6 +322,8 @@ export const useGridDataSourceBase = <Api extends GridPrivateApiPro>(
[apiRef],
);

const debouncedFetchRows = React.useMemo(() => debounce(fetchRows, 0), [fetchRows]);
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if using a microtask callback wouldn't have been better than a timeout (aka task). It would keep the overall update to within the same event loop cycle.


const handleStrategyActivityChange = React.useCallback<
GridEventListener<'strategyAvailabilityChange'>
>(() => {
Expand Down Expand Up @@ -422,9 +422,9 @@ export const useGridDataSourceBase = <Api extends GridPrivateApiPro>(
},
events: {
strategyAvailabilityChange: handleStrategyActivityChange,
sortModelChange: runIf(defaultRowsUpdateStrategyActive, () => fetchRows()),
filterModelChange: runIf(defaultRowsUpdateStrategyActive, () => fetchRows()),
paginationModelChange: runIf(defaultRowsUpdateStrategyActive, () => fetchRows()),
sortModelChange: runIf(defaultRowsUpdateStrategyActive, () => debouncedFetchRows()),
filterModelChange: runIf(defaultRowsUpdateStrategyActive, () => debouncedFetchRows()),
paginationModelChange: runIf(defaultRowsUpdateStrategyActive, () => debouncedFetchRows()),
romgrk marked this conversation as resolved.
Show resolved Hide resolved
},
};
};
50 changes: 27 additions & 23 deletions ...es/x-data-grid-pro/src/hooks/features/serverSideLazyLoader/useGridDataSourceLazyLoader.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as React from 'react';
import { RefObject } from '@mui/x-internals/types';
import { throttle } from '@mui/x-internals/throttle';
import { unstable_debounce as debounce } from '@mui/utils';
import {
useGridApiEventHandler,
useGridSelector,
Expand Down Expand Up @@ -79,6 +80,15 @@ export const useGridDataSourceLazyLoader = (
const loadingTrigger = React.useRef<LoadingTrigger | null>(null);
const rowsStale = React.useRef<boolean>(false);

const fetchRows = React.useCallback(
(params: Partial<GridGetRowsParams>) => {
privateApiRef.current.unstable_dataSource.fetchRows(GRID_ROOT_GROUP_ID, params);
},
[privateApiRef],
);

const debouncedFetchRows = React.useMemo(() => debounce(fetchRows, 0), [fetchRows]);

// Adjust the render context range to fit the pagination model's page size
// First row index should be decreased to the start of the page, end row index should be increased to the end of the page
const adjustRowParams = React.useCallback(
Expand Down Expand Up @@ -108,8 +118,8 @@ export const useGridDataSourceLazyLoader = (
filterModel,
};

privateApiRef.current.unstable_dataSource.fetchRows(GRID_ROOT_GROUP_ID, getRowsParams);
}, [privateApiRef, sortModel, filterModel, paginationModel.pageSize]);
fetchRows(getRowsParams);
}, [privateApiRef, sortModel, filterModel, paginationModel.pageSize, fetchRows]);

const ensureValidRowCount = React.useCallback(
(previousLoadingTrigger: LoadingTrigger, newLoadingTrigger: LoadingTrigger) => {
Expand Down Expand Up @@ -303,7 +313,7 @@ export const useGridDataSourceLazyLoader = (
);

const handleRowCountChange = React.useCallback(() => {
if (loadingTrigger.current === null) {
if (rowsStale.current || loadingTrigger.current === null) {
return;
}

Expand All @@ -314,11 +324,12 @@ export const useGridDataSourceLazyLoader = (

const handleScrolling: GridEventListener<'scrollPositionChange'> = React.useCallback(
(newScrollPosition) => {
if (rowsStale.current || loadingTrigger.current !== LoadingTrigger.SCROLL_END) {
return;
}

const renderContext = gridRenderContextSelector(privateApiRef);
if (
loadingTrigger.current !== LoadingTrigger.SCROLL_END ||
previousLastRowIndex.current >= renderContext.lastRowIndex
) {
if (previousLastRowIndex.current >= renderContext.lastRowIndex) {
return;
}

Expand All @@ -336,10 +347,7 @@ export const useGridDataSourceLazyLoader = (
};

privateApiRef.current.setLoading(true);
privateApiRef.current.unstable_dataSource.fetchRows(
GRID_ROOT_GROUP_ID,
adjustRowParams(getRowsParams),
);
fetchRows(adjustRowParams(getRowsParams));
}
},
[
Expand All @@ -350,14 +358,15 @@ export const useGridDataSourceLazyLoader = (
dimensions,
paginationModel.pageSize,
adjustRowParams,
fetchRows,
],
);

const handleRenderedRowsIntervalChange = React.useCallback<
GridEventListener<'renderedRowsIntervalChange'>
>(
(params) => {
if (loadingTrigger.current !== LoadingTrigger.VIEWPORT) {
if (rowsStale.current || loadingTrigger.current !== LoadingTrigger.VIEWPORT) {
return;
}

Expand Down Expand Up @@ -401,10 +410,7 @@ export const useGridDataSourceLazyLoader = (
getRowsParams.start = skeletonRowsSection.firstRowIndex;
getRowsParams.end = skeletonRowsSection.lastRowIndex;

privateApiRef.current.unstable_dataSource.fetchRows(
GRID_ROOT_GROUP_ID,
adjustRowParams(getRowsParams),
);
fetchRows(adjustRowParams(getRowsParams));
},
[
privateApiRef,
Expand All @@ -413,6 +419,7 @@ export const useGridDataSourceLazyLoader = (
sortModel,
filterModel,
adjustRowParams,
fetchRows,
],
);

Expand Down Expand Up @@ -444,12 +451,9 @@ export const useGridDataSourceLazyLoader = (
};

privateApiRef.current.setLoading(true);
privateApiRef.current.unstable_dataSource.fetchRows(
GRID_ROOT_GROUP_ID,
adjustRowParams(getRowsParams),
);
debouncedFetchRows(adjustRowParams(getRowsParams));
},
[privateApiRef, filterModel, paginationModel.pageSize, adjustRowParams],
[privateApiRef, filterModel, paginationModel.pageSize, adjustRowParams, debouncedFetchRows],
);

const handleGridFilterModelChange = React.useCallback<GridEventListener<'filterModelChange'>>(
Expand All @@ -464,9 +468,9 @@ export const useGridDataSourceLazyLoader = (
};

privateApiRef.current.setLoading(true);
privateApiRef.current.unstable_dataSource.fetchRows(GRID_ROOT_GROUP_ID, getRowsParams);
debouncedFetchRows(getRowsParams);
},
[privateApiRef, sortModel, paginationModel.pageSize],
[privateApiRef, sortModel, paginationModel.pageSize, debouncedFetchRows],
);

const handleStrategyActivityChange = React.useCallback<
Expand Down
Loading
Loading