refactor(base-data-service)!: require getNextPageParam and initialPageParam (align with TanStack)

packages/base-data-service/CHANGELOG.md

@@ -40,6 +40,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - This is technically a breaking change, but this was not used in any of our codebases
 - **BREAKING:** Bump `@tanstack/query-core` from `^4.43.0` to `^5.62.16` ([#9712](https://github.com/MetaMask/core/pull/9712))
   - The option types accepted by `fetchQuery`, `fetchInfiniteQuery`, and `invalidateQueries` now follow the query-core v5 API. Subclasses may need to rename `cacheTime` to `gcTime`, and infinite queries no longer accept an explicit page param through the `fetchMore` meta.
+- **BREAKING:** Require `getNextPageParam` and `initialPageParam` in `fetchInfiniteQuery` options ([#9872](https://github.com/MetaMask/core/pull/9872))
+  - Aligns with `@tanstack/query-core`'s own `fetchInfiniteQuery`. Data services that paginate must now provide a `getNextPageParam` (used to walk forward when refetching a multi-page query) and an `initialPageParam` (the first-page param, e.g. `null`). `getPreviousPageParam` stays optional.
 - Bump `@metamask/utils` from `^11.9.0` to `^11.11.0` ([#9074](https://github.com/MetaMask/core/pull/9074))
 - Bump `@metamask/controller-utils` from `^12.1.0` to `^12.3.0` ([#9058](https://github.com/MetaMask/core/pull/9058), [#9083](https://github.com/MetaMask/core/pull/9083), [#9218](https://github.com/MetaMask/core/pull/9218))
 - Bump `@metamask/messenger` from `^1.2.0` to `^2.0.0` ([#9392](https://github.com/MetaMask/core/pull/9392))

packages/base-data-service/src/BaseDataService.test.ts

@@ -160,55 +160,20 @@ describe('BaseDataService', () => {
     expect(first.data).not.toStrictEqual(jumped.data);
   });
 
-  it('handles paginated queries without page-param callbacks', async () => {
-    const messenger = new Messenger({ namespace: serviceName });
-    const service = new ExampleDataService(messenger);
-
-    const page1 = await service.getActivityByCursor(TEST_ADDRESS);
-
-    expect(page1.data).toHaveLength(3);
-
-    const page2 = await service.getActivityByCursor(TEST_ADDRESS, {
-      after: page1.pageInfo.endCursor,
-    });
-
-    expect(page2.data).toHaveLength(3);
-    expect(page2.data).not.toStrictEqual(page1.data);
-  });
-
-  it('refetches stale paginated queries without page-param callbacks', async () => {
-    const messenger = new Messenger({ namespace: serviceName });
-    const service = new ExampleDataService(messenger);
-
-    const page1 = await service.getActivityByCursor(TEST_ADDRESS);
-    await service.getActivityByCursor(TEST_ADDRESS, {
-      after: page1.pageInfo.endCursor,
-    });
-
-    // The query is stale (zero `staleTime`), so a param-less call rebuilds the
-    // cached pages. That rebuild walks `getNextPageParam`, which this query
-    // does not provide, so the base service must supply a no-op to avoid a
-    // throw.
-    mockTransactionsPage1();
-    const rebuilt = await service.getActivityByCursor(TEST_ADDRESS);
-
-    expect(rebuilt.data).toHaveLength(3);
-  });
-
   it('recovers the first page after a cold jump on refetch', async () => {
     const messenger = new Messenger({ namespace: serviceName });
-    const service = new ExampleDataService(messenger);
+    const service = new ExampleDataService(messenger, { activityStaleTime: 0 });
 
     // Cold jump straight to a later page.
-    const jumped = await service.getActivityByCursor(TEST_ADDRESS, {
+    const jumped = await service.getActivity(TEST_ADDRESS, {
       after: TRANSACTIONS_PAGE_2_CURSOR,
     });
     expect(jumped.data).toHaveLength(3);
 
     // A param-less refetch must fetch the real first page, not the jumped-to
     // page. The jump must not have overwritten the query's initial page param.
     mockTransactionsPage1();
-    const first = await service.getActivityByCursor(TEST_ADDRESS);
+    const first = await service.getActivity(TEST_ADDRESS);
 
     expect(first.data).toHaveLength(3);
     expect(first.data).not.toStrictEqual(jumped.data);
@@ -218,8 +183,8 @@ describe('BaseDataService', () => {
     const messenger = new Messenger({ namespace: serviceName });
     const service = new ExampleDataService(messenger);
 
-    // `getActivityByCursor` uses `null` as its initial page param.
-    await service.getActivityByCursor(TEST_ADDRESS);
+    // `getActivity` uses `null` as its initial page param.
+    await service.getActivity(TEST_ADDRESS);
 
     // `null` is a valid page param, so it must reach the query function rather
     // than being coerced to `undefined`.
@@ -230,9 +195,9 @@ describe('BaseDataService', () => {
     const messenger = new Messenger({ namespace: serviceName });
     const service = new ExampleDataService(messenger);
 
-    // `getActivityByCursor` sets a `null` initial page param, but an
-    // explicit jump target must still win.
-    const page = await service.getActivityByCursor(TEST_ADDRESS, {
+    // `getActivity` sets a `null` initial page param, but an explicit jump
+    // target must still win.
+    const page = await service.getActivity(TEST_ADDRESS, {
       after: TRANSACTIONS_PAGE_2_CURSOR,
     });
 

packages/base-data-service/src/BaseDataService.ts

@@ -338,11 +338,12 @@ export class BaseDataService<
           SkipToken
         >
       >;
-      // These are required by @tanstack/query-core for infinite queries but
-      // remain optional here: consumers may drive pagination purely by passing
-      // an explicit `pageParam` (see below).
-      initialPageParam?: TPageParam;
-      getNextPageParam?: GetNextPageParamFunction<TPageParam, TQueryFnData>;
+      // Required, matching `@tanstack/query-core`'s own `fetchInfiniteQuery`:
+      // the first-page param and the resolver used to walk forward when
+      // refetching a multi-page query. `getPreviousPageParam` stays optional
+      // (only needed for backward pagination).
+      initialPageParam: TPageParam;
+      getNextPageParam: GetNextPageParamFunction<TPageParam, TQueryFnData>;
       getPreviousPageParam?: GetPreviousPageParamFunction<
         TPageParam,
         TQueryFnData
@@ -392,27 +393,16 @@ export class BaseDataService<
         TPageParam
       >({
         ...options,
-        // `initialPageParam` identifies the query's first page. Keep it as the
-        // consumer's value (which may be `undefined` for the very first page)
-        // rather than an explicit per-call `pageParam`: overwriting it would
-        // make a cold jump to page N masquerade as the first page, so a later
-        // refetch could never recover the real first page. query-core accepts
-        // `undefined` at runtime but not in its `TPageParam` type, hence the
-        // cast.
-        initialPageParam: options.initialPageParam as TPageParam,
-        // Provide a no-op `getNextPageParam` when the consumer omits one.
-        // @tanstack/query-core walks `getNextPageParam` when it refetches a
-        // multi-page infinite query, so a missing resolver would throw once more
-        // than one page has been cached.
-        getNextPageParam: options.getNextPageParam ?? ((): null => null),
         queryFn: (context) =>
           this.#policy.execute(() =>
             // On a cold jump the caller passes an explicit `pageParam`; fetch
-            // that page, overriding whatever first-page param query-core would
-            // use (which may be a non-`undefined` `initialPageParam`).
-            // Otherwise use query-core's context param, which preserves a
-            // `null` initial page param. Only the fresh path reaches this
-            // wrapper; the cached path below supplies its own query function.
+            // that page, overriding the consumer's `initialPageParam` that
+            // query-core would otherwise use for the first page. Overriding via
+            // `queryFn` (rather than `initialPageParam`) keeps the query's
+            // stored first-page param intact, so a later refetch can still
+            // recover the real first page. Otherwise use query-core's context
+            // param. Only the fresh path reaches this wrapper; the cached path
+            // below supplies its own query function.
             options.queryFn(
               pageParam === undefined ? context : { ...context, pageParam },
             ),

packages/base-data-service/tests/ExampleDataService.ts

@@ -60,13 +60,21 @@ export class ExampleDataService extends BaseDataService<
 
   readonly #tokensBaseUrl = 'https://tokens.api.cx.metamask.io';
 
-  // Records the page params that `getActivityByCursor`'s query function
-  // is invoked with, so tests can assert what actually reached it.
+  // Records the page params that `getActivity`'s query function is invoked
+  // with, so tests can assert what actually reached it.
   readonly pageParamsSeen: (PageParam | null | undefined)[] = [];
 
+  readonly #activityStaleTime: number;
+
   constructor(
     messenger: ExampleMessenger,
-    { persistenceConfig }: { persistenceConfig?: PersistenceConfiguration } = {
+    {
+      persistenceConfig,
+      activityStaleTime = inMilliseconds(5, Duration.Minute),
+    }: {
+      persistenceConfig?: PersistenceConfiguration;
+      activityStaleTime?: number;
+    } = {
       persistenceConfig: { maxAge: inMilliseconds(1, Duration.Day) },
     },
   ) {
@@ -81,6 +89,8 @@ export class ExampleDataService extends BaseDataService<
       persistenceConfig,
     });
 
+    this.#activityStaleTime = activityStaleTime;
+
     this.messenger.registerMethodActionHandlers(
       this,
       MESSENGER_EXPOSED_METHODS,
@@ -117,11 +127,13 @@ export class ExampleDataService extends BaseDataService<
       unknown,
       GetActivityResponse,
       [string, string],
-      PageParam
+      PageParam | null
     >(
       {
         queryKey: [`${this.name}:getActivity`, address],
         queryFn: async ({ pageParam }) => {
+          this.pageParamsSeen.push(pageParam);
+
           const caipAddress = `eip155:0:${address.toLowerCase()}`;
           const url = new URL(
             `${this.#accountsBaseUrl}/v4/multiaccount/transactions?limit=3&accountAddresses=${caipAddress}`,
@@ -143,71 +155,14 @@ export class ExampleDataService extends BaseDataService<
 
           return response.json();
         },
+        initialPageParam: null,
         getPreviousPageParam: ({ pageInfo }) =>
           pageInfo.hasPreviousPage
             ? { before: pageInfo.startCursor }
             : undefined,
         getNextPageParam: ({ pageInfo }) =>
           pageInfo.hasNextPage ? { after: pageInfo.endCursor } : undefined,
-        staleTime: inMilliseconds(5, Duration.Minute),
-      },
-      page,
-    );
-  }
-
-  /**
-   * Fetch activity by cursor. Unlike `getActivity`, this omits the
-   * `getNextPageParam` / `getPreviousPageParam` callbacks and drives pagination
-   * purely by the explicit page param passed to the base method, the way a
-   * consumer that paginates by cursor does. Uses `null` as its first-page param
-   * and a zero `staleTime` so refetches can be exercised, and records every
-   * page param the query function receives in `pageParamsSeen`.
-   *
-   * @param address - The account address.
-   * @param page - The page to fetch. Passed last so this method works when
-   * invoked through `createUIQueryClient`, which appends the page param as the
-   * final argument.
-   * @returns A page of activity.
-   */
-  async getActivityByCursor(
-    address: string,
-    page?: PageParam,
-  ): Promise<GetActivityResponse> {
-    return this.fetchInfiniteQuery<
-      GetActivityResponse,
-      unknown,
-      GetActivityResponse,
-      [string, string],
-      PageParam | null
-    >(
-      {
-        queryKey: [`${this.name}:getActivityByCursor`, address],
-        queryFn: async ({ pageParam }) => {
-          this.pageParamsSeen.push(pageParam);
-
-          const caipAddress = `eip155:0:${address.toLowerCase()}`;
-          const url = new URL(
-            `${this.#accountsBaseUrl}/v4/multiaccount/transactions?limit=3&accountAddresses=${caipAddress}`,
-          );
-
-          if (pageParam?.after) {
-            url.searchParams.set('after', pageParam.after);
-          } else if (pageParam?.before) {
-            url.searchParams.set('before', pageParam.before);
-          }
-
-          const response = await fetch(url);
-
-          if (!response.ok) {
-            throw new Error(
-              `Query failed with status code: ${response.status}.`,
-            );
-          }
-
-          return response.json();
-        },
-        initialPageParam: null,
-        staleTime: 0,
+        staleTime: this.#activityStaleTime,
       },
       page,
     );

packages/money-account-api-data-service/src/money-account-api-data-service.test.ts

@@ -7,7 +7,11 @@ import type {
 } from '@metamask/messenger';
 import nock, { cleanAll as nockCleanAll } from 'nock';
 
-import { Env, MONEY_ACCOUNT_API_URL_MAP } from './constants.js';
+import {
+  DEFAULT_STALE_TIME_MS,
+  Env,
+  MONEY_ACCOUNT_API_URL_MAP,
+} from './constants.js';
 import { MoneyAccountApiResponseValidationError } from './errors.js';
 import type {
   MoneyAccountApiDataServiceMessenger,
@@ -645,6 +649,50 @@ describe('MoneyAccountApiDataService', () => {
       service.destroy();
     });
 
+    it('refetches all cached pages when the query is stale', async () => {
+      const { service } = createService(Env.DEV);
+
+      const page2Response = {
+        ...MOCK_HISTORY_RESPONSE,
+        next_cursor: null,
+        has_more: false,
+      };
+
+      // Build a two-page cache.
+      nock(MONEY_ACCOUNT_API_URL_MAP[Env.DEV])
+        .get(`/v1/positions/${MOCK_ADDRESS}/history`)
+        .reply(200, MOCK_HISTORY_RESPONSE);
+      await service.fetchHistory(MOCK_ADDRESS);
+
+      nock(MONEY_ACCOUNT_API_URL_MAP[Env.DEV])
+        .get(`/v1/positions/${MOCK_ADDRESS}/history`)
+        .query({ cursor: 'eyJiIjoxMDAwMH0=' })
+        .reply(200, page2Response);
+      await service.fetchHistory(MOCK_ADDRESS, { cursor: 'eyJiIjoxMDAwMH0=' });
+
+      // Make the cache stale, then refetch both pages. The rebuild walks
+      // `getNextPageParam` to re-derive the second page's cursor from the first.
+      const now = Date.now();
+      const nowSpy = jest
+        .spyOn(Date, 'now')
+        .mockReturnValue(now + DEFAULT_STALE_TIME_MS + 1);
+
+      nock(MONEY_ACCOUNT_API_URL_MAP[Env.DEV])
+        .get(`/v1/positions/${MOCK_ADDRESS}/history`)
+        .reply(200, MOCK_HISTORY_RESPONSE);
+      nock(MONEY_ACCOUNT_API_URL_MAP[Env.DEV])
+        .get(`/v1/positions/${MOCK_ADDRESS}/history`)
+        .query({ cursor: 'eyJiIjoxMDAwMH0=' })
+        .reply(200, page2Response);
+
+      const refetched = await service.fetchHistory(MOCK_ADDRESS);
+
+      expect(refetched.cash_flows).toHaveLength(1);
+
+      nowSpy.mockRestore();
+      service.destroy();
+    });
+
     it('throws HttpError on non-2xx response', async () => {
       const { service } = createService(Env.DEV);
 

packages/money-account-api-data-service/src/money-account-api-data-service.ts

@@ -436,6 +436,8 @@ export class MoneyAccountApiDataService extends BaseDataService<
             },
           );
         },
+        initialPageParam: null,
+        getNextPageParam: (lastPage) => lastPage.next_cursor,
       },
       options?.cursor ?? undefined,
     );

packages/react-data-query/src/createUIQueryClient.test.ts

@@ -403,14 +403,14 @@ describe('createUIQueryClient', () => {
 
     const observerA = new InfiniteQueryObserver(clientA, {
       queryKey: getActivityQueryKey,
-      initialPageParam: undefined,
+      initialPageParam: null,
       getNextPageParam,
       getPreviousPageParam,
     });
 
     const observerB = new InfiniteQueryObserver(clientB, {
       queryKey: getActivityQueryKey,
-      initialPageParam: undefined,
+      initialPageParam: null,
       getNextPageParam,
       getPreviousPageParam,
     });