From 13ce0d36a12e0f3ad8c2b8d2517ab360aa1960b5 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Kaiser Date: Wed, 20 Dec 2023 18:11:34 +0100 Subject: [PATCH] Update `useInfiniteGetList` to skip `getOne` cache population for large responses --- .../dataProvider/useInfiniteGetList.spec.tsx | 243 ++++++++++++++++++ .../src/dataProvider/useInfiniteGetList.ts | 30 ++- 2 files changed, 262 insertions(+), 11 deletions(-) diff --git a/packages/ra-core/src/dataProvider/useInfiniteGetList.spec.tsx b/packages/ra-core/src/dataProvider/useInfiniteGetList.spec.tsx index f91af408f39..6596fac1497 100644 --- a/packages/ra-core/src/dataProvider/useInfiniteGetList.spec.tsx +++ b/packages/ra-core/src/dataProvider/useInfiniteGetList.spec.tsx @@ -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
hello
; + }; + it('should call dataProvider.getList() on mount', async () => { const dataProvider = { getList: jest.fn(() => @@ -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( + + + + ); + 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( + + + + ); + 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( + + + + ); + 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(); diff --git a/packages/ra-core/src/dataProvider/useInfiniteGetList.ts b/packages/ra-core/src/dataProvider/useInfiniteGetList.ts index 2d62d51e7de..a5c16d0ba33 100644 --- a/packages/ra-core/src/dataProvider/useInfiniteGetList.ts +++ b/packages/ra-core/src/dataProvider/useInfiniteGetList.ts @@ -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. @@ -159,18 +161,24 @@ export const useInfiniteGetList = ( 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]);