From 4c9bb9ab84b54f227bca9e363e0543cbd1942196 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Wed, 29 Jul 2026 09:25:58 -0600 Subject: [PATCH 1/2] WIP - Bump query-core in base-data-service --- packages/base-data-service/package.json | 2 +- .../src/BaseDataService.test.ts | 4 +- .../base-data-service/src/BaseDataService.ts | 135 +++++++++++++----- packages/messenger/src/Messenger.ts | 16 +++ packages/messenger/src/index.ts | 1 + yarn.config.cjs | 4 +- yarn.lock | 2 +- 7 files changed, 119 insertions(+), 45 deletions(-) diff --git a/packages/base-data-service/package.json b/packages/base-data-service/package.json index 32a4ca2d3a7..0ba3f0fb40c 100644 --- a/packages/base-data-service/package.json +++ b/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" diff --git a/packages/base-data-service/src/BaseDataService.test.ts b/packages/base-data-service/src/BaseDataService.test.ts index df6ed594d36..a237f8d11b8 100644 --- a/packages/base-data-service/src/BaseDataService.test.ts +++ b/packages/base-data-service/src/BaseDataService.test.ts @@ -67,12 +67,14 @@ describe('BaseDataService', () => { ]); }); - it('handles paginated queries', async () => { + it.only('handles paginated queries', async () => { const messenger = new Messenger({ namespace: serviceName }); const service = new ExampleDataService(messenger); const page1 = await service.getActivity(TEST_ADDRESS); + console.dir(page1, { depth: null }); + expect(page1.data).toHaveLength(3); const page2 = await service.getActivity(TEST_ADDRESS, { diff --git a/packages/base-data-service/src/BaseDataService.ts b/packages/base-data-service/src/BaseDataService.ts index 7ac764fcad5..28d9702df08 100644 --- a/packages/base-data-service/src/BaseDataService.ts +++ b/packages/base-data-service/src/BaseDataService.ts @@ -1,8 +1,4 @@ -import { - Messenger, - ActionConstraint, - EventConstraint, -} from '@metamask/messenger'; +import { Messenger, BaseMessenger } from '@metamask/messenger'; import type { StorageServiceGetItemAction, StorageServiceRemoveItemAction, @@ -11,16 +7,20 @@ import type { import { Duration, inMilliseconds } from '@metamask/utils'; import type { Json } from '@metamask/utils'; import { + DefaultError, DefaultOptions, DehydratedState, FetchInfiniteQueryOptions, FetchQueryOptions, + GetNextPageParamFunction, InfiniteData, InvalidateOptions, InvalidateQueryFilters, OmitKeyof, QueryClient, QueryClientConfig, + QueryFunction, + SkipToken, WithRequired, dehydrate, hydrate, @@ -53,10 +53,10 @@ type CacheUpdatedType = DataServiceCacheUpdatedPayload['type']; export type DataServiceInvalidateQueriesAction = { type: `${ServiceName}:invalidateQueries`; - handler: ( - filters?: InvalidateQueryFilters, - options?: InvalidateOptions, - ) => Promise; + handler: BaseDataService< + ServiceName, + BaseMessenger + >['invalidateQueries']; }; type DataServiceActions = @@ -118,15 +118,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, > { public readonly name: ServiceName; @@ -238,23 +230,42 @@ 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< TQueryFnData extends Json, - TError = unknown, + TError = DefaultError, TData = TQueryFnData, TQueryKey extends QueryKey = QueryKey, + TPageParam extends Json = Json, >( options: WithRequired< OmitKeyof< - FetchQueryOptions, - 'retry' | 'retryDelay' + FetchQueryOptions, + 'retry' | 'retryDelay' | 'queryFn' >, - 'queryKey' | 'queryFn' - >, + 'queryKey' + > & { + queryFn: NonNullable< + Exclude< + FetchQueryOptions< + TQueryFnData, + TError, + TData, + TQueryKey, + TPageParam + >['queryFn'], + SkipToken + > + >; + }, ): Promise { return this.#queryClient.fetchQuery({ ...options, @@ -266,27 +277,74 @@ 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 `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`). * @param pageParam - An optional page parameter. - * @returns The query result, exclusively the requested page is returned. + * @returns The query result (the requested page). */ protected async fetchInfiniteQuery< TQueryFnData extends Json, - TError = unknown, - TData extends TQueryFnData = TQueryFnData, + TError = DefaultError, + TData = TQueryFnData, TQueryKey extends QueryKey = QueryKey, TPageParam extends Json = Json, >( options: WithRequired< OmitKeyof< - FetchInfiniteQueryOptions, - 'retry' | 'retryDelay' + FetchInfiniteQueryOptions< + TQueryFnData, + TError, + TData, + TQueryKey, + TPageParam + >, + 'retry' | 'retryDelay' | 'queryFn' >, - 'queryKey' | 'queryFn' - >, + 'queryKey' + > & { + queryFn: NonNullable< + Exclude< + FetchInfiniteQueryOptions< + TQueryFnData, + TError, + TData, + TQueryKey, + TPageParam + >['queryFn'], + SkipToken + > + >; + } & ( + | { + pages?: never; + } + | { + pages: number; + getNextPageParam: GetNextPageParamFunction< + TPageParam, + TQueryFnData + >; + } + ), pageParam?: TPageParam, - ): Promise { + ): Promise> { + return await this.#queryClient.fetchInfiniteQuery({ + ...options, + queryFn: (context) => + this.#policy.execute(() => + options.queryFn({ + ...context, + pageParam: context.pageParam ?? pageParam, + }), + ), + }); + + /* const cache = this.#queryClient.getQueryCache(); const query = cache.find>({ @@ -294,7 +352,7 @@ export class BaseDataService< }); if (!query?.state.data || pageParam === undefined) { - const result = await this.#queryClient.fetchInfiniteQuery({ + return await this.#queryClient.fetchInfiniteQuery({ ...options, queryFn: (context) => this.#policy.execute(() => @@ -304,8 +362,6 @@ export class BaseDataService< }), ), }); - - return result.pages[0]; } const { pages } = query.state.data; @@ -327,6 +383,7 @@ export class BaseDataService< ); return result.pages[pageIndex]; + */ } /** @@ -337,7 +394,7 @@ export class BaseDataService< * @returns Nothing. */ async invalidateQueries( - filters?: InvalidateQueryFilters, + filters?: InvalidateQueryFilters, options?: InvalidateOptions, ): Promise { return this.#queryClient.invalidateQueries(filters, options); diff --git a/packages/messenger/src/Messenger.ts b/packages/messenger/src/Messenger.ts index ba63f9ab405..07a40a621b4 100644 --- a/packages/messenger/src/Messenger.ts +++ b/packages/messenger/src/Messenger.ts @@ -224,6 +224,22 @@ type DelegatedMessenger = Pick< type StripNamespace = Namespaced extends `${string}:${infer Name}` ? Name : never; +/** + * The supertype of all messengers, scoped to a namespace. + * + * @template Namespace - The namespace for the messenger's own actions and + * events. + */ +export type BaseMessenger = 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 +>; + /** * A message broker for "actions" and "events". * diff --git a/packages/messenger/src/index.ts b/packages/messenger/src/index.ts index f7d78fca57a..1e2df995001 100644 --- a/packages/messenger/src/index.ts +++ b/packages/messenger/src/index.ts @@ -1,5 +1,6 @@ export type { ActionHandler, + BaseMessenger, ExtractActionParameters, ExtractActionResponse, ExtractEventHandler, diff --git a/yarn.config.cjs b/yarn.config.cjs index 724034ff43a..1e6814bc09c 100644 --- a/yarn.config.cjs +++ b/yarn.config.cjs @@ -23,9 +23,7 @@ const { inspect } = require('util'); * Only intended as temporary measures to faciliate upgrades and releases. * This should trend towards empty. */ -const ALLOWED_INCONSISTENT_DEPENDENCIES = { - '@tanstack/query-core': ['^4.43.0'], -}; +const ALLOWED_INCONSISTENT_DEPENDENCIES = {}; /** * These packages are allowed as peer dependencies without requiring installation as diff --git a/yarn.lock b/yarn.lock index 9a56be86c58..62f806b427e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6149,7 +6149,7 @@ __metadata: "@metamask/messenger": "npm:^2.0.0" "@metamask/storage-service": "npm:^1.0.2" "@metamask/utils": "npm:^11.11.0" - "@tanstack/query-core": "npm:^4.43.0" + "@tanstack/query-core": "npm:^5.62.16" "@ts-bridge/cli": "npm:^0.6.4" "@types/jest": "npm:^30.0.0" "@types/lodash": "npm:^4.14.191" From 155eacd9669eae7086750d446907a30067c971c4 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Fri, 31 Jul 2026 14:34:53 -0600 Subject: [PATCH 2/2] Get types working? --- .../src/BaseDataService.test.ts | 11 ++--- .../base-data-service/src/BaseDataService.ts | 49 ++++++++++--------- .../tests/ExampleDataService.ts | 32 +++++++++--- 3 files changed, 55 insertions(+), 37 deletions(-) diff --git a/packages/base-data-service/src/BaseDataService.test.ts b/packages/base-data-service/src/BaseDataService.test.ts index a237f8d11b8..152eb6bd816 100644 --- a/packages/base-data-service/src/BaseDataService.test.ts +++ b/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'; @@ -67,14 +67,12 @@ describe('BaseDataService', () => { ]); }); - it.only('handles paginated queries', async () => { + it('handles paginated queries', async () => { const messenger = new Messenger({ namespace: serviceName }); const service = new ExampleDataService(messenger); const page1 = await service.getActivity(TEST_ADDRESS); - console.dir(page1, { depth: null }); - expect(page1.data).toHaveLength(3); const page2 = await service.getActivity(TEST_ADDRESS, { @@ -133,7 +131,7 @@ describe('BaseDataService', () => { const queryKey = ['ExampleDataService:getAssets', MOCK_ASSETS]; - const hash = hashQueryKey(queryKey); + const hash = hashKey(queryKey); expect(publishSpy).toHaveBeenNthCalledWith( 6, @@ -188,7 +186,7 @@ describe('BaseDataService', () => { const queryKey = ['ExampleDataService:getAssets', MOCK_ASSETS]; - const hash = hashQueryKey(queryKey); + const hash = hashKey(queryKey); expect(publishSpy).toHaveBeenNthCalledWith( 8, @@ -335,6 +333,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: [ diff --git a/packages/base-data-service/src/BaseDataService.ts b/packages/base-data-service/src/BaseDataService.ts index 28d9702df08..105952e3f14 100644 --- a/packages/base-data-service/src/BaseDataService.ts +++ b/packages/base-data-service/src/BaseDataService.ts @@ -14,6 +14,7 @@ import { FetchQueryOptions, GetNextPageParamFunction, InfiniteData, + InfiniteQueryPageParamsOptions, InvalidateOptions, InvalidateQueryFilters, OmitKeyof, @@ -278,14 +279,16 @@ export class BaseDataService< * Fetch a paginated query. * * @param options - The options defining the query. Note that although this - * method wraps `fetchQuery` from `@tanstack/query-core`, there are a few - * restrictions: + * 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`). + * - This function returns a page's worth of data (the same thing that + * `queryFn` returns), not a list of pages * @param pageParam - An optional page parameter. - * @returns The query result (the requested page). + * @returns The requested page. */ protected async fetchInfiniteQuery< TQueryFnData extends Json, @@ -330,29 +333,23 @@ export class BaseDataService< TQueryFnData >; } - ), + ) & + InfiniteQueryPageParamsOptions, pageParam?: TPageParam, - ): Promise> { - return await this.#queryClient.fetchInfiniteQuery({ - ...options, - queryFn: (context) => - this.#policy.execute(() => - options.queryFn({ - ...context, - pageParam: context.pageParam ?? pageParam, - }), - ), - }); - - /* + // ): Promise | TQueryFnData | TData> { + ): Promise { const cache = this.#queryClient.getQueryCache(); - const query = cache.find>({ + const query = cache.find< + TQueryFnData, + TError, + InfiniteData + >({ queryKey: options.queryKey, }); if (!query?.state.data || pageParam === undefined) { - return await this.#queryClient.fetchInfiniteQuery({ + const result = await this.#queryClient.fetchInfiniteQuery({ ...options, queryFn: (context) => this.#policy.execute(() => @@ -362,10 +359,18 @@ export class BaseDataService< }), ), }); + // We have to assume that `fetchInfiniteQuery` returns the same data + // that `queryFn` returns. + return result.pages[0] as unknown as TQueryFnData; } - 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'; @@ -373,7 +378,6 @@ export class BaseDataService< meta: { fetchMore: { direction, - pageParam, }, }, }); @@ -383,7 +387,6 @@ export class BaseDataService< ); return result.pages[pageIndex]; - */ } /** diff --git a/packages/base-data-service/tests/ExampleDataService.ts b/packages/base-data-service/tests/ExampleDataService.ts index d84e9edf5d1..67eb5a9ae39 100644 --- a/packages/base-data-service/tests/ExampleDataService.ts +++ b/packages/base-data-service/tests/ExampleDataService.ts @@ -1,5 +1,6 @@ import { Messenger } from '@metamask/messenger'; import { CaipAssetId, Duration, inMilliseconds, Json } from '@metamask/utils'; +import { DefaultError } from '@tanstack/query-core'; import { ConstantBackoff } from 'cockatiel'; import { @@ -8,6 +9,7 @@ import { DataServiceCacheUpdatedEvent, DataServiceGranularCacheUpdatedEvent, PersistenceConfiguration, + QueryKey, } from '../src/BaseDataService.js'; import { ExampleDataServiceMethodActions } from './ExampleDataService-method-action-types.js'; @@ -49,7 +51,8 @@ export type PageParam = | { before: string; } - | { after: string }; + | { after: string } + | null; const MESSENGER_EXPOSED_METHODS = ['getAssets', 'getActivity'] as const; @@ -101,15 +104,21 @@ export class ExampleDataService extends BaseDataService< return response.json(); }, staleTime: inMilliseconds(1, Duration.Day), - cacheTime: inMilliseconds(1, Duration.Day), + gcTime: inMilliseconds(1, Duration.Day), }); } async getActivity( address: string, - page?: PageParam, + page: PageParam = null, ): Promise { - return this.fetchInfiniteQuery( + return this.fetchInfiniteQuery< + GetActivityResponse, + DefaultError, + GetActivityResponse, + QueryKey, + PageParam + >( { queryKey: [`${this.name}:getActivity`, address], queryFn: async ({ pageParam }) => { @@ -118,10 +127,16 @@ export class ExampleDataService extends BaseDataService< `${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); + if (pageParam !== null) { + // We need to discriminate this union. + // eslint-disable-next-line no-restricted-syntax + if ('after' in pageParam) { + url.searchParams.set('after', pageParam.after); + // We need to discriminate this union. + // eslint-disable-next-line no-restricted-syntax + } else if ('before' in pageParam) { + url.searchParams.set('before', pageParam.before); + } } const response = await fetch(url); @@ -134,6 +149,7 @@ export class ExampleDataService extends BaseDataService< return response.json(); }, + initialPageParam: null, getPreviousPageParam: ({ pageInfo }) => pageInfo.hasPreviousPage ? { before: pageInfo.startCursor }