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

Update useInfiniteGetList to skip getOne cache population for large responses #9536

Merged
merged 1 commit into from
Dec 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
243 changes: 243 additions & 0 deletions packages/ra-core/src/dataProvider/useInfiniteGetList.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,39 @@ import * as React from 'react';
import expect from 'expect';
import { screen, render, waitFor, fireEvent } from '@testing-library/react';
import { Basic, PageInfo } from './useInfiniteGetList.stories';
import { QueryClient } from '@tanstack/react-query';
import { testDataProvider } from './testDataProvider';
import { PaginationPayload, SortPayload } from '../types';
import { useInfiniteGetList } from './useInfiniteGetList';
import { CoreAdminContext } from '..';

describe('useInfiniteGetList', () => {
const UseInfiniteGetList = ({
resource = 'posts',
pagination = { page: 1, perPage: 10 },
sort = { field: 'id', order: 'DESC' } as const,
filter = {},
options = {},
meta = undefined,
callback = null,
}: {
resource?: string;
pagination?: PaginationPayload;
sort?: SortPayload;
filter?: any;
options?: any;
meta?: any;
callback?: any;
}) => {
const hookValue = useInfiniteGetList(
resource,
{ pagination, sort, filter, meta },
options
);
if (callback) callback(hookValue);
return <div>hello</div>;
};

it('should call dataProvider.getList() on mount', async () => {
const dataProvider = {
getList: jest.fn(() =>
Expand Down Expand Up @@ -136,6 +167,218 @@ describe('useInfiniteGetList', () => {
});
});

it('should not pre-populate getOne Query Cache if more than 100 results', async () => {
const callback: any = jest.fn();
const queryClient = new QueryClient();
const dataProvider = testDataProvider({
// @ts-ignore
getList: jest.fn((_resource, { pagination: { page, perPage } }) =>
Promise.resolve({
data: Array.from(Array(perPage).keys()).map(index => ({
id: index + 1 + (page - 1) * perPage,
title: `item ${index + 1 + (page - 1) * perPage}`,
})),
total: perPage * 2,
})
),
});

render(
<CoreAdminContext
queryClient={queryClient}
dataProvider={dataProvider}
>
<UseInfiniteGetList
callback={callback}
pagination={{ page: 1, perPage: 101 }}
/>
</CoreAdminContext>
);
await waitFor(() => {
expect(callback).toHaveBeenCalledWith(
expect.objectContaining({
data: expect.objectContaining({
pages: expect.arrayContaining([
expect.objectContaining({
data: expect.arrayContaining([
{ id: 1, title: 'item 1' },
{ id: 101, title: 'item 101' },
]),
}),
]),
}),
})
);
});
expect(
queryClient.getQueryData(['posts', 'getOne', { id: '1' }])
).toBeUndefined();
});

it('should not pre-populate getOne Query Cache if more than 100 results across several pages', async () => {
let hookValue;
const callback: any = jest.fn(value => {
hookValue = value;
});
const queryClient = new QueryClient();
const dataProvider = testDataProvider({
// @ts-ignore
getList: jest.fn((_resource, { pagination: { page, perPage } }) =>
Promise.resolve({
data: Array.from(Array(perPage).keys()).map(index => ({
id: index + 1 + (page - 1) * perPage,
title: `item ${index + 1 + (page - 1) * perPage}`,
})),
total: perPage * 2,
})
),
});

render(
<CoreAdminContext
queryClient={queryClient}
dataProvider={dataProvider}
>
<UseInfiniteGetList
callback={callback}
pagination={{ page: 1, perPage: 51 }}
/>
</CoreAdminContext>
);
await waitFor(() => {
expect(callback).toHaveBeenCalledWith(
expect.objectContaining({
data: expect.objectContaining({
pages: expect.arrayContaining([
expect.objectContaining({
data: expect.arrayContaining([
{ id: 1, title: 'item 1' },
{ id: 51, title: 'item 51' },
]),
}),
]),
}),
})
);
});
expect(
queryClient.getQueryData(['posts', 'getOne', { id: '1' }])
).toBeDefined();
expect(
queryClient.getQueryData(['posts', 'getOne', { id: '51' }])
).toBeDefined();
expect(
queryClient.getQueryData(['posts', 'getOne', { id: '52' }])
).not.toBeDefined();
// Fetch next page
hookValue.fetchNextPage();
await waitFor(() => {
expect(callback).toHaveBeenCalledWith(
expect.objectContaining({
data: expect.objectContaining({
pages: expect.arrayContaining([
expect.objectContaining({
data: expect.arrayContaining([
{ id: 52, title: 'item 52' },
{ id: 102, title: 'item 102' },
]),
}),
]),
}),
})
);
});
expect(
queryClient.getQueryData(['posts', 'getOne', { id: '1' }])
).toBeDefined();
expect(
queryClient.getQueryData(['posts', 'getOne', { id: '51' }])
).toBeDefined();
// query data for item 52 should still be undefined
expect(
queryClient.getQueryData(['posts', 'getOne', { id: '52' }])
).not.toBeDefined();
});

it('should only populate the getOne Query Cache with the records from the last fetched page', async () => {
let hookValue;
const callback: any = jest.fn(value => {
hookValue = value;
});
const queryClient = new QueryClient();
const dataProvider = testDataProvider({
// @ts-ignore
getList: jest.fn((_resource, { pagination: { page } }) =>
Promise.resolve({
data: [
{
id: page,
title: `item ${page}`,
},
],
total: 2,
})
),
});
render(
<CoreAdminContext
queryClient={queryClient}
dataProvider={dataProvider}
>
<UseInfiniteGetList
callback={callback}
pagination={{ page: 1, perPage: 1 }}
/>
</CoreAdminContext>
);
await waitFor(() => {
expect(callback).toHaveBeenCalledWith(
expect.objectContaining({
data: expect.objectContaining({
pages: expect.arrayContaining([
expect.objectContaining({
data: [{ id: 1, title: 'item 1' }],
}),
]),
}),
})
);
});
expect(
queryClient.getQueryData(['posts', 'getOne', { id: '1' }])
).toEqual({ id: 1, title: 'item 1' });
expect(
queryClient.getQueryData(['posts', 'getOne', { id: '2' }])
).toBeUndefined();
// Manually change query data for item 1
queryClient.setQueryData(['posts', 'getOne', { id: '1' }], {
id: 1,
title: 'changed!',
});
// Fetch next page
hookValue.fetchNextPage();
await waitFor(() => {
expect(callback).toHaveBeenCalledWith(
expect.objectContaining({
data: expect.objectContaining({
pages: expect.arrayContaining([
expect.objectContaining({
data: [{ id: 2, title: 'item 2' }],
}),
]),
}),
})
);
});
expect(
queryClient.getQueryData(['posts', 'getOne', { id: '2' }])
).toEqual({ id: 2, title: 'item 2' });
// Check that the getOne Query Cache for item 1 has not been overriden
expect(
queryClient.getQueryData(['posts', 'getOne', { id: '1' }])
).toEqual({ id: 1, title: 'changed!' });
});

describe('fetchNextPage', () => {
it('should fetch the next page when the dataProvider uses total', async () => {
render(<Basic />);
Expand Down
30 changes: 19 additions & 11 deletions packages/ra-core/src/dataProvider/useInfiniteGetList.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import { useDataProvider } from './useDataProvider';
import { useEffect, useRef } from 'react';
import { useEvent } from '../util';

const MAX_DATA_LENGTH_TO_CACHE = 100;

/**
* Call the dataProvider.getList() method and return the resolved result
* as well as the loading state. The return from useInfiniteGetList is equivalent to the return from react-hook form useInfiniteQuery.
Expand Down Expand Up @@ -159,18 +161,24 @@ export const useInfiniteGetList = <RecordType extends RaRecord = any>(
useEffect(() => {
if (result.data === undefined || result.isFetching) return;
// optimistically populate the getOne cache
result.data.pages.forEach(page => {
page.data.forEach(record => {
queryClient.setQueryData(
[
resourceValue.current,
'getOne',
{ id: String(record.id), meta: metaValue.current },
],
oldRecord => oldRecord ?? record
);
const allPagesDataLength = result.data.pages.reduce(
(acc, page) => acc + page.data.length,
0
);
if (allPagesDataLength <= MAX_DATA_LENGTH_TO_CACHE) {
result.data.pages.forEach(page => {
page.data.forEach(record => {
queryClient.setQueryData(
[
resourceValue.current,
'getOne',
{ id: String(record.id), meta: metaValue.current },
],
oldRecord => oldRecord ?? record
);
});
});
});
}

onSuccessEvent(result.data);
}, [onSuccessEvent, queryClient, result.data, result.isFetching]);
Expand Down
Loading