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

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

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ describe('<DataGridPremium /> - Data source aggregation', () => {

it('should show aggregation option in the column menu', async () => {
const { user } = render(<TestDataSourceAggregation />);
await waitFor(() => {
expect(getRowsSpy.callCount).to.be.greaterThan(0);
});
await user.click(within(getColumnHeaderCell(0)).getByLabelText('Menu'));
expect(screen.queryByLabelText('Aggregation')).not.to.equal(null);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,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 @@ -63,9 +64,6 @@ export const useGridDataSourceBase = <Api extends GridPrivateApiPro>(
| 'unstable_dataSource'
| 'unstable_dataSourceCache'
| 'unstable_onDataSourceError'
| 'sortingMode'
| 'filterMode'
| 'paginationMode'
| 'pageSizeOptions'
| 'treeData'
| 'unstable_lazyLoading'
Expand Down Expand Up @@ -323,6 +321,8 @@ export const useGridDataSourceBase = <Api extends GridPrivateApiPro>(
[apiRef],
);

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

const handleStrategyActivityChange = React.useCallback<
GridEventListener<'strategyAvailabilityChange'>
>(() => {
Expand Down Expand Up @@ -421,9 +421,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()),
},
};
};
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as React from 'react';
import { throttle } from '@mui/x-internals/throttle';
import { unstable_debounce as debounce } from '@mui/utils';
import {
useGridApiEventHandler,
useGridSelector,
Expand Down Expand Up @@ -78,6 +79,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 @@ -107,8 +117,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 @@ -302,7 +312,7 @@ export const useGridDataSourceLazyLoader = (
);

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

Expand All @@ -313,11 +323,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 @@ -335,10 +346,7 @@ export const useGridDataSourceLazyLoader = (
};

privateApiRef.current.setLoading(true);
privateApiRef.current.unstable_dataSource.fetchRows(
GRID_ROOT_GROUP_ID,
adjustRowParams(getRowsParams),
);
fetchRows(adjustRowParams(getRowsParams));
}
},
[
Expand All @@ -349,14 +357,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 @@ -400,10 +409,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 @@ -412,6 +418,7 @@ export const useGridDataSourceLazyLoader = (
sortModel,
filterModel,
adjustRowParams,
fetchRows,
],
);

Expand Down Expand Up @@ -443,12 +450,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 @@ -463,9 +467,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
40 changes: 34 additions & 6 deletions packages/x-data-grid-pro/src/tests/dataSource.DataGridPro.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ import {
GridGetRowsResponse,
useGridApiRef,
} from '@mui/x-data-grid-pro';
import { SinonSpy, spy } from 'sinon';
import { SinonStub, spy, stub } from 'sinon';
import { describeSkipIf, isJSDOM } from 'test/utils/skipIf';
import useLazyRef from '@mui/utils/useLazyRef';
import { getKeyDefault } from '../hooks/features/dataSource/cache';

const cache = new Map<string, GridGetRowsResponse>();
Expand All @@ -29,7 +30,7 @@ describeSkipIf(isJSDOM)('<DataGridPro /> - Data source', () => {
const { render } = createRenderer();

let apiRef: React.RefObject<GridApi>;
let fetchRowsSpy: SinonSpy;
let fetchRowsSpy: SinonStub;
let mockServer: ReturnType<typeof useMockServer>;

function TestDataSource(props: Partial<DataGridProProps> & { shouldRequestsFail?: boolean }) {
Expand All @@ -40,8 +41,15 @@ describeSkipIf(isJSDOM)('<DataGridPro /> - Data source', () => {
{ useCursorPagination: false, minDelay: 0, maxDelay: 0, verbose: false },
shouldRequestsFail,
);
fetchRowsSpy = spy(mockServer, 'fetchRows');
const { fetchRows } = mockServer;

// Reuse the same stub between rerenders to properly count the calls
fetchRowsSpy = useLazyRef(() => stub()).current;
Copy link
Member

Choose a reason for hiding this comment

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

The reason setProps was causing issues is that fetchRowsSpy was recreated on each TestDataSource rerender 😅
I've fixed that and reverted back the usage of setProps.
Some tests will probably fail though, but IMO in that case there's something wrong with the test or implementation.


const originalFetchRows = mockServer.fetchRows;
const fetchRows = React.useMemo<typeof originalFetchRows>(() => {
fetchRowsSpy.callsFake(originalFetchRows);
return (...args) => fetchRowsSpy(...args);
}, [originalFetchRows]);

const dataSource: GridDataSource = React.useMemo(
() => ({
Expand Down Expand Up @@ -83,6 +91,7 @@ describeSkipIf(isJSDOM)('<DataGridPro /> - Data source', () => {
// eslint-disable-next-line mocha/no-top-level-hooks
beforeEach(() => {
cache.clear();
fetchRowsSpy?.reset();
});

it('should fetch the data on initial render', async () => {
Expand All @@ -93,11 +102,13 @@ describeSkipIf(isJSDOM)('<DataGridPro /> - Data source', () => {
});

it('should re-fetch the data on filter change', async () => {
const { setProps } = render(<TestDataSource />);
render(<TestDataSource />);
await waitFor(() => {
expect(fetchRowsSpy.callCount).to.equal(1);
});
setProps({ filterModel: { items: [{ field: 'name', value: 'John', operator: 'contains' }] } });
apiRef.current.setFilterModel({
items: [{ field: 'name', value: 'John', operator: 'contains' }],
});
await waitFor(() => {
expect(fetchRowsSpy.callCount).to.equal(2);
});
Expand Down Expand Up @@ -125,6 +136,23 @@ describeSkipIf(isJSDOM)('<DataGridPro /> - Data source', () => {
});
});

it('should re-fetch the data once if multiple models have changed', async () => {
const { setProps } = render(<TestDataSource />);
await waitFor(() => {
expect(fetchRowsSpy.callCount).to.equal(1);
});

setProps({
paginationModel: { page: 1, pageSize: 10 },
sortModel: [{ field: 'name', sort: 'asc' }],
filterModel: { items: [{ field: 'name', value: 'John', operator: 'contains' }] },
});

await waitFor(() => {
expect(fetchRowsSpy.callCount).to.equal(2);
});
});

describe('Cache', () => {
it('should cache the data using the default cache', async () => {
render(<TestDataSource />);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ import {
GridGetRowsResponse,
useGridApiRef,
} from '@mui/x-data-grid-pro';
import { SinonSpy, spy } from 'sinon';
import { SinonStub, stub } from 'sinon';
import { describeSkipIf, isJSDOM } from 'test/utils/skipIf';
import useLazyRef from '@mui/utils/useLazyRef';

// Needs layout
describeSkipIf(isJSDOM)('<DataGridPro /> - Data source lazy loader', () => {
Expand All @@ -22,7 +23,7 @@ describeSkipIf(isJSDOM)('<DataGridPro /> - Data source lazy loader', () => {

let transformGetRowsResponse: (response: GridGetRowsResponse) => GridGetRowsResponse;
let apiRef: React.RefObject<GridApi>;
let fetchRowsSpy: SinonSpy;
let fetchRowsSpy: SinonStub;
let mockServer: ReturnType<typeof useMockServer>;

function TestDataSourceLazyLoader(props: Partial<DataGridProProps>) {
Expand All @@ -31,8 +32,15 @@ describeSkipIf(isJSDOM)('<DataGridPro /> - Data source lazy loader', () => {
{ rowLength: 100, maxColumns: 1 },
{ useCursorPagination: false, minDelay: 0, maxDelay: 0, verbose: false },
);
fetchRowsSpy = spy(mockServer, 'fetchRows');
const { fetchRows } = mockServer;

// Reuse the same stub between rerenders to properly count the calls
fetchRowsSpy = useLazyRef(() => stub()).current;

const originalFetchRows = mockServer.fetchRows;
const fetchRows = React.useMemo<typeof originalFetchRows>(() => {
fetchRowsSpy.callsFake(originalFetchRows);
return (...args) => fetchRowsSpy(...args);
}, [originalFetchRows]);

const dataSource: GridDataSource = React.useMemo(
() => ({
Expand Down Expand Up @@ -76,6 +84,7 @@ describeSkipIf(isJSDOM)('<DataGridPro /> - Data source lazy loader', () => {
// eslint-disable-next-line mocha/no-top-level-hooks
beforeEach(() => {
transformGetRowsResponse = defaultTransformGetRowsResponse;
fetchRowsSpy?.reset();
});

it('should load the first page initially', async () => {
Expand All @@ -85,6 +94,22 @@ describeSkipIf(isJSDOM)('<DataGridPro /> - Data source lazy loader', () => {
});
});

it('should re-fetch the data once if multiple models have changed', async () => {
const { setProps } = render(<TestDataSourceLazyLoader />);
await waitFor(() => {
expect(fetchRowsSpy.callCount).to.equal(1);
});

setProps({
sortModel: [{ field: 'name', sort: 'asc' }],
filterModel: { items: [{ field: 'name', value: 'John', operator: 'contains' }] },
});

await waitFor(() => {
expect(fetchRowsSpy.callCount).to.equal(2);
});
});

describe('Viewport loading', () => {
it('should render skeleton rows if rowCount is bigger than the number of rows', async () => {
render(<TestDataSourceLazyLoader />);
Expand Down Expand Up @@ -212,6 +237,11 @@ describeSkipIf(isJSDOM)('<DataGridPro /> - Data source lazy loader', () => {

apiRef.current.scrollToIndexes({ rowIndex: 9 });

// wait until the debounced fetch
await waitFor(() => {
expect(fetchRowsSpy.callCount).to.equal(2);
});

// wait until the rows are rendered
await waitFor(() => expect(getRow(10)).not.to.be.undefined);

Expand All @@ -221,6 +251,10 @@ describeSkipIf(isJSDOM)('<DataGridPro /> - Data source lazy loader', () => {

apiRef.current.sortColumn(mockServer.columns[0].field, 'asc');

await waitFor(() => {
expect(fetchRowsSpy.callCount).to.equal(3);
});

const afterSortingSearchParams = new URL(fetchRowsSpy.lastCall.args[0]).searchParams;
// last row is the end of the first page
expect(afterSortingSearchParams.get('end')).to.equal('9');
Expand All @@ -233,6 +267,11 @@ describeSkipIf(isJSDOM)('<DataGridPro /> - Data source lazy loader', () => {

apiRef.current.scrollToIndexes({ rowIndex: 9 });

// wait until the debounced fetch
await waitFor(() => {
expect(fetchRowsSpy.callCount).to.equal(2);
});

// wait until the rows are rendered
await waitFor(() => expect(getRow(10)).not.to.be.undefined);

Expand All @@ -250,6 +289,10 @@ describeSkipIf(isJSDOM)('<DataGridPro /> - Data source lazy loader', () => {
],
});

await waitFor(() => {
expect(fetchRowsSpy.callCount).to.equal(3);
});

const afterFilteringSearchParams = new URL(fetchRowsSpy.lastCall.args[0]).searchParams;
// last row is the end of the first page
expect(afterFilteringSearchParams.get('end')).to.equal('9');
Expand All @@ -271,7 +314,9 @@ describeSkipIf(isJSDOM)('<DataGridPro /> - Data source lazy loader', () => {
setProps({ rowCount: 100 });

// The 11th row should be a skeleton
expect(getRow(10).dataset.id).to.equal('auto-generated-skeleton-row-root-10');
await waitFor(() =>
expect(getRow(10).dataset.id).to.equal('auto-generated-skeleton-row-root-10'),
);
});

it('should reset the grid if the rowCount becomes unknown', async () => {
Expand Down
Loading
Loading