From a88cf2183280fcca826ece316f7114eb517be1f9 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Thu, 30 Oct 2025 11:25:48 +0100 Subject: [PATCH 1/3] [WIP] prevent refetching `skipToken` --- src/__tests__/__snapshots__/exports.ts.snap | 1 + src/core/ObservableQuery.ts | 10 ++++++++++ src/core/QueryManager.ts | 6 +++++- .../hooks/__tests__/useSuspenseQuery.test.tsx | 2 +- src/react/hooks/useBackgroundQuery.ts | 2 +- src/react/hooks/useQuery.ts | 14 ++++++-------- src/react/hooks/useSuspenseQuery.ts | 7 +++++-- src/react/index.ts | 4 ++-- src/react/internal/cache/QueryReference.ts | 1 + src/utilities/internal/index.ts | 2 ++ .../internal/skipToken.ts} | 0 11 files changed, 34 insertions(+), 15 deletions(-) rename src/{react/hooks/constants.ts => utilities/internal/skipToken.ts} (100%) diff --git a/src/__tests__/__snapshots__/exports.ts.snap b/src/__tests__/__snapshots__/exports.ts.snap index c7343506bff..41bb3792449 100644 --- a/src/__tests__/__snapshots__/exports.ts.snap +++ b/src/__tests__/__snapshots__/exports.ts.snap @@ -467,6 +467,7 @@ Array [ "removeMaskedFragmentSpreads", "resultKeyNameFromField", "shouldInclude", + "skipToken", "storeKeyNameFromField", "stringifyForDisplay", "toQueryResult", diff --git a/src/core/ObservableQuery.ts b/src/core/ObservableQuery.ts index 5b81a7a9ec1..4fd3760d602 100644 --- a/src/core/ObservableQuery.ts +++ b/src/core/ObservableQuery.ts @@ -24,6 +24,7 @@ import { getOperationName, getQueryDefinition, preventUnhandledRejection, + skipToken, toQueryResult, } from "@apollo/client/utilities/internal"; import { invariant } from "@apollo/client/utilities/invariant"; @@ -1067,7 +1068,16 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`, public applyOptions( newOptions: Partial> ): void { + console.log("applying options", newOptions); const mergedOptions = compact(this.options, newOptions || {}); + if (skipToken in (newOptions.variables || {})) { + Object.assign( + mergedOptions.variables, + this.options.variables, + newOptions.variables + ); + console.log("merging variables to", mergedOptions); + } assign(this.options, mergedOptions); this.updatePolling(); } diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index d9fef505af9..a48a9209d1e 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -61,6 +61,7 @@ import { isNonNullObject, makeUniqueId, removeDirectivesFromDocument, + skipToken, toQueryResult, } from "@apollo/client/utilities/internal"; import { @@ -1295,7 +1296,10 @@ export class QueryManager { if (include) { this.getObservableQueries(include).forEach((oq) => { - if (oq.options.fetchPolicy === "cache-only") { + if ( + oq.options.fetchPolicy === "cache-only" || + skipToken in oq.options.variables + ) { return; } diff --git a/src/react/hooks/__tests__/useSuspenseQuery.test.tsx b/src/react/hooks/__tests__/useSuspenseQuery.test.tsx index 7122e12171a..d523e2ddcf1 100644 --- a/src/react/hooks/__tests__/useSuspenseQuery.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseQuery.test.tsx @@ -5778,7 +5778,7 @@ describe("useSuspenseQuery", () => { ]); }); - it("renders skip result, does not suspend, and maintains `data` when skipping a query with `skipToken` as options after it was enabled", async () => { + it.only("renders skip result, does not suspend, and maintains `data` when skipping a query with `skipToken` as options after it was enabled", async () => { const { query, mocks } = useSimpleQueryCase(); const cache = new InMemoryCache(); diff --git a/src/react/hooks/useBackgroundQuery.ts b/src/react/hooks/useBackgroundQuery.ts index 6634b99adc4..137293c5eb2 100644 --- a/src/react/hooks/useBackgroundQuery.ts +++ b/src/react/hooks/useBackgroundQuery.ts @@ -28,10 +28,10 @@ import { import type { DocumentationTypes as UtilityDocumentationTypes, NoInfer, + SkipToken, VariablesOption, } from "@apollo/client/utilities/internal"; -import type { SkipToken } from "./constants.js"; import { wrapHook } from "./internal/index.js"; import { useApolloClient } from "./useApolloClient.js"; import { useWatchQueryOptions } from "./useSuspenseQuery.js"; diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index 20bb81a6842..f355b97ea74 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -37,15 +37,15 @@ import type { MaybeMasked } from "@apollo/client/masking"; import type { DocumentationTypes as UtilityDocumentationTypes, NoInfer, + SkipToken, VariablesOption, } from "@apollo/client/utilities/internal"; import { maybeDeepFreeze, mergeOptions, + skipToken, } from "@apollo/client/utilities/internal"; -import type { SkipToken } from "./constants.js"; -import { skipToken } from "./constants.js"; import { useDeepMemo, wrapHook } from "./internal/index.js"; import { useApolloClient } from "./useApolloClient.js"; import { useSyncExternalStore } from "./useSyncExternalStore.js"; @@ -212,7 +212,7 @@ const lastWatchOptions = Symbol(); interface ObsQueryWithMeta extends ObservableQuery { [lastWatchOptions]?: Readonly< - ApolloClient.WatchQueryOptions + ApolloClient.WatchQueryOptions >; } @@ -491,8 +491,6 @@ function useQuery_( }, [result, client, observable, previousData, obsQueryFields]); } -const fromSkipToken = Symbol(); - function useOptions( query: DocumentNode | TypedDocumentNode, options: SkipToken | useQuery.Options, NoInfer>, @@ -504,10 +502,9 @@ function useOptions( mergeOptions(defaultOptions as any, { query, fetchPolicy: "standby", + variables: { [skipToken]: true }, }); - (opts as any)[fromSkipToken] = true; - return opts; } @@ -616,7 +613,8 @@ function useResubscribeIfNecessary< // initialFetchPolicy until the hook is rerendered with real options, so we // set it the next time we get real options if ( - (observable[lastWatchOptions] as any)[fromSkipToken] && + (observable as ObsQueryWithMeta)[lastWatchOptions]! + .variables?.[skipToken] && !watchQueryOptions.initialFetchPolicy ) { (watchQueryOptions.initialFetchPolicy as any) = diff --git a/src/react/hooks/useSuspenseQuery.ts b/src/react/hooks/useSuspenseQuery.ts index b8ff4434a72..f763db17eef 100644 --- a/src/react/hooks/useSuspenseQuery.ts +++ b/src/react/hooks/useSuspenseQuery.ts @@ -31,9 +31,9 @@ import type { NoInfer, VariablesOption, } from "@apollo/client/utilities/internal"; +import type { SkipToken } from "@apollo/client/utilities/internal"; +import { skipToken } from "@apollo/client/utilities/internal"; -import type { SkipToken } from "./constants.js"; -import { skipToken } from "./constants.js"; import { __use, useDeepMemo, wrapHook } from "./internal/index.js"; import { validateSuspenseHookOptions } from "./internal/validateSuspenseHookOptions.js"; import { useApolloClient } from "./useApolloClient.js"; @@ -384,6 +384,7 @@ function useSuspenseQuery_< ...([] as any[]).concat(queryKey), ]; + console.log(cacheKey); const queryRef = suspenseCache.getQueryRef(cacheKey, () => client.watchQuery(watchQueryOptions) ); @@ -505,6 +506,7 @@ export function useWatchQueryOptions< return { query, fetchPolicy: "standby", + variables: { [skipToken]: true } as any, } as ApolloClient.WatchQueryOptions; } @@ -515,6 +517,7 @@ export function useWatchQueryOptions< const watchQueryOptions: ApolloClient.WatchQueryOptions = { + variables: {}, ...options, fetchPolicy, query, diff --git a/src/react/index.ts b/src/react/index.ts index fbfcb2d00b2..064803d8898 100644 --- a/src/react/index.ts +++ b/src/react/index.ts @@ -15,8 +15,8 @@ export { useSuspenseFragment } from "./hooks/useSuspenseFragment.js"; export { useLoadableQuery } from "./hooks/useLoadableQuery.js"; export { useQueryRefHandlers } from "./hooks/useQueryRefHandlers.js"; export { useReadQuery } from "./hooks/useReadQuery.js"; -export { skipToken } from "./hooks/constants.js"; -export type { SkipToken } from "./hooks/constants.js"; +export { skipToken } from "@apollo/client/utilities/internal"; +export type { SkipToken } from "@apollo/client/utilities/internal"; export type { PreloadQueryFetchPolicy, diff --git a/src/react/internal/cache/QueryReference.ts b/src/react/internal/cache/QueryReference.ts index b8a30eab748..06c7375db42 100644 --- a/src/react/internal/cache/QueryReference.ts +++ b/src/react/internal/cache/QueryReference.ts @@ -320,6 +320,7 @@ export class InternalQueryReference< // "standby" is used when `skip` is set to `true`. Detect when we've // enabled the query (i.e. `skip` is `false`) to execute a network request. + console.log("queryRef applying options", watchQueryOptions); if ( currentFetchPolicy === "standby" && currentFetchPolicy !== watchQueryOptions.fetchPolicy diff --git a/src/utilities/internal/index.ts b/src/utilities/internal/index.ts index 47add2520ec..bf4953d1ef6 100644 --- a/src/utilities/internal/index.ts +++ b/src/utilities/internal/index.ts @@ -62,6 +62,8 @@ export { toQueryResult } from "./toQueryResult.js"; export { filterMap } from "./filterMap.js"; export { equalByQuery } from "./equalByQuery.js"; export { canonicalStringify } from "./canonicalStringify.js"; +export { skipToken } from "./skipToken.js"; +export type { SkipToken } from "./skipToken.js"; export { getApolloCacheMemoryInternals, diff --git a/src/react/hooks/constants.ts b/src/utilities/internal/skipToken.ts similarity index 100% rename from src/react/hooks/constants.ts rename to src/utilities/internal/skipToken.ts From 674bc8b5d1feede31d8d65a6a97e571f05d82dd8 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Thu, 30 Oct 2025 11:45:16 +0100 Subject: [PATCH 2/3] document failing case of switching `useSuspenseQuery` to `skipToken` when variables were present --- .../hooks/__tests__/useSuspenseQuery.test.tsx | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/react/hooks/__tests__/useSuspenseQuery.test.tsx b/src/react/hooks/__tests__/useSuspenseQuery.test.tsx index d523e2ddcf1..6b5ea26fc6c 100644 --- a/src/react/hooks/__tests__/useSuspenseQuery.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseQuery.test.tsx @@ -168,19 +168,24 @@ async function renderSuspenseHook( } function useSimpleQueryCase() { - const query: TypedDocumentNode> = gql` - query UserQuery { + const query: TypedDocumentNode< + SimpleQueryData, + { optionalVariable?: string } + > = gql` + query UserQuery($optionalVariable: String) { greeting } `; const mocks = [ { - request: { query }, + request: { query, variables: () => true }, result: { data: { greeting: "Hello" } }, delay: 20, }, - ]; + ] satisfies Array< + MockLink.MockedResponse + >; return { query, mocks }; } @@ -5780,11 +5785,14 @@ describe("useSuspenseQuery", () => { it.only("renders skip result, does not suspend, and maintains `data` when skipping a query with `skipToken` as options after it was enabled", async () => { const { query, mocks } = useSimpleQueryCase(); - const cache = new InMemoryCache(); const { result, renders, rerenderAsync } = await renderSuspenseHook( - ({ skip }) => useSuspenseQuery(query, skip ? skipToken : void 0), + ({ skip }) => + useSuspenseQuery( + query, + skip ? skipToken : { variables: { optionalVariable: "foo" } } + ), { cache, mocks, initialProps: { skip: false } } ); From 6dcfae4e7c14d3d754853ffc81f221d717abf2eb Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Thu, 30 Oct 2025 14:03:53 +0100 Subject: [PATCH 3/3] progress --- src/core/ObservableQuery.ts | 2 - src/react/hooks/__tests__/useQuery.test.tsx | 5 +- .../hooks/__tests__/useSuspenseQuery.test.tsx | 218 +++++++++++++----- src/react/hooks/useLoadableQuery.ts | 2 +- src/react/hooks/useQuery.ts | 21 +- src/react/hooks/useSuspenseQuery.ts | 16 +- src/react/internal/cache/QueryReference.ts | 1 - .../matchers/toHaveSuspenseCacheEntryUsing.ts | 2 +- 8 files changed, 194 insertions(+), 73 deletions(-) diff --git a/src/core/ObservableQuery.ts b/src/core/ObservableQuery.ts index 4fd3760d602..dd29d05d735 100644 --- a/src/core/ObservableQuery.ts +++ b/src/core/ObservableQuery.ts @@ -1068,7 +1068,6 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`, public applyOptions( newOptions: Partial> ): void { - console.log("applying options", newOptions); const mergedOptions = compact(this.options, newOptions || {}); if (skipToken in (newOptions.variables || {})) { Object.assign( @@ -1076,7 +1075,6 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`, this.options.variables, newOptions.variables ); - console.log("merging variables to", mergedOptions); } assign(this.options, mergedOptions); this.updatePolling(); diff --git a/src/react/hooks/__tests__/useQuery.test.tsx b/src/react/hooks/__tests__/useQuery.test.tsx index 2d544bb11ac..99db9d61a0a 100644 --- a/src/react/hooks/__tests__/useQuery.test.tsx +++ b/src/react/hooks/__tests__/useQuery.test.tsx @@ -1749,7 +1749,7 @@ describe("useQuery Hook", () => { }); await rerender(skipToken); - + // !!! rerenders with `undefined` variables await expect(renderStream).toRerenderWithSimilarSnapshot(); client.writeQuery({ @@ -1889,7 +1889,7 @@ describe("useQuery Hook", () => { }); await rerender(skipToken); - + // !!! rerenders with `undefined` variables await expect(renderStream).toRerenderWithSimilarSnapshot(); client.writeQuery({ @@ -8702,6 +8702,7 @@ describe("useQuery Hook", () => { const { fetchPolicy, initialFetchPolicy } = observable.options; expect(fetchPolicy).toBe(expectedFetchPolicy); + // last failing test, should be "cache-and-network", but is "cache-first" expect(initialFetchPolicy).toBe(expectedInitialFetchPolicy); } diff --git a/src/react/hooks/__tests__/useSuspenseQuery.test.tsx b/src/react/hooks/__tests__/useSuspenseQuery.test.tsx index 6b5ea26fc6c..20d913b08d7 100644 --- a/src/react/hooks/__tests__/useSuspenseQuery.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseQuery.test.tsx @@ -1115,7 +1115,7 @@ describe("useSuspenseQuery", () => { }); }); - expect(renders.count).toBe(4 + (IS_REACT_19 ? renders.suspenseCount : 0)); + expect(renders.count).toBe(5 + (IS_REACT_19 ? renders.suspenseCount : 0)); expect(renders.suspenseCount).toBe(2); expect(renders.frames).toStrictEqualTyped([ { @@ -1130,6 +1130,12 @@ describe("useSuspenseQuery", () => { networkStatus: NetworkStatus.ready, error: undefined, }, + { + ...mocks[1].result, + dataState: "complete", + networkStatus: NetworkStatus.ready, + error: undefined, + }, ]); }); @@ -1612,7 +1618,7 @@ describe("useSuspenseQuery", () => { }); expect(renders.suspenseCount).toBe(2); - expect(renders.count).toBe(5 + (IS_REACT_19 ? renders.suspenseCount : 0)); + expect(renders.count).toBe(6 + (IS_REACT_19 ? renders.suspenseCount : 0)); expect(renders.frames).toStrictEqualTyped([ { ...mocks[0].result, @@ -1626,6 +1632,12 @@ describe("useSuspenseQuery", () => { networkStatus: NetworkStatus.ready, error: undefined, }, + { + ...mocks[1].result, + dataState: "complete", + networkStatus: NetworkStatus.ready, + error: undefined, + }, { data: { character: { id: "2", name: "Cached hero" } }, dataState: "complete", @@ -1676,7 +1688,7 @@ describe("useSuspenseQuery", () => { error: undefined, }); - expect(renders.count).toBe(5 + (IS_REACT_19 ? renders.suspenseCount : 0)); + expect(renders.count).toBe(7 + (IS_REACT_19 ? renders.suspenseCount : 0)); expect(renders.suspenseCount).toBe(2); expect(renders.frames).toStrictEqualTyped([ { @@ -1691,6 +1703,18 @@ describe("useSuspenseQuery", () => { networkStatus: NetworkStatus.ready, error: undefined, }, + { + ...mocks[1].result, + dataState: "complete", + networkStatus: NetworkStatus.ready, + error: undefined, + }, + { + ...mocks[0].result, + dataState: "complete", + networkStatus: NetworkStatus.ready, + error: undefined, + }, { ...mocks[0].result, dataState: "complete", @@ -1796,7 +1820,7 @@ describe("useSuspenseQuery", () => { }); }); - expect(renders.count).toBe(6 + (IS_REACT_19 ? renders.suspenseCount : 0)); + expect(renders.count).toBe(8 + (IS_REACT_19 ? renders.suspenseCount : 0)); expect(renders.suspenseCount).toBe(2); expect(renders.frames).toStrictEqualTyped([ { @@ -1811,6 +1835,18 @@ describe("useSuspenseQuery", () => { networkStatus: NetworkStatus.ready, error: undefined, }, + { + ...mocks[1].result, + dataState: "complete", + networkStatus: NetworkStatus.ready, + error: undefined, + }, + { + ...mocks[0].result, + dataState: "complete", + networkStatus: NetworkStatus.loading, + error: undefined, + }, { ...mocks[0].result, dataState: "complete", @@ -1915,7 +1951,7 @@ describe("useSuspenseQuery", () => { }); }); - expect(renders.count).toBe(6 + (IS_REACT_19 ? renders.suspenseCount : 0)); + expect(renders.count).toBe(8 + (IS_REACT_19 ? renders.suspenseCount : 0)); expect(renders.suspenseCount).toBe(3); expect(renders.frames).toStrictEqualTyped([ { @@ -1930,6 +1966,18 @@ describe("useSuspenseQuery", () => { networkStatus: NetworkStatus.ready, error: undefined, }, + { + ...mocks[1].result, + dataState: "complete", + networkStatus: NetworkStatus.ready, + error: undefined, + }, + { + ...mocks[2].result, + dataState: "complete", + networkStatus: NetworkStatus.ready, + error: undefined, + }, { ...mocks[2].result, dataState: "complete", @@ -2028,7 +2076,7 @@ describe("useSuspenseQuery", () => { }); }); - expect(renders.count).toBe(6 + (IS_REACT_19 ? renders.suspenseCount : 0)); + expect(renders.count).toBe(8 + (IS_REACT_19 ? renders.suspenseCount : 0)); expect(renders.suspenseCount).toBe(3); expect(renders.frames).toStrictEqualTyped([ { @@ -2043,6 +2091,18 @@ describe("useSuspenseQuery", () => { networkStatus: NetworkStatus.ready, error: undefined, }, + { + ...mocks[1].result, + dataState: "complete", + networkStatus: NetworkStatus.ready, + error: undefined, + }, + { + ...mocks[2].result, + dataState: "complete", + networkStatus: NetworkStatus.ready, + error: undefined, + }, { ...mocks[2].result, dataState: "complete", @@ -2114,7 +2174,7 @@ describe("useSuspenseQuery", () => { }); expect(renders.suspenseCount).toBe(2); - expect(renders.count).toBe(6 + (IS_REACT_19 ? renders.suspenseCount : 0)); + expect(renders.count).toBe(8 + (IS_REACT_19 ? renders.suspenseCount : 0)); expect(renders.frames).toStrictEqualTyped([ { ...mocks[0].result, @@ -2128,6 +2188,18 @@ describe("useSuspenseQuery", () => { networkStatus: NetworkStatus.ready, error: undefined, }, + { + ...mocks[1].result, + dataState: "complete", + networkStatus: NetworkStatus.ready, + error: undefined, + }, + { + ...mocks[0].result, + dataState: "complete", + networkStatus: NetworkStatus.ready, + error: undefined, + }, { ...mocks[0].result, dataState: "complete", @@ -2400,7 +2472,7 @@ describe("useSuspenseQuery", () => { }); }); - expect(renders.count).toBe(4 + (IS_REACT_19 ? renders.suspenseCount : 0)); + expect(renders.count).toBe(5 + (IS_REACT_19 ? renders.suspenseCount : 0)); expect(renders.suspenseCount).toBe(1); expect(renders.frames).toStrictEqualTyped([ { @@ -2421,6 +2493,12 @@ describe("useSuspenseQuery", () => { networkStatus: NetworkStatus.ready, error: undefined, }, + { + ...mocks[1].result, + dataState: "complete", + networkStatus: NetworkStatus.ready, + error: undefined, + }, ]); }); @@ -2890,7 +2968,7 @@ describe("useSuspenseQuery", () => { }); }); - expect(renders.count).toBe(4 + (IS_REACT_19 ? renders.suspenseCount : 0)); + expect(renders.count).toBe(5 + (IS_REACT_19 ? renders.suspenseCount : 0)); expect(renders.suspenseCount).toBe(1); expect(renders.frames).toStrictEqualTyped([ { @@ -2911,6 +2989,12 @@ describe("useSuspenseQuery", () => { networkStatus: NetworkStatus.ready, error: undefined, }, + { + ...mocks[1].result, + dataState: "complete", + networkStatus: NetworkStatus.ready, + error: undefined, + }, ]); }); @@ -3101,7 +3185,7 @@ describe("useSuspenseQuery", () => { // 2. Unsuspend and return results from initial fetch // 3. Change variables and suspend // 5. Unsuspend and return results from refetch - expect(renders.count).toBe(4 + (IS_REACT_19 ? renders.suspenseCount : 0)); + expect(renders.count).toBe(5 + (IS_REACT_19 ? renders.suspenseCount : 0)); expect(renders.suspenseCount).toBe(2); expect(renders.frames).toStrictEqualTyped([ { @@ -3116,6 +3200,12 @@ describe("useSuspenseQuery", () => { networkStatus: NetworkStatus.ready, error: undefined, }, + { + ...mocks[1].result, + dataState: "complete", + networkStatus: NetworkStatus.ready, + error: undefined, + }, ]); } ); @@ -3580,6 +3670,14 @@ describe("useSuspenseQuery", () => { networkStatus: NetworkStatus.ready, error: undefined, }, + { + data: { + vars: { source: "rerender", globalOnlyVar: true, localOnlyVar: true }, + }, + dataState: "complete", + networkStatus: NetworkStatus.ready, + error: undefined, + }, ]); }); @@ -4401,7 +4499,7 @@ describe("useSuspenseQuery", () => { }); }); - expect(renders.count).toBe(4 + (IS_REACT_19 ? renders.suspenseCount : 0)); + expect(renders.count).toBe(5 + (IS_REACT_19 ? renders.suspenseCount : 0)); expect(renders.errorCount).toBe(0); expect(renders.errors).toEqual([]); expect(renders.suspenseCount).toBe(2); @@ -4418,6 +4516,12 @@ describe("useSuspenseQuery", () => { networkStatus: NetworkStatus.ready, error: undefined, }, + { + data: mocks[1].result.data, + dataState: "complete", + networkStatus: NetworkStatus.ready, + error: undefined, + }, ]); }); @@ -5715,7 +5819,7 @@ describe("useSuspenseQuery", () => { }); }); - expect(renders.count).toBe(3 + (IS_REACT_19 ? renders.suspenseCount : 0)); + expect(renders.count).toBe(4 + (IS_REACT_19 ? renders.suspenseCount : 0)); expect(renders.suspenseCount).toBe(1); expect(renders.frames).toStrictEqualTyped([ { @@ -5730,6 +5834,12 @@ describe("useSuspenseQuery", () => { networkStatus: NetworkStatus.ready, error: undefined, }, + { + ...mocks[0].result, + dataState: "complete", + networkStatus: NetworkStatus.ready, + error: undefined, + }, ]); }); @@ -5783,58 +5893,60 @@ describe("useSuspenseQuery", () => { ]); }); - it.only("renders skip result, does not suspend, and maintains `data` when skipping a query with `skipToken` as options after it was enabled", async () => { - const { query, mocks } = useSimpleQueryCase(); - const cache = new InMemoryCache(); + it.each([ + ["with options", { variables: { optionalVariable: "foo" } }], + ["no options", undefined], + ] as const)( + "renders skip result, does not suspend, and maintains `data` when skipping a query with `skipToken` as options after it was enabled (%s)", + async (_, options) => { + const { query, mocks } = useSimpleQueryCase(); + const cache = new InMemoryCache(); - const { result, renders, rerenderAsync } = await renderSuspenseHook( - ({ skip }) => - useSuspenseQuery( - query, - skip ? skipToken : { variables: { optionalVariable: "foo" } } - ), - { cache, mocks, initialProps: { skip: false } } - ); + const { result, renders, rerenderAsync } = await renderSuspenseHook( + ({ skip }) => useSuspenseQuery(query, skip ? skipToken : options), + { cache, mocks, initialProps: { skip: false } } + ); - expect(renders.suspenseCount).toBe(1); + expect(renders.suspenseCount).toBe(1); - await waitFor(() => { - expect(result.current).toStrictEqualTyped({ - ...mocks[0].result, - dataState: "complete", - networkStatus: NetworkStatus.ready, - error: undefined, + await waitFor(() => { + expect(result.current).toStrictEqualTyped({ + ...mocks[0].result, + dataState: "complete", + networkStatus: NetworkStatus.ready, + error: undefined, + }); }); - }); - await rerenderAsync({ skip: true }); + await rerenderAsync({ skip: true }); - expect(renders.suspenseCount).toBe(1); - - expect(result.current).toStrictEqualTyped({ - ...mocks[0].result, - dataState: "complete", - networkStatus: NetworkStatus.ready, - error: undefined, - }); + expect(renders.suspenseCount).toBe(1); - expect(renders.count).toBe(3 + (IS_REACT_19 ? renders.suspenseCount : 0)); - expect(renders.suspenseCount).toBe(1); - expect(renders.frames).toStrictEqualTyped([ - { - ...mocks[0].result, - dataState: "complete", - networkStatus: NetworkStatus.ready, - error: undefined, - }, - { + expect(result.current).toStrictEqualTyped({ ...mocks[0].result, dataState: "complete", networkStatus: NetworkStatus.ready, error: undefined, - }, - ]); - }); + }); + + expect(renders.count).toBe(3 + (IS_REACT_19 ? renders.suspenseCount : 0)); + expect(renders.suspenseCount).toBe(1); + expect(renders.frames).toStrictEqualTyped([ + { + ...mocks[0].result, + dataState: "complete", + networkStatus: NetworkStatus.ready, + error: undefined, + }, + { + ...mocks[0].result, + dataState: "complete", + networkStatus: NetworkStatus.ready, + error: undefined, + }, + ]); + } + ); it("does not make network requests when `skip` is `true`", async () => { const { query, mocks } = useVariablesQueryCase(); diff --git a/src/react/hooks/useLoadableQuery.ts b/src/react/hooks/useLoadableQuery.ts index 62f838ae445..09c0f7e7287 100644 --- a/src/react/hooks/useLoadableQuery.ts +++ b/src/react/hooks/useLoadableQuery.ts @@ -282,7 +282,7 @@ export function useLoadableQuery< "useLoadableQuery: 'loadQuery' should not be called during render. To start a query during render, use the 'useBackgroundQuery' hook." ); - const [variables] = args; + const [variables = {}] = args; const cacheKey: CacheKey = [ query, diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index f355b97ea74..5bf53acc746 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -439,6 +439,14 @@ function useQuery_( let [state, setState] = React.useState(createState); + // TODO: needs to move into non-memoized hook + if (options === skipToken && state.observable[lastWatchOptions]?.variables) { + watchQueryOptions.variables = { + ...state.observable[lastWatchOptions].variables, + [skipToken]: true, + }; + } + if (client !== state.client || query !== state.query) { // If the client or query have changed, we need to create a new InternalState. // This will trigger a re-render with the new state, but it will also continue @@ -602,19 +610,16 @@ function useResubscribeIfNecessary< resultData: InternalResult, /** this hook will mutate properties on `observable` */ observable: ObsQueryWithMeta, - watchQueryOptions: Readonly> + watchQueryOptions: ApolloClient.WatchQueryOptions ) { "use no memo"; - if ( - observable[lastWatchOptions] && - !equal(observable[lastWatchOptions], watchQueryOptions) - ) { + const lastOptions = observable[lastWatchOptions]; + if (lastOptions && !equal(lastOptions, watchQueryOptions)) { // If skipToken was used to generate options, we won't know the correct // initialFetchPolicy until the hook is rerendered with real options, so we // set it the next time we get real options if ( - (observable as ObsQueryWithMeta)[lastWatchOptions]! - .variables?.[skipToken] && + lastOptions.variables?.[skipToken] && !watchQueryOptions.initialFetchPolicy ) { (watchQueryOptions.initialFetchPolicy as any) = @@ -628,7 +633,7 @@ function useResubscribeIfNecessary< // subscriptions, though it does feel less than ideal that reobserve // (potentially) kicks off a network request (for example, when the // variables have changed), which is technically a side-effect. - if (shouldReobserve(observable[lastWatchOptions], watchQueryOptions)) { + if (shouldReobserve(lastOptions, watchQueryOptions)) { observable.reobserve(watchQueryOptions); } else { observable.applyOptions(watchQueryOptions); diff --git a/src/react/hooks/useSuspenseQuery.ts b/src/react/hooks/useSuspenseQuery.ts index f763db17eef..70ca12247b5 100644 --- a/src/react/hooks/useSuspenseQuery.ts +++ b/src/react/hooks/useSuspenseQuery.ts @@ -1,3 +1,4 @@ +import { equal } from "@wry/equality"; import * as React from "react"; import type { @@ -383,8 +384,6 @@ function useSuspenseQuery_< canonicalStringify(variables), ...([] as any[]).concat(queryKey), ]; - - console.log(cacheKey); const queryRef = suspenseCache.getQueryRef(cacheKey, () => client.watchQuery(watchQueryOptions) ); @@ -501,12 +500,19 @@ export function useWatchQueryOptions< TData, TVariables >): ApolloClient.WatchQueryOptions { + const incomingVariables = + options === skipToken ? undefined : options.variables || ({} as TVariables); + let [variables, setLastVariables] = React.useState(incomingVariables); + if (options !== skipToken && !equal(incomingVariables, variables)) { + setLastVariables((variables = incomingVariables)); + } + return useDeepMemo>(() => { if (options === skipToken) { return { query, fetchPolicy: "standby", - variables: { [skipToken]: true } as any, + variables: { [skipToken]: true, ...variables } as any, } as ApolloClient.WatchQueryOptions; } @@ -517,13 +523,13 @@ export function useWatchQueryOptions< const watchQueryOptions: ApolloClient.WatchQueryOptions = { - variables: {}, ...options, + variables, fetchPolicy, query, notifyOnNetworkStatusChange: false, nextFetchPolicy: void 0, - }; + } satisfies ApolloClient.WatchQueryOptions as any; if (__DEV__) { validateSuspenseHookOptions(watchQueryOptions); diff --git a/src/react/internal/cache/QueryReference.ts b/src/react/internal/cache/QueryReference.ts index 06c7375db42..b8a30eab748 100644 --- a/src/react/internal/cache/QueryReference.ts +++ b/src/react/internal/cache/QueryReference.ts @@ -320,7 +320,6 @@ export class InternalQueryReference< // "standby" is used when `skip` is set to `true`. Detect when we've // enabled the query (i.e. `skip` is `false`) to execute a network request. - console.log("queryRef applying options", watchQueryOptions); if ( currentFetchPolicy === "standby" && currentFetchPolicy !== watchQueryOptions.fetchPolicy diff --git a/src/testing/matchers/toHaveSuspenseCacheEntryUsing.ts b/src/testing/matchers/toHaveSuspenseCacheEntryUsing.ts index 4e7b85dc7f6..587db51d2be 100644 --- a/src/testing/matchers/toHaveSuspenseCacheEntryUsing.ts +++ b/src/testing/matchers/toHaveSuspenseCacheEntryUsing.ts @@ -15,7 +15,7 @@ export const toHaveSuspenseCacheEntryUsing: MatcherFunction< queryKey?: string | number | any[]; }, ] -> = function (client, query, { variables, queryKey = [] } = {}) { +> = function (client, query, { variables = {}, queryKey = [] } = {}) { if (!(client instanceof ApolloClient)) { throw new Error("Actual must be an instance of `ApolloClient`"); }