diff --git a/packages/base-data-service/CHANGELOG.md b/packages/base-data-service/CHANGELOG.md index c3b10e0015..0e6fa4d503 100644 --- a/packages/base-data-service/CHANGELOG.md +++ b/packages/base-data-service/CHANGELOG.md @@ -46,6 +46,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Bump `@metamask/messenger` from `^1.2.0` to `^2.0.0` ([#9392](https://github.com/MetaMask/core/pull/9392)) - Add dependency `cockatiel` (`^3.1.2`) ([#9418](https://github.com/MetaMask/core/pull/9418)) +### Fixed + +- Prevent `fetchInfiniteQuery` from duplicating a page when re-fetching a page param that is already present in the cache ([#9915](https://github.com/MetaMask/core/pull/9915), [#9900](https://github.com/MetaMask/core/issues/9900)) + - The fresh page now replaces the cached one in place instead of being appended or prepended, which previously left duplicate, out-of-order pages in the cache. + ## [0.1.3] ### Changed diff --git a/packages/base-data-service/src/BaseDataService.test.ts b/packages/base-data-service/src/BaseDataService.test.ts index 152eb6bd81..d0add6ac36 100644 --- a/packages/base-data-service/src/BaseDataService.test.ts +++ b/packages/base-data-service/src/BaseDataService.test.ts @@ -121,6 +121,100 @@ describe('BaseDataService', () => { expect(page2.data).not.toStrictEqual(page3.data); }); + it('replaces an already-cached page in place when fetching it again', async () => { + cleanAll(); + const messenger = new Messenger({ namespace: serviceName }); + const service = new ExampleDataService(messenger); + const publishSpy = jest.spyOn(messenger, 'publish'); + + const page2Body = { + data: [ + { + hash: '0xcecd28aa5bd781ffd2a6d960578ffc6c89ac390e8d02baebc977a827956394e9', + timestamp: '2025-12-29T11:51:08.000Z', + }, + ], + pageInfo: { + count: 1, + hasNextPage: false, + hasPreviousPage: true, + startCursor: 'page1-cursor', + endCursor: 'page3-cursor', + }, + }; + mockTransactionsPage2({ status: 200, body: page2Body }); + mockTransactionsPage2({ status: 200, body: page2Body }); + + const page2 = await service.getActivity(TEST_ADDRESS, { + after: TRANSACTIONS_PAGE_2_CURSOR, + }); + expect(page2.data).toHaveLength(1); + + // A refetch re-requests a page that is already present in the cache. + const page2Again = await service.getActivity(TEST_ADDRESS, { + after: TRANSACTIONS_PAGE_2_CURSOR, + }); + expect(page2Again.data).toStrictEqual(page2.data); + + const queryKey = ['ExampleDataService:getActivity', TEST_ADDRESS]; + const hash = hashKey(queryKey); + const cacheUpdate = publishSpy.mock.calls + .filter(([event]) => event === `ExampleDataService:cacheUpdated:${hash}`) + .at(-1)?.[1] as { + state: { queries: [{ state: { data: { pages: unknown[] } } }] }; + }; + + // The cache must hold each page exactly once. + expect(cacheUpdate.state.queries[0].state.data.pages).toHaveLength(1); + }); + + it('replaces an already-cached page in place when fetching it again in the forward direction', async () => { + cleanAll(); + const messenger = new Messenger({ namespace: serviceName }); + const service = new ExampleDataService(messenger); + const publishSpy = jest.spyOn(messenger, 'publish'); + + const page2Body = { + data: [ + { + hash: '0xcecd28aa5bd781ffd2a6d960578ffc6c89ac390e8d02baebc977a827956394e9', + timestamp: '2025-12-29T11:51:08.000Z', + }, + ], + pageInfo: { + count: 1, + hasNextPage: true, + hasPreviousPage: false, + startCursor: null, + endCursor: TRANSACTIONS_PAGE_2_CURSOR, + }, + }; + mockTransactionsPage2({ status: 200, body: page2Body }); + mockTransactionsPage2({ status: 200, body: page2Body }); + + const page2 = await service.getActivity(TEST_ADDRESS, { + after: TRANSACTIONS_PAGE_2_CURSOR, + }); + expect(page2.data).toHaveLength(1); + + // A refetch re-requests a page that is already present in the cache. + const page2Again = await service.getActivity(TEST_ADDRESS, { + after: TRANSACTIONS_PAGE_2_CURSOR, + }); + expect(page2Again.data).toStrictEqual(page2.data); + + const queryKey = ['ExampleDataService:getActivity', TEST_ADDRESS]; + const hash = hashKey(queryKey); + const cacheUpdate = publishSpy.mock.calls + .filter(([event]) => event === `ExampleDataService:cacheUpdated:${hash}`) + .at(-1)?.[1] as { + state: { queries: [{ state: { data: { pages: unknown[] } } }] }; + }; + + // The cache must hold each page exactly once. + expect(cacheUpdate.state.queries[0].state.data.pages).toHaveLength(1); + }); + it('emits `:cacheUpdated` events when cache is updated', async () => { const messenger = new Messenger({ namespace: serviceName }); const service = new ExampleDataService(messenger); diff --git a/packages/base-data-service/src/BaseDataService.ts b/packages/base-data-service/src/BaseDataService.ts index d0d419f65c..ba2cbf7596 100644 --- a/packages/base-data-service/src/BaseDataService.ts +++ b/packages/base-data-service/src/BaseDataService.ts @@ -333,6 +333,11 @@ export class BaseDataService< } const { pages, pageParams } = query.state.data; + + const existingIndex = pageParams.findIndex((param) => + deepEqual(param, pageParam), + ); + const next = options.getNextPageParam( pages[pages.length - 1], pages, @@ -347,6 +352,28 @@ export class BaseDataService< { meta: { fetchMore: { direction } } }, ); + if (existingIndex !== -1) { + // The requested page was already cached. `query.fetch` appended or + // prepended a fresh copy instead of replacing it, which would leave + // duplicate, out-of-order pages in the cache. Collapse the duplicate + // and keep the fresh page at the original position. + const isForward = direction === 'forward'; + const nextPages = [...result.pages]; + const nextPageParams = [...result.pageParams]; + const freshPage = isForward ? nextPages.pop() : nextPages.shift(); + if (isForward) { + nextPageParams.pop(); + } else { + nextPageParams.shift(); + } + nextPages[existingIndex] = freshPage as TData; + this.#queryClient.setQueryData(options.queryKey, { + pages: nextPages, + pageParams: nextPageParams, + }); + return freshPage as TData; + } + const pageIndex = result.pageParams.findIndex((param) => deepEqual(param, pageParam), );