fix(base-data-service): adapt to @tanstack/query-core v5 API

packages/base-data-service/CHANGELOG.md

@@ -38,6 +38,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - **BREAKING:** Remove `TPageData` type parameter from `invalidateQueries` method ([#9526](https://github.com/MetaMask/core/pull/9526))
   - 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.
 - 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/package.json

@@ -59,7 +59,7 @@
     "@metamask/messenger": "^2.0.0",
     "@metamask/storage-service": "^1.0.2",
     "@metamask/utils": "^11.11.0",
-    "@tanstack/query-core": "^4.43.0",
+    "@tanstack/query-core": "^5.62.16",
     "cockatiel": "^3.1.2",
     "fast-deep-equal": "^3.1.3",
     "lodash": "^4.17.21"

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

@@ -1,5 +1,5 @@
 import { MOCK_ANY_NAMESPACE, Messenger } from '@metamask/messenger';
-import { hashQueryKey } from '@tanstack/query-core';
+import { hashKey } from '@tanstack/query-core';
 import { BrokenCircuitError } from 'cockatiel';
 import { cleanAll } from 'nock';
 
@@ -121,6 +121,66 @@ describe('BaseDataService', () => {
     expect(page2.data).not.toStrictEqual(page3.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.getActivityWithoutCallbacks(TEST_ADDRESS);
+
+    expect(page1.data).toHaveLength(3);
+
+    const page2 = await service.getActivityWithoutCallbacks(TEST_ADDRESS, {
+      page: { 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.getActivityWithoutCallbacks(TEST_ADDRESS);
+    await service.getActivityWithoutCallbacks(TEST_ADDRESS, {
+      page: { 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.getActivityWithoutCallbacks(TEST_ADDRESS);
+
+    expect(rebuilt.data).toHaveLength(3);
+  });
+
+  it('preserves a `null` initial page param', async () => {
+    const messenger = new Messenger({ namespace: serviceName });
+    const service = new ExampleDataService(messenger);
+
+    await service.getActivityWithoutCallbacks(TEST_ADDRESS, {
+      initialPageParam: null,
+    });
+
+    // `null` is a valid page param, so it must reach the query function rather
+    // than being coerced to `undefined`.
+    expect(service.pageParamsSeen).toStrictEqual([null]);
+  });
+
+  it('does not refetch a fresh paginated query', async () => {
+    const messenger = new Messenger({ namespace: serviceName });
+    const service = new ExampleDataService(messenger);
+
+    // Only one page-1 response is mocked, so a second fetch would fail. A fresh
+    // cached query must be served without another request.
+    const page1 = await service.getActivity(TEST_ADDRESS);
+    const pageAgain = await service.getActivity(TEST_ADDRESS);
+
+    expect(pageAgain.data).toStrictEqual(page1.data);
+  });
+
   it('emits `:cacheUpdated` events when cache is updated', async () => {
     const messenger = new Messenger({ namespace: serviceName });
     const service = new ExampleDataService(messenger);
@@ -131,7 +191,7 @@ describe('BaseDataService', () => {
 
     const queryKey = ['ExampleDataService:getAssets', MOCK_ASSETS];
 
-    const hash = hashQueryKey(queryKey);
+    const hash = hashKey(queryKey);
 
     expect(publishSpy).toHaveBeenNthCalledWith(
       6,
@@ -186,7 +246,7 @@ describe('BaseDataService', () => {
 
     const queryKey = ['ExampleDataService:getAssets', MOCK_ASSETS];
 
-    const hash = hashQueryKey(queryKey);
+    const hash = hashKey(queryKey);
 
     expect(publishSpy).toHaveBeenNthCalledWith(
       8,
@@ -333,6 +393,7 @@ describe('BaseDataService', () => {
         state: {
           queries: [
             {
+              dehydratedAt: expect.any(Number),
               queryHash:
                 '["ExampleDataService:getAssets",["eip155:1/slip44:60","bip122:000000000019d6689c085ae165831e93/slip44:0","eip155:1/erc20:0x6b175474e89094c44da98b954eedeac495271d0f"]]',
               queryKey: [

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

@@ -15,12 +15,15 @@ import {
   DehydratedState,
   FetchInfiniteQueryOptions,
   FetchQueryOptions,
+  GetNextPageParamFunction,
+  GetPreviousPageParamFunction,
   InfiniteData,
   InvalidateOptions,
   InvalidateQueryFilters,
   OmitKeyof,
   QueryClient,
   QueryClientConfig,
+  SkipToken,
   WithRequired,
   dehydrate,
   hydrate,
@@ -37,6 +40,22 @@ import {
 // Data service queries use the following format: ['ServiceActionName', ...params]
 export type QueryKey = [string, ...Json[]];
 
+/**
+ * The supertype of all messengers, scoped to a namespace.
+ *
+ * @template Namespace - The namespace for the messenger's own actions and
+ * events.
+ */
+export type BaseMessenger<Namespace extends string> = Messenger<
+  Namespace,
+  ActionConstraint,
+  EventConstraint,
+  // Use `any` to allow any parent to be set. `any` is harmless in a type constraint anyway,
+  // it's the one totally safe place to use it.
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  any
+>;
+
 export type DataServiceGranularCacheUpdatedPayload =
   | { type: 'added' | 'updated'; state: DehydratedState }
   | {
@@ -53,10 +72,10 @@ type CacheUpdatedType = DataServiceCacheUpdatedPayload['type'];
 
 export type DataServiceInvalidateQueriesAction<ServiceName extends string> = {
   type: `${ServiceName}:invalidateQueries`;
-  handler: (
-    filters?: InvalidateQueryFilters<Json>,
-    options?: InvalidateOptions,
-  ) => Promise<void>;
+  handler: BaseDataService<
+    ServiceName,
+    BaseMessenger<ServiceName>
+  >['invalidateQueries'];
 };
 
 export type DataServiceActions<ServiceName extends string> =
@@ -118,15 +137,7 @@ type PersistedCache = {
 
 export class BaseDataService<
   ServiceName extends string,
-  ServiceMessenger extends Messenger<
-    ServiceName,
-    ActionConstraint,
-    EventConstraint,
-    // Use `any` to allow any parent to be set. `any` is harmless in a type constraint anyway,
-    // it's the one totally safe place to use it.
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    any
-  >,
+  ServiceMessenger extends BaseMessenger<ServiceName>,
 > {
   public readonly name: ServiceName;
 
@@ -238,8 +249,13 @@ export class BaseDataService<
   /**
    * Fetch a query.
    *
-   * @param options - The options defining the query. Keep in mind that `queryKey` and `queryFn` are required when using data services.
-   * Additionally `retry` and `retryDelay` are not available, retries can be customized using the `servicePolicyOptions`.
+   * @param options - The options defining the query. Note that although this
+   * method wraps `fetchQuery` from `@tanstack/query-core`, there are a few
+   * restrictions:
+   * - `queryKey` and `queryFn` are required
+   * - `queryFn` must be a function, not a skip token
+   * - `retry` and `retryDelay` are not available (retries can be customized
+   *   using the constructor's `servicePolicyOptions`).
    * @returns The query results.
    */
   protected async fetchQuery<
@@ -251,10 +267,19 @@ export class BaseDataService<
     options: WithRequired<
       OmitKeyof<
         FetchQueryOptions<TQueryFnData, TError, TData, TQueryKey>,
-        'retry' | 'retryDelay'
+        'retry' | 'retryDelay' | 'queryFn'
       >,
-      'queryKey' | 'queryFn'
-    >,
+      'queryKey'
+    > & {
+      // @tanstack/query-core's fetchQuery function accepts a "skip" token,
+      // but data services always provide a concrete query function.
+      queryFn: NonNullable<
+        Exclude<
+          FetchQueryOptions<TQueryFnData, TError, TData, TQueryKey>['queryFn'],
+          SkipToken
+        >
+      >;
+    },
   ): Promise<TData> {
     return this.#queryClient.fetchQuery({
       ...options,
@@ -266,10 +291,17 @@ export class BaseDataService<
   /**
    * Fetch a paginated query.
    *
-   * @param options - The options defining the query. Keep in mind that `queryKey` and `queryFn` are required when using data services.
-   * Additionally `retry` and `retryDelay` are not available, retries can be customized using the `servicePolicyOptions`.
+   * @param options - The options defining the query. Note that although this
+   * method wraps `fetchInfiniteQuery` from `@tanstack/query-core`, there are a
+   * few differences:
+   * - `queryKey` and `queryFn` are required
+   * - `queryFn` must be a function, not a skip token
+   * - `retry` and `retryDelay` are not available (retries can be customized
+   *   using the constructor's `servicePolicyOptions`).
    * @param pageParam - An optional page parameter.
-   * @returns The query result, exclusively the requested page is returned.
+   * @returns A page's worth of data (i.e. what `queryFn` returns). Note that
+   * this is different from `@tanstack/query-core`'s `fetchInfiniteQuery`
+   * method, which returns all pages.
    */
   protected async fetchInfiniteQuery<
     TQueryFnData extends Json,
@@ -280,47 +312,117 @@ export class BaseDataService<
   >(
     options: WithRequired<
       OmitKeyof<
-        FetchInfiniteQueryOptions<TQueryFnData, TError, TData, TQueryKey>,
-        'retry' | 'retryDelay'
+        FetchQueryOptions<
+          TQueryFnData,
+          TError,
+          InfiniteData<TData, TPageParam>,
+          TQueryKey,
+          TPageParam
+        >,
+        'retry' | 'retryDelay' | 'queryFn' | 'initialPageParam'
       >,
-      'queryKey' | 'queryFn'
-    >,
+      'queryKey'
+    > & {
+      // @tanstack/query-core's fetchInfiniteQuery function accepts a "skip"
+      // token, but data services always provide a concrete query function.
+      queryFn: NonNullable<
+        Exclude<
+          FetchInfiniteQueryOptions<
+            TQueryFnData,
+            TError,
+            TData,
+            TQueryKey,
+            TPageParam
+          >['queryFn'],
+          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>;
+      getPreviousPageParam?: GetPreviousPageParamFunction<
+        TPageParam,
+        TQueryFnData
+      >;
+    },
     pageParam?: TPageParam,
   ): Promise<TData> {
     const cache = this.#queryClient.getQueryCache();
 
-    const query = cache.find<TQueryFnData, TError, InfiniteData<TData>>({
+    const query = cache.find<
+      TQueryFnData,
+      TError,
+      InfiniteData<TData, TPageParam>
+    >({
       queryKey: options.queryKey,
     });
 
     if (!query?.state.data || pageParam === undefined) {
-      const result = await this.#queryClient.fetchInfiniteQuery({
+      // @tanstack/query-core requires an `initialPageParam`, which becomes the
+      // param of the first (and only) page this fetches. Prefer an explicit
+      // per-call `pageParam` (a cold jump to a specific page); otherwise use the
+      // consumer's `initialPageParam`. Branching on a strict `undefined` check
+      // (rather than `??`) preserves `null`, which is a valid `Json` page param
+      // and query-core's usual first-page sentinel. The value can legitimately
+      // be `undefined` here (the very first page), which query-core accepts at
+      // runtime but not in its `TPageParam` type, hence the cast.
+      let initialPageParam: TPageParam;
+      if (pageParam === undefined) {
+        initialPageParam = options.initialPageParam as TPageParam;
+      } else {
+        initialPageParam = pageParam;
+      }
+
+      const result = await this.#queryClient.fetchInfiniteQuery<
+        TQueryFnData,
+        TError,
+        TData,
+        TQueryKey,
+        TPageParam
+      >({
         ...options,
+        initialPageParam,
+        // 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(() =>
-            options.queryFn({
-              ...context,
-              pageParam: context.pageParam ?? pageParam,
-            }),
-          ),
+          this.#policy.execute(() => options.queryFn(context)),
       });
 
       return result.pages[0];
     }
 
-    const { pages } = query.state.data;
-    const previous = options.getPreviousPageParam?.(pages[0], pages);
+    const { pages, pageParams } = query.state.data;
+    const previous = options.getPreviousPageParam?.(
+      pages[0],
+      pages,
+      pageParams[0],
+      pageParams,
+    );
 
     const direction = deepEqual(pageParam, previous) ? 'backward' : 'forward';
 
-    const result = await query.fetch(undefined, {
-      meta: {
-        fetchMore: {
-          direction,
-          pageParam,
+    // Override the next/previous param callbacks to return exactly the
+    // requested page, so pagination works even when the consumer did not
+    // provide page-param callbacks.
+    const result = await query.fetch(
+      {
+        ...query.options,
+        getNextPageParam: () => pageParam,
+        getPreviousPageParam: () => pageParam,
+      } as typeof query.options,
+      {
+        meta: {
+          fetchMore: {
+            direction,
+          },
         },
       },
-    });
+    );
 
     const pageIndex = result.pageParams.findIndex((param) =>
       deepEqual(param, pageParam),
@@ -337,7 +439,7 @@ export class BaseDataService<
    * @returns Nothing.
    */
   async invalidateQueries(
-    filters?: InvalidateQueryFilters<Json>,
+    filters?: InvalidateQueryFilters<Json[]>,
     options?: InvalidateOptions,
   ): Promise<void> {
     return this.#queryClient.invalidateQueries(filters, options);