fix(base-data-service): adapt to @tanstack/query-core v5 API - #9712
Open
cryptodev-2s wants to merge 14 commits into
Open
fix(base-data-service): adapt to @tanstack/query-core v5 API#9712cryptodev-2s wants to merge 14 commits into
@tanstack/query-core v5 API#9712cryptodev-2s wants to merge 14 commits into
Conversation
cryptodev-2s
changed the base branch from
update-tanstack-query-core-v5
to
main
August 3, 2026 21:46
cryptodev-2s
force-pushed
the
fix/query-core-v5-base-data-service
branch
from
August 3, 2026 21:46
def5a29 to
88ec84f
Compare
cryptodev-2s
marked this pull request as ready for review
August 4, 2026 10:09
cryptodev-2s
force-pushed
the
fix/query-core-v5-base-data-service
branch
from
August 4, 2026 10:09
637de58 to
6dbe63d
Compare
Contributor
Author
|
@metamaskbot publish-preview |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
cryptodev-2s
added a commit
that referenced
this pull request
Aug 4, 2026
… param callbacks query-core v5 walks `getNextPageParam` when it refetches an infinite query that has more than one page cached. Consumers that paginate by explicit cursor without defining `getNextPageParam` (like `MoneyAccountApiDataService`) would throw once a second page was cached and a stale refetch with no page param ran. Default `getNextPageParam` to a resolver that returns `null` so the refetch rebuilds just the first page instead of throwing. The consumer repopulates the cache by navigating again with explicit page params. Also adds a `fetchInfiniteQuery` test suite covering forward and backward navigation with and without page param callbacks, single page returns, `staleTime` deduplication, and this refetch regression. Flagged by Cursor Bugbot on #9712.
A recent PR updated this package to v5 in one package (#9563). This PR updates to v5 in all remaining packages.
The v5 bump updated the package.json versions but left the v4 API usage in place, so the build fails. This adapts the code to v5. * `invalidateQueries` filter: the generic is now the query key (not page data), so drop the `Json` argument * handle the new `skipToken` sentinel by typing `queryFn` as a concrete function (data services never use it) * `fetchInfiniteQuery`: pass `initialPageParam` and inject page param resolvers at fetch time, since v5 no longer accepts an explicit `pageParam` via `fetchMore` meta. This keeps cursor pagination working for consumers that do not define page param callbacks * chomp: `cacheTime` is now `gcTime` * tests: `hashQueryKey` is now `hashKey`, and `dehydrate` adds a `dehydratedAt` field
… param callbacks query-core v5 walks `getNextPageParam` when it refetches an infinite query that has more than one page cached. Consumers that paginate by explicit cursor without defining `getNextPageParam` (like `MoneyAccountApiDataService`) would throw once a second page was cached and a stale refetch with no page param ran. Default `getNextPageParam` to a resolver that returns `null` so the refetch rebuilds just the first page instead of throwing. The consumer repopulates the cache by navigating again with explicit page params. Also adds a `fetchInfiniteQuery` test suite covering forward and backward navigation with and without page param callbacks, single page returns, `staleTime` deduplication, and this refetch regression. Flagged by Cursor Bugbot on #9712.
cryptodev-2s
force-pushed
the
fix/query-core-v5-base-data-service
branch
from
August 4, 2026 13:13
4561b46 to
5a1dcce
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5a1dcce. Configure here.
Contributor
Author
|
@metamaskbot publish-preview |
4 tasks
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
mcmire
reviewed
Aug 10, 2026
Comment on lines
+372
to
+375
| // query-core v5 no longer accepts an explicit page param via the `fetchMore` | ||
| // meta; it derives the next/previous param from these callbacks instead. | ||
| // Override them to return exactly the requested page so pagination works | ||
| // even when the consumer did not provide page-param callbacks. |
Collaborator
There was a problem hiding this comment.
Nit: Saying why we're not doing something could be confusing to people who don't have context. What if we simply say:
Suggested change
| // query-core v5 no longer accepts an explicit page param via the `fetchMore` | |
| // meta; it derives the next/previous param from these callbacks instead. | |
| // Override them to return exactly the requested page so pagination works | |
| // even when the consumer did not provide page-param callbacks. | |
| // 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. |
| // and query-core's usual first-page sentinel. | ||
| let initialPageParam: TPageParam; | ||
| if (pageParam === undefined) { | ||
| initialPageParam = options.initialPageParam as TPageParam; |
Collaborator
There was a problem hiding this comment.
Not requiring initialPageParam is fine — our version of fetchInfiniteQuery is intentionally different from TanStack Query's version — but is there a way to not use a typecast here? Basically we are allowing the user to specify a initialPageParam that doesn't match TPageParam which seems odd. It seems that we need a variant of TPageParam that allows undefined.
| > & { | ||
| // Data services always provide a concrete query function; the `skipToken` | ||
| // sentinel added in query-core v5 is not supported here. | ||
| queryFn: QueryFunction<TQueryFnData, TQueryKey>; |
Collaborator
There was a problem hiding this comment.
Nit: Would the following be more future-proof?
Suggested change
| queryFn: QueryFunction<TQueryFnData, TQueryKey>; | |
| queryFn: NonNullable< | |
| Exclude< | |
| FetchQueryOptions< | |
| TQueryFnData, | |
| TError, | |
| TData, | |
| TQueryKey, | |
| TPageParam | |
| >['queryFn'], | |
| SkipToken | |
| > | |
| >; |
| > & { | ||
| // Data services always provide a concrete query function; the `skipToken` | ||
| // sentinel added in query-core v5 is not supported here. | ||
| queryFn: QueryFunction<TQueryFnData, TQueryKey, TPageParam>; |
Collaborator
There was a problem hiding this comment.
Nit: Would the following be more future-proof?
Suggested change
| queryFn: QueryFunction<TQueryFnData, TQueryKey, TPageParam>; | |
| queryFn: NonNullable< | |
| Exclude< | |
| FetchInfiniteQueryOptions< | |
| TQueryFnData, | |
| TError, | |
| TData, | |
| TQueryKey, | |
| TPageParam | |
| >['queryFn'], | |
| SkipToken | |
| > | |
| >; |
Comment on lines
+303
to
+305
| // These are required by query-core v5 for infinite queries but remain | ||
| // optional here: consumers may drive pagination purely by passing an | ||
| // explicit `pageParam` (see below). |
Collaborator
There was a problem hiding this comment.
Nit: Maybe we can drop mention of v5 so this comment doesn't become stale?
Suggested change
| // These are required by query-core v5 for infinite queries but remain | |
| // optional here: consumers may drive pagination purely by passing an | |
| // explicit `pageParam` (see below). | |
| // 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). |
| */ | ||
| async invalidateQueries( | ||
| filters?: InvalidateQueryFilters<Json>, | ||
| filters?: InvalidateQueryFilters, |
Collaborator
There was a problem hiding this comment.
By default the type parameter to InvalidateQueryFilters is unknown. Do we want that? I think we were trying to keep this method JSON-compatible, so perhaps we should say:
Suggested change
| filters?: InvalidateQueryFilters, | |
| filters?: InvalidateQueryFilters<Json[]>, |
| }); | ||
| }); | ||
|
|
||
| describe('fetchInfiniteQuery', () => { |
Collaborator
There was a problem hiding this comment.
While structuring these tests using a describe block definitely matches our guidelines elsewhere, it does not follow the existing conventions of this file. We already have tests for fetchInfiniteQuery: see "handles paginated queries", "handles paginated queries starting at a specific page", etc. We also already have a method that makes use of fetchInfiniteQuery internally in ExampleDataService: see getActivity. Perhaps we can adapt or extend those tests and also ExampleDataService?
Comment on lines
243
to
244
Collaborator
There was a problem hiding this comment.
What do you think about updating this JSDoc?
Suggested change
| * @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`). |
Comment on lines
275
to
276
Collaborator
There was a problem hiding this comment.
What do you think about updating this JSDoc?
Suggested change
| * @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`). |
Collaborator
There was a problem hiding this comment.
Nit: Thoughts on tweaking this while we're at it?
Suggested change
| * @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. |
…nd docs * derive the `queryFn` option types from `@tanstack/query-core`'s own option types (`NonNullable<Exclude<...['queryFn'], SkipToken>>`) so they track upstream * type the invalidate filter as `InvalidateQueryFilters<Json[]>` to keep it JSON shaped * base the `invalidateQueries` action handler type on `BaseDataService['invalidateQueries']` via a new `BaseMessenger` type so it cannot drift * reword the JSDoc and comments, dropping version specific mentions The `initialPageParam` cast stays: widening the page param generic to allow `undefined` just moves the cast onto `queryFn` and `getNextPageParam` (function param contravariance), which is worse.
…DataService Follow the file's existing conventions instead of a bespoke harness. Drop the `PaginatedService` and `withService` helpers and the nested describe block, and add flat tests built on `ExampleDataService.getActivity`. Extend `ExampleDataService` with `getActivityWithoutCallbacks` (drives pagination by explicit cursor with no page param callbacks, records the params its query function sees) to cover pagination without callbacks, stale refetch of a multi page query, and preserving a `null` initial page param. Also fixes `PageParam` so its properties are optional, which the query function already relied on.
…e-data-service # Conflicts: # packages/chomp-api-service/CHANGELOG.md # packages/money-account-api-data-service/CHANGELOG.md # packages/sentinel-api-service/CHANGELOG.md
`claims-controller`, `subscription-controller`, and `shield-controller` landed on main still using v4, which broke the monorepo version consistency constraint. Bump them to `^5.62.16`. * rename `cacheTime` to `gcTime` (v5 renamed the option) in `ShieldApiService` and `SubscriptionService` * rework a brittle latency test in `shield-controller` that depended on the exact number of `Date.now()` calls query-core makes internally, which v5 changed; it now keys off fetch progress instead
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Stacked on top of #9686. That PR bumps
@tanstack/query-corethis adapts the code to v5.Changes
packages/base-data-service/src/BaseDataService.tsinvalidateQueriesfilter type: in v5 the generic is the query key (constrained toreadonly unknown[]), not page data, soInvalidateQueryFilters<Json>no longer type checks. Dropped the argument to keep the loose v4 ergonomics.queryFncan now be theskipTokensentinel (aunique symbol, not callable). Typed the options soqueryFnis always a concrete function, since data services never useskipToken.fetchInfiniteQuery: v5 requiresinitialPageParam, andfetchMoremeta no longer carries an explicitpageParam(v5 derives it fromgetNextPageParam/getPreviousPageParam). Now passesinitialPageParamand injects page param resolvers atquery.fetchtime so pagination still fetches the exact requested page. This keeps cursor pagination working even for consumers that do not define page param callbacks.packages/chomp-api-service/src/chomp-api-service.tscacheTimewas renamed togcTimein v5. Without this, the "evict on settle" behavior stopped working.Tests / helpers
hashQueryKeywas renamed tohashKey.dehydratenow writes adehydratedAtfield into query state.cacheTimetogcTimeand typed the page param in the example service.Why the injection matters
MoneyAccountApiDataServicepaginates via an explicit cursor without defininggetNextPageParam. Under the raw v5 bump its pagination test crashed withoptions.getNextPageParam is not a function. The resolver injection restores that behavior.Note
Medium Risk
Breaking base-data-service API and query-cache/pagination semantics touch many controllers;
gcTimerenames fix regressions in “no cache” paths for auth-scoped API calls.Overview
Upgrades
@tanstack/query-corefrom v4 to v5 across data-service packages and alignsBaseDataServicewith the new query-core contracts.BaseDataServicenow typesfetchQuery/fetchInfiniteQueryfor v5 (concretequeryFn, noskipToken), fixesinvalidateQueriesfilters for v5’s query-key generic, and reworks infinite pagination: requiredinitialPageParam, nopageParamonfetchMoremeta, default no-opgetNextPageParamfor refetches, and temporarygetNextPageParam/getPreviousPageParamoverrides at fetch time so cursor-only consumers still load the requested page. Tests cover pagination without page-param callbacks,nullinitial params, and fresh-cache behavior; cache event hashing useshashKeyand persisted state expectsdehydratedAt.Downstream services rename
cacheTimetogcTime(Chomp associated addresses, Shield logging/init queries, Subscription#fetchJson) so immediate eviction still works. Example service and Shield coverage polling tests are updated for v5 clock/hash behavior; changelogs andyarn.config.cjsdrop the old v4 version exception.Reviewed by Cursor Bugbot for commit 6e8f2c9. Bugbot is set up for automated code reviews on this repo. Configure here.