Repository navigation
ref(api): Let apiFetch serve as a mutationFn - #127699
Merged
Merged
Conversation
apiFetch now accepts just {queryKey} rather than a full
QueryFunctionContext, so mutations can call it directly and resolve to
the same ApiResponse ({json, headers, status}) shape the query cache
uses. queryFn usage is unchanged, since a full context still satisfies
the narrower parameter; usePinnedLogsQuery built a context literal with
extra fields and now passes only the queryKey.
The mock API client now awaits a promise body when includeAllArgs is
set. apiFetch always sets it, so specs that hold a mutation in flight
with a promise body would otherwise receive the pending promise as json.
One preprod spec passed only because of that leak and now mocks a
resolved size analysis instead.
This was referenced Oct 15, 2026
github-actions
Bot
added
the
Scope: Frontend
This was referenced Oct 15, 2026
Narrowing the parameter to just queryKey meant apiFetch could never use other context fields, even though a mutation could supply them itself (for example its own AbortSignal). Keep queryKey required and accept the rest optionally, so queries pass the full context as before and mutations pass what they have. usePinnedLogsQuery can again pass its full context literal.
Mutations have no query context to forward, and the other QueryFunctionContext fields (signal, client, meta) describe a running query that a mutation would have to invent. Accept just the queryKey that both callers can supply. usePinnedLogsQuery built a context literal with extra fields and now passes only the queryKey.
Match apiFetch: destructure the two context fields the function reads instead of accepting the whole query context.
ryan953
commented
Oct 16, 2026
Comment on lines
+43
to
+48
| export async function apiFetch<TQueryFnData = unknown>({ | ||
| queryKey, | ||
| }: Pick<QueryFunctionContext<ApiQueryKey>, 'queryKey'>): Promise< | ||
| ApiResponse<TQueryFnData> | ||
| > { | ||
| const [url, options] = queryKey; |
Member
Author
There was a problem hiding this comment.
narrow the type sig so callsites don't need to pass extra fields that are ignored anyway.
ryan953
commented
Oct 16, 2026
Comment on lines
+68
to
+74
| queryKey, | ||
| pageParam, | ||
| }: Pick< | ||
| QueryFunctionContext<InfiniteApiQueryKey, null | undefined | ParsedHeader>, | ||
| 'queryKey' | 'pageParam' | ||
| >): Promise<ApiResponse<TQueryFnData>> { | ||
| const [url, options] = queryKey; |
Member
Author
There was a problem hiding this comment.
narrow the type sig so callsites don't need to pass extra fields that are ignored anyway.
ryan953
commented
Oct 16, 2026
Comment on lines
-80
to
-81
| signal: new AbortController().signal, | ||
| meta: undefined, |
Member
Author
There was a problem hiding this comment.
remove extra fields that were ignored anyway.
ryan953
commented
Oct 16, 2026
| MockApiClient.addMockResponse({ | ||
| url: SIZE_ANALYSIS_URL, | ||
| method: 'GET', | ||
| body: new Promise(() => {}), // Keep pending |
Member
Author
There was a problem hiding this comment.
The test before was broken because static/app/__mocks__/api.tsx was returning the object with type==Promise as the mocked value. So the request looked like it was fetched right away.
The fix to the mock means that in the future we will be able to hold a request in the pending state... but this test only passes when SIZE_ANALYSIS_URL returns quickly right away. The comment was maybe aspirational, but wrong.
ryan953
marked this pull request as ready for review
October 16, 2026 01:38
The functions only read queryKey (and pageParam), so spell those out with ApiQueryKey and InfiniteApiQueryKey instead of picking them from QueryFunctionContext.
roggenkemper
approved these changes
Oct 16, 2026
useConcurrencyLimitedQueryFn reads signal itself to skip aborted work, but required the wrapped queryFn to accept it too. apiFetch now takes only {queryKey}, so it no longer satisfied that constraint. Require signal on the returned function, which TanStack calls with the full query context, and let the wrapped queryFn take any narrower context.
ryan953
commented
Oct 16, 2026
| const deferred = Promise.withResolvers<TResult>(); | ||
| addItem({ | ||
| reject: deferred.reject, | ||
| run: async () => deferred.resolve(await queryFn(context)), |
Member
Author
There was a problem hiding this comment.
we only need to pass in TContext here queryFn(context). signal isn't required at this point
but signal is required in the surrounding code, so the types have been fiddled with.
scttcper
approved these changes
Oct 16, 2026
This branch was successfully deployed
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.
apiFetchandapiFetchInfinitenow take only the context fields they read, instead of the wholeQueryFunctionContext. This lets amutationFncallapiFetch({queryKey: [url, {method, data}]})and get the sameApiResponse({json, headers, status}) that queries get. A mutation's own context has noqueryKeyorsignal, and the other query fields describe a running query that a mutation would have to invent. No runtime behavior changes.apiFetchtakes{queryKey: ApiQueryKey}, andapiFetchInfinitetakes{queryKey: InfiniteApiQueryKey, pageParam}. Passing either one as aqueryFnstill works, because the full context satisfies the narrower type.usePinnedLogsQuerywas the one caller that built a context literal with extra fields; it now passes only thequeryKey.bodywhenincludeAllArgsis set.apiFetchalways sets it, so a spec that keeps a request pending with a promise body got the pending promise back asjson. WithoutincludeAllArgs,resolve()already adopted the promise. One preprod spec passed only because of that leak (its "pending" size analysis was treated as loaded data); it now mocks a resolved response and lets thePROCESSINGbuild state drive the message.No feature flags. No UI changes, so no screenshots.