From c2bd48b8ca8dabfad25f8b20772c7db0bd5a66bc Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Mon, 27 May 2024 17:40:34 +0200 Subject: [PATCH 01/32] extract hooks from InternalState --- src/react/hooks/useLazyQuery.ts | 4 +- src/react/hooks/useQuery.ts | 491 ++++++++++++++++---------------- 2 files changed, 253 insertions(+), 242 deletions(-) diff --git a/src/react/hooks/useLazyQuery.ts b/src/react/hooks/useLazyQuery.ts index a8d6eb00a67..68172e06fc9 100644 --- a/src/react/hooks/useLazyQuery.ts +++ b/src/react/hooks/useLazyQuery.ts @@ -10,7 +10,7 @@ import type { LazyQueryResultTuple, NoInfer, } from "../types/types.js"; -import { useInternalState } from "./useQuery.js"; +import { useInternalState, useQueryWithInternalState } from "./useQuery.js"; import { useApolloClient } from "./useApolloClient.js"; // The following methods, when called will execute the query, regardless of @@ -85,7 +85,7 @@ export function useLazyQuery< document ); - const useQueryResult = internalState.useQuery({ + const useQueryResult = useQueryWithInternalState(internalState, { ...merged, skip: !execOptionsRef.current, }); diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index 61ca66527b6..46b4259f022 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -100,11 +100,121 @@ function _useQuery< query: DocumentNode | TypedDocumentNode, options: QueryHookOptions, NoInfer> ) { - return useInternalState(useApolloClient(options.client), query).useQuery( + return useQueryWithInternalState( + useInternalState(useApolloClient(options.client), query), options ); } +export function useQueryWithInternalState< + TData = any, + TVariables extends OperationVariables = OperationVariables, +>( + internalState: InternalState, + options: QueryHookOptions, NoInfer> +) { + // The renderPromises field gets initialized here in the useQuery method, at + // the beginning of everything (for a given component rendering, at least), + // so we can safely use this.renderPromises in other/later InternalState + // methods without worrying it might be uninitialized. Even after + // initialization, this.renderPromises is usually undefined (unless SSR is + // happening), but that's fine as long as it has been initialized that way, + // rather than left uninitialized. + internalState.renderPromises = + React.useContext(getApolloContext()).renderPromises; + + useOptions(internalState, options); + + const obsQuery = useObservableQuery(internalState); + + const result = useSyncExternalStore( + React.useCallback( + (handleStoreChange) => { + if (internalState.renderPromises) { + return () => {}; + } + + internalState.forceUpdate = handleStoreChange; + + const onNext = () => { + const previousResult = internalState.result; + // We use `getCurrentResult()` instead of the onNext argument because + // the values differ slightly. Specifically, loading results will have + // an empty object for data instead of `undefined` for some reason. + const result = obsQuery.getCurrentResult(); + // Make sure we're not attempting to re-render similar results + if ( + previousResult && + previousResult.loading === result.loading && + previousResult.networkStatus === result.networkStatus && + equal(previousResult.data, result.data) + ) { + return; + } + + internalState.setResult(result); + }; + + const onError = (error: Error) => { + subscription.unsubscribe(); + subscription = obsQuery.resubscribeAfterError(onNext, onError); + + if (!hasOwnProperty.call(error, "graphQLErrors")) { + // The error is not a GraphQL error + throw error; + } + + const previousResult = internalState.result; + if ( + !previousResult || + (previousResult && previousResult.loading) || + !equal(error, previousResult.error) + ) { + internalState.setResult({ + data: (previousResult && previousResult.data) as TData, + error: error as ApolloError, + loading: false, + networkStatus: NetworkStatus.error, + }); + } + }; + + let subscription = obsQuery.subscribe(onNext, onError); + + // Do the "unsubscribe" with a short delay. + // This way, an existing subscription can be reused without an additional + // request if "unsubscribe" and "resubscribe" to the same ObservableQuery + // happen in very fast succession. + return () => { + setTimeout(() => subscription.unsubscribe()); + internalState.forceUpdate = () => internalState.forceUpdateState(); + }; + }, + // eslint-disable-next-line react-compiler/react-compiler + // eslint-disable-next-line react-hooks/exhaustive-deps + [ + // We memoize the subscribe function using useCallback and the following + // dependency keys, because the subscribe function reference is all that + // useSyncExternalStore uses internally as a dependency key for the + // useEffect ultimately responsible for the subscription, so we are + // effectively passing this dependency array to that useEffect buried + // inside useSyncExternalStore, as desired. + obsQuery, + internalState.renderPromises, + internalState.client.disableNetworkFetches, + ] + ), + + () => internalState.getCurrentResult(), + () => internalState.getCurrentResult() + ); + + // TODO Remove this method when we remove support for options.partialRefetch. + internalState.unsafeHandlePartialRefetch(result); + + return internalState.toQueryResult(result); +} + export function useInternalState( client: ApolloClient, query: DocumentNode | TypedDocumentNode @@ -137,6 +247,128 @@ export function useInternalState( return state; } +function useOptions( + internalState: InternalState, + options: QueryHookOptions +) { + const watchQueryOptions = internalState.createWatchQueryOptions( + (internalState.queryHookOptions = options) + ); + + // Update this.watchQueryOptions, but only when they have changed, which + // allows us to depend on the referential stability of + // this.watchQueryOptions elsewhere. + const currentWatchQueryOptions = internalState.watchQueryOptions; + + if (!equal(watchQueryOptions, currentWatchQueryOptions)) { + internalState.watchQueryOptions = watchQueryOptions; + + if (currentWatchQueryOptions && internalState.observable) { + // Though it might be tempting to postpone this reobserve call to the + // useEffect block, we need getCurrentResult to return an appropriate + // loading:true result synchronously (later within the same call to + // useQuery). Since we already have this.observable here (not true for + // the very first call to useQuery), we are not initiating any new + // 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. + internalState.observable.reobserve(internalState.getObsQueryOptions()); + + // Make sure getCurrentResult returns a fresh ApolloQueryResult, + // but save the current data as this.previousData, just like setResult + // usually does. + internalState.previousData = + internalState.result?.data || internalState.previousData; + internalState.result = void 0; + } + } + + // Make sure state.onCompleted and state.onError always reflect the latest + // options.onCompleted and options.onError callbacks provided to useQuery, + // since those functions are often recreated every time useQuery is called. + // Like the forceUpdate method, the versions of these methods inherited from + // InternalState.prototype are empty no-ops, but we can override them on the + // base state object (without modifying the prototype). + internalState.onCompleted = + options.onCompleted || InternalState.prototype.onCompleted; + internalState.onError = options.onError || InternalState.prototype.onError; + + if ( + (internalState.renderPromises || + internalState.client.disableNetworkFetches) && + internalState.queryHookOptions.ssr === false && + !internalState.queryHookOptions.skip + ) { + // If SSR has been explicitly disabled, and this function has been called + // on the server side, return the default loading state. + internalState.result = internalState.ssrDisabledResult; + } else if ( + internalState.queryHookOptions.skip || + internalState.watchQueryOptions.fetchPolicy === "standby" + ) { + // When skipping a query (ie. we're not querying for data but still want to + // render children), make sure the `data` is cleared out and `loading` is + // set to `false` (since we aren't loading anything). + // + // NOTE: We no longer think this is the correct behavior. Skipping should + // not automatically set `data` to `undefined`, but instead leave the + // previous data in place. In other words, skipping should not mandate that + // previously received data is all of a sudden removed. Unfortunately, + // changing this is breaking, so we'll have to wait until Apollo Client 4.0 + // to address this. + internalState.result = internalState.skipStandbyResult; + } else if ( + internalState.result === internalState.ssrDisabledResult || + internalState.result === internalState.skipStandbyResult + ) { + internalState.result = void 0; + } +} + +function useObservableQuery( + internalState: InternalState +) { + // See if there is an existing observable that was used to fetch the same + // data and if so, use it instead since it will contain the proper queryId + // to fetch the result set. This is used during SSR. + const obsQuery = (internalState.observable = + (internalState.renderPromises && + internalState.renderPromises.getSSRObservable( + internalState.watchQueryOptions + )) || + internalState.observable || // Reuse this.observable if possible (and not SSR) + internalState.client.watchQuery(internalState.getObsQueryOptions())); + + internalState.obsQueryFields = React.useMemo( + () => ({ + refetch: obsQuery.refetch.bind(obsQuery), + reobserve: obsQuery.reobserve.bind(obsQuery), + fetchMore: obsQuery.fetchMore.bind(obsQuery), + updateQuery: obsQuery.updateQuery.bind(obsQuery), + startPolling: obsQuery.startPolling.bind(obsQuery), + stopPolling: obsQuery.stopPolling.bind(obsQuery), + subscribeToMore: obsQuery.subscribeToMore.bind(obsQuery), + }), + [obsQuery] + ); + + const ssrAllowed = !( + internalState.queryHookOptions.ssr === false || + internalState.queryHookOptions.skip + ); + + if (internalState.renderPromises && ssrAllowed) { + internalState.renderPromises.registerSSRObservable(obsQuery); + + if (obsQuery.getCurrentResult().loading) { + // TODO: This is a legacy API which could probably be cleaned up + internalState.renderPromises.addObservableQueryPromise(obsQuery); + } + } + + return obsQuery; +} + class InternalState { constructor( public readonly client: ReturnType, @@ -219,196 +451,15 @@ class InternalState { }); } - // Methods beginning with use- should be called according to the standard - // rules of React hooks: only at the top level of the calling function, and - // without any dynamic conditional logic. - useQuery(options: QueryHookOptions) { - // The renderPromises field gets initialized here in the useQuery method, at - // the beginning of everything (for a given component rendering, at least), - // so we can safely use this.renderPromises in other/later InternalState - // methods without worrying it might be uninitialized. Even after - // initialization, this.renderPromises is usually undefined (unless SSR is - // happening), but that's fine as long as it has been initialized that way, - // rather than left uninitialized. - // eslint-disable-next-line react-hooks/rules-of-hooks - this.renderPromises = React.useContext(getApolloContext()).renderPromises; - - this.useOptions(options); - - const obsQuery = this.useObservableQuery(); - - // eslint-disable-next-line react-hooks/rules-of-hooks - const result = useSyncExternalStore( - // eslint-disable-next-line react-hooks/rules-of-hooks - React.useCallback( - (handleStoreChange) => { - if (this.renderPromises) { - return () => {}; - } - - this.forceUpdate = handleStoreChange; - - const onNext = () => { - const previousResult = this.result; - // We use `getCurrentResult()` instead of the onNext argument because - // the values differ slightly. Specifically, loading results will have - // an empty object for data instead of `undefined` for some reason. - const result = obsQuery.getCurrentResult(); - // Make sure we're not attempting to re-render similar results - if ( - previousResult && - previousResult.loading === result.loading && - previousResult.networkStatus === result.networkStatus && - equal(previousResult.data, result.data) - ) { - return; - } - - this.setResult(result); - }; - - const onError = (error: Error) => { - subscription.unsubscribe(); - subscription = obsQuery.resubscribeAfterError(onNext, onError); - - if (!hasOwnProperty.call(error, "graphQLErrors")) { - // The error is not a GraphQL error - throw error; - } - - const previousResult = this.result; - if ( - !previousResult || - (previousResult && previousResult.loading) || - !equal(error, previousResult.error) - ) { - this.setResult({ - data: (previousResult && previousResult.data) as TData, - error: error as ApolloError, - loading: false, - networkStatus: NetworkStatus.error, - }); - } - }; - - let subscription = obsQuery.subscribe(onNext, onError); - - // Do the "unsubscribe" with a short delay. - // This way, an existing subscription can be reused without an additional - // request if "unsubscribe" and "resubscribe" to the same ObservableQuery - // happen in very fast succession. - return () => { - setTimeout(() => subscription.unsubscribe()); - this.forceUpdate = () => this.forceUpdateState(); - }; - }, - [ - // We memoize the subscribe function using useCallback and the following - // dependency keys, because the subscribe function reference is all that - // useSyncExternalStore uses internally as a dependency key for the - // useEffect ultimately responsible for the subscription, so we are - // effectively passing this dependency array to that useEffect buried - // inside useSyncExternalStore, as desired. - obsQuery, - // eslint-disable-next-line react-hooks/exhaustive-deps - this.renderPromises, - // eslint-disable-next-line react-hooks/exhaustive-deps - this.client.disableNetworkFetches, - ] - ), - - () => this.getCurrentResult(), - () => this.getCurrentResult() - ); - - // TODO Remove this method when we remove support for options.partialRefetch. - this.unsafeHandlePartialRefetch(result); - - return this.toQueryResult(result); - } - // These members (except for renderPromises) are all populated by the // useOptions method, which is called unconditionally at the beginning of the // useQuery method, so we can safely use these members in other/later methods // without worrying they might be uninitialized. - private renderPromises: ApolloContextValue["renderPromises"]; - private queryHookOptions!: QueryHookOptions; - private watchQueryOptions!: WatchQueryOptions; - - private useOptions(options: QueryHookOptions) { - const watchQueryOptions = this.createWatchQueryOptions( - (this.queryHookOptions = options) - ); - - // Update this.watchQueryOptions, but only when they have changed, which - // allows us to depend on the referential stability of - // this.watchQueryOptions elsewhere. - const currentWatchQueryOptions = this.watchQueryOptions; - - if (!equal(watchQueryOptions, currentWatchQueryOptions)) { - this.watchQueryOptions = watchQueryOptions; - - if (currentWatchQueryOptions && this.observable) { - // Though it might be tempting to postpone this reobserve call to the - // useEffect block, we need getCurrentResult to return an appropriate - // loading:true result synchronously (later within the same call to - // useQuery). Since we already have this.observable here (not true for - // the very first call to useQuery), we are not initiating any new - // 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. - this.observable.reobserve(this.getObsQueryOptions()); - - // Make sure getCurrentResult returns a fresh ApolloQueryResult, - // but save the current data as this.previousData, just like setResult - // usually does. - this.previousData = this.result?.data || this.previousData; - this.result = void 0; - } - } - - // Make sure state.onCompleted and state.onError always reflect the latest - // options.onCompleted and options.onError callbacks provided to useQuery, - // since those functions are often recreated every time useQuery is called. - // Like the forceUpdate method, the versions of these methods inherited from - // InternalState.prototype are empty no-ops, but we can override them on the - // base state object (without modifying the prototype). - this.onCompleted = - options.onCompleted || InternalState.prototype.onCompleted; - this.onError = options.onError || InternalState.prototype.onError; - - if ( - (this.renderPromises || this.client.disableNetworkFetches) && - this.queryHookOptions.ssr === false && - !this.queryHookOptions.skip - ) { - // If SSR has been explicitly disabled, and this function has been called - // on the server side, return the default loading state. - this.result = this.ssrDisabledResult; - } else if ( - this.queryHookOptions.skip || - this.watchQueryOptions.fetchPolicy === "standby" - ) { - // When skipping a query (ie. we're not querying for data but still want to - // render children), make sure the `data` is cleared out and `loading` is - // set to `false` (since we aren't loading anything). - // - // NOTE: We no longer think this is the correct behavior. Skipping should - // not automatically set `data` to `undefined`, but instead leave the - // previous data in place. In other words, skipping should not mandate that - // previously received data is all of a sudden removed. Unfortunately, - // changing this is breaking, so we'll have to wait until Apollo Client 4.0 - // to address this. - this.result = this.skipStandbyResult; - } else if ( - this.result === this.ssrDisabledResult || - this.result === this.skipStandbyResult - ) { - this.result = void 0; - } - } + public renderPromises: ApolloContextValue["renderPromises"]; + public queryHookOptions!: QueryHookOptions; + public watchQueryOptions!: WatchQueryOptions; - private getObsQueryOptions(): WatchQueryOptions { + public getObsQueryOptions(): WatchQueryOptions { const toMerge: Array>> = []; const globalDefaults = this.client.defaultOptions.watchQuery; @@ -438,14 +489,14 @@ class InternalState { return toMerge.reduce(mergeOptions) as WatchQueryOptions; } - private ssrDisabledResult = maybeDeepFreeze({ + public ssrDisabledResult = maybeDeepFreeze({ loading: true, data: void 0 as unknown as TData, error: void 0, networkStatus: NetworkStatus.loading, }); - private skipStandbyResult = maybeDeepFreeze({ + public skipStandbyResult = maybeDeepFreeze({ loading: false, data: void 0 as unknown as TData, error: void 0, @@ -453,7 +504,7 @@ class InternalState { }); // A function to massage options before passing them to ObservableQuery. - private createWatchQueryOptions({ + public createWatchQueryOptions({ skip, ssr, onCompleted, @@ -519,61 +570,21 @@ class InternalState { // Defining these methods as no-ops on the prototype allows us to call // state.onCompleted and/or state.onError without worrying about whether a // callback was provided. - private onCompleted(data: TData) {} - private onError(error: ApolloError) {} + public onCompleted(data: TData) {} + public onError(error: ApolloError) {} - private observable!: ObservableQuery; + public observable!: ObservableQuery; public obsQueryFields!: Omit< ObservableQueryFields, "variables" >; - private useObservableQuery() { - // See if there is an existing observable that was used to fetch the same - // data and if so, use it instead since it will contain the proper queryId - // to fetch the result set. This is used during SSR. - const obsQuery = (this.observable = - (this.renderPromises && - this.renderPromises.getSSRObservable(this.watchQueryOptions)) || - this.observable || // Reuse this.observable if possible (and not SSR) - this.client.watchQuery(this.getObsQueryOptions())); - - // eslint-disable-next-line react-hooks/rules-of-hooks - this.obsQueryFields = React.useMemo( - () => ({ - refetch: obsQuery.refetch.bind(obsQuery), - reobserve: obsQuery.reobserve.bind(obsQuery), - fetchMore: obsQuery.fetchMore.bind(obsQuery), - updateQuery: obsQuery.updateQuery.bind(obsQuery), - startPolling: obsQuery.startPolling.bind(obsQuery), - stopPolling: obsQuery.stopPolling.bind(obsQuery), - subscribeToMore: obsQuery.subscribeToMore.bind(obsQuery), - }), - [obsQuery] - ); - - const ssrAllowed = !( - this.queryHookOptions.ssr === false || this.queryHookOptions.skip - ); - - if (this.renderPromises && ssrAllowed) { - this.renderPromises.registerSSRObservable(obsQuery); - - if (obsQuery.getCurrentResult().loading) { - // TODO: This is a legacy API which could probably be cleaned up - this.renderPromises.addObservableQueryPromise(obsQuery); - } - } - - return obsQuery; - } - // These members are populated by getCurrentResult and setResult, and it's // okay/normal for them to be initially undefined. - private result: undefined | ApolloQueryResult; - private previousData: undefined | TData; + public result: undefined | ApolloQueryResult; + public previousData: undefined | TData; - private setResult(nextResult: ApolloQueryResult) { + public setResult(nextResult: ApolloQueryResult) { const previousResult = this.result; if (previousResult && previousResult.data) { this.previousData = previousResult.data; @@ -585,7 +596,7 @@ class InternalState { this.handleErrorOrCompleted(nextResult, previousResult); } - private handleErrorOrCompleted( + public handleErrorOrCompleted( result: ApolloQueryResult, previousResult?: ApolloQueryResult ) { @@ -611,7 +622,7 @@ class InternalState { } } - private toApolloError( + public toApolloError( result: ApolloQueryResult ): ApolloError | undefined { return isNonEmptyArray(result.errors) ? @@ -619,7 +630,7 @@ class InternalState { : result.error; } - private getCurrentResult(): ApolloQueryResult { + public getCurrentResult(): ApolloQueryResult { // Using this.result as a cache ensures getCurrentResult continues returning // the same (===) result object, unless state.setResult has been called, or // we're doing server rendering and therefore override the result below. @@ -634,7 +645,7 @@ class InternalState { // This cache allows the referential stability of this.result (as returned by // getCurrentResult) to translate into referential stability of the resulting // QueryResult object returned by toQueryResult. - private toQueryResultCache = new (canUseWeakMap ? WeakMap : Map)< + public toQueryResultCache = new (canUseWeakMap ? WeakMap : Map)< ApolloQueryResult, QueryResult >(); @@ -671,7 +682,7 @@ class InternalState { return queryResult; } - private unsafeHandlePartialRefetch(result: ApolloQueryResult) { + public unsafeHandlePartialRefetch(result: ApolloQueryResult) { // WARNING: SIDE-EFFECTS IN THE RENDER FUNCTION // // TODO: This code should be removed when the partialRefetch option is From dc58beb82a2f15b9fb2733e6f4ab562855680af4 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Mon, 27 May 2024 18:00:33 +0200 Subject: [PATCH 02/32] inline hooks and functions into each other, move `createWatchQueryOptions` out --- src/react/hooks/useLazyQuery.ts | 2 +- src/react/hooks/useQuery.ts | 349 +++++++++++++++----------------- 2 files changed, 168 insertions(+), 183 deletions(-) diff --git a/src/react/hooks/useLazyQuery.ts b/src/react/hooks/useLazyQuery.ts index 68172e06fc9..e44f79a8464 100644 --- a/src/react/hooks/useLazyQuery.ts +++ b/src/react/hooks/useLazyQuery.ts @@ -142,7 +142,7 @@ export function useLazyQuery< }); const promise = internalState - .executeQuery({ ...options, skip: false }) + .executeQuery({ ...options, skip: false }, false) .then((queryResult) => Object.assign(queryResult, eagerMethods)); // Because the return value of `useLazyQuery` is usually floated, we need diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index 46b4259f022..4d6fa35d732 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -9,7 +9,6 @@ import type { WatchQueryFetchPolicy, } from "../../core/index.js"; import { mergeOptions } from "../../utilities/index.js"; -import type { ApolloContextValue } from "../context/index.js"; import { getApolloContext } from "../context/index.js"; import { ApolloError } from "../../errors/index.js"; import type { @@ -120,17 +119,122 @@ export function useQueryWithInternalState< // initialization, this.renderPromises is usually undefined (unless SSR is // happening), but that's fine as long as it has been initialized that way, // rather than left uninitialized. - internalState.renderPromises = - React.useContext(getApolloContext()).renderPromises; + const renderPromises = React.useContext(getApolloContext()).renderPromises; - useOptions(internalState, options); + const watchQueryOptions = createWatchQueryOptions( + (internalState.queryHookOptions = options), + internalState, + !!renderPromises + ); + + // Update this.watchQueryOptions, but only when they have changed, which + // allows us to depend on the referential stability of + // this.watchQueryOptions elsewhere. + const currentWatchQueryOptions = internalState.watchQueryOptions; + + if (!equal(watchQueryOptions, currentWatchQueryOptions)) { + internalState.watchQueryOptions = watchQueryOptions; + + if (currentWatchQueryOptions && internalState.observable) { + // Though it might be tempting to postpone this reobserve call to the + // useEffect block, we need getCurrentResult to return an appropriate + // loading:true result synchronously (later within the same call to + // useQuery). Since we already have this.observable here (not true for + // the very first call to useQuery), we are not initiating any new + // 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. + internalState.observable.reobserve(internalState.getObsQueryOptions()); + + // Make sure getCurrentResult returns a fresh ApolloQueryResult, + // but save the current data as this.previousData, just like setResult + // usually does. + internalState.previousData = + internalState.result?.data || internalState.previousData; + internalState.result = void 0; + } + } - const obsQuery = useObservableQuery(internalState); + // Make sure state.onCompleted and state.onError always reflect the latest + // options.onCompleted and options.onError callbacks provided to useQuery, + // since those functions are often recreated every time useQuery is called. + // Like the forceUpdate method, the versions of these methods inherited from + // InternalState.prototype are empty no-ops, but we can override them on the + // base state object (without modifying the prototype). + internalState.onCompleted = + options.onCompleted || InternalState.prototype.onCompleted; + internalState.onError = options.onError || InternalState.prototype.onError; + + if ( + (renderPromises || internalState.client.disableNetworkFetches) && + internalState.queryHookOptions.ssr === false && + !internalState.queryHookOptions.skip + ) { + // If SSR has been explicitly disabled, and this function has been called + // on the server side, return the default loading state. + internalState.result = internalState.ssrDisabledResult; + } else if ( + internalState.queryHookOptions.skip || + internalState.watchQueryOptions.fetchPolicy === "standby" + ) { + // When skipping a query (ie. we're not querying for data but still want to + // render children), make sure the `data` is cleared out and `loading` is + // set to `false` (since we aren't loading anything). + // + // NOTE: We no longer think this is the correct behavior. Skipping should + // not automatically set `data` to `undefined`, but instead leave the + // previous data in place. In other words, skipping should not mandate that + // previously received data is all of a sudden removed. Unfortunately, + // changing this is breaking, so we'll have to wait until Apollo Client 4.0 + // to address this. + internalState.result = internalState.skipStandbyResult; + } else if ( + internalState.result === internalState.ssrDisabledResult || + internalState.result === internalState.skipStandbyResult + ) { + internalState.result = void 0; + } + + // See if there is an existing observable that was used to fetch the same + // data and if so, use it instead since it will contain the proper queryId + // to fetch the result set. This is used during SSR. + const obsQuery = (internalState.observable = + (renderPromises && + renderPromises.getSSRObservable(internalState.watchQueryOptions)) || + internalState.observable || // Reuse this.observable if possible (and not SSR) + internalState.client.watchQuery(internalState.getObsQueryOptions())); + + internalState.obsQueryFields = React.useMemo( + () => ({ + refetch: obsQuery.refetch.bind(obsQuery), + reobserve: obsQuery.reobserve.bind(obsQuery), + fetchMore: obsQuery.fetchMore.bind(obsQuery), + updateQuery: obsQuery.updateQuery.bind(obsQuery), + startPolling: obsQuery.startPolling.bind(obsQuery), + stopPolling: obsQuery.stopPolling.bind(obsQuery), + subscribeToMore: obsQuery.subscribeToMore.bind(obsQuery), + }), + [obsQuery] + ); + + const ssrAllowed = !( + internalState.queryHookOptions.ssr === false || + internalState.queryHookOptions.skip + ); + + if (renderPromises && ssrAllowed) { + renderPromises.registerSSRObservable(obsQuery); + + if (obsQuery.getCurrentResult().loading) { + // TODO: This is a legacy API which could probably be cleaned up + renderPromises.addObservableQueryPromise(obsQuery); + } + } const result = useSyncExternalStore( React.useCallback( (handleStoreChange) => { - if (internalState.renderPromises) { + if (renderPromises) { return () => {}; } @@ -200,7 +304,7 @@ export function useQueryWithInternalState< // effectively passing this dependency array to that useEffect buried // inside useSyncExternalStore, as desired. obsQuery, - internalState.renderPromises, + renderPromises, internalState.client.disableNetworkFetches, ] ), @@ -246,127 +350,66 @@ export function useInternalState( return state; } - -function useOptions( +// A function to massage options before passing them to ObservableQuery. +function createWatchQueryOptions< + TData = any, + TVariables extends OperationVariables = OperationVariables, +>( + { + skip, + ssr, + onCompleted, + onError, + defaultOptions, + // The above options are useQuery-specific, so this ...otherOptions spread + // makes otherOptions almost a WatchQueryOptions object, except for the + // query property that we add below. + ...otherOptions + }: QueryHookOptions = {}, internalState: InternalState, - options: QueryHookOptions -) { - const watchQueryOptions = internalState.createWatchQueryOptions( - (internalState.queryHookOptions = options) + hasRenderPromises: boolean +): WatchQueryOptions { + // This Object.assign is safe because otherOptions is a fresh ...rest object + // that did not exist until just now, so modifications are still allowed. + const watchQueryOptions: WatchQueryOptions = Object.assign( + otherOptions, + { query: internalState.query } ); - // Update this.watchQueryOptions, but only when they have changed, which - // allows us to depend on the referential stability of - // this.watchQueryOptions elsewhere. - const currentWatchQueryOptions = internalState.watchQueryOptions; - - if (!equal(watchQueryOptions, currentWatchQueryOptions)) { - internalState.watchQueryOptions = watchQueryOptions; - - if (currentWatchQueryOptions && internalState.observable) { - // Though it might be tempting to postpone this reobserve call to the - // useEffect block, we need getCurrentResult to return an appropriate - // loading:true result synchronously (later within the same call to - // useQuery). Since we already have this.observable here (not true for - // the very first call to useQuery), we are not initiating any new - // 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. - internalState.observable.reobserve(internalState.getObsQueryOptions()); - - // Make sure getCurrentResult returns a fresh ApolloQueryResult, - // but save the current data as this.previousData, just like setResult - // usually does. - internalState.previousData = - internalState.result?.data || internalState.previousData; - internalState.result = void 0; - } - } - - // Make sure state.onCompleted and state.onError always reflect the latest - // options.onCompleted and options.onError callbacks provided to useQuery, - // since those functions are often recreated every time useQuery is called. - // Like the forceUpdate method, the versions of these methods inherited from - // InternalState.prototype are empty no-ops, but we can override them on the - // base state object (without modifying the prototype). - internalState.onCompleted = - options.onCompleted || InternalState.prototype.onCompleted; - internalState.onError = options.onError || InternalState.prototype.onError; - if ( - (internalState.renderPromises || - internalState.client.disableNetworkFetches) && - internalState.queryHookOptions.ssr === false && - !internalState.queryHookOptions.skip - ) { - // If SSR has been explicitly disabled, and this function has been called - // on the server side, return the default loading state. - internalState.result = internalState.ssrDisabledResult; - } else if ( - internalState.queryHookOptions.skip || - internalState.watchQueryOptions.fetchPolicy === "standby" - ) { - // When skipping a query (ie. we're not querying for data but still want to - // render children), make sure the `data` is cleared out and `loading` is - // set to `false` (since we aren't loading anything). - // - // NOTE: We no longer think this is the correct behavior. Skipping should - // not automatically set `data` to `undefined`, but instead leave the - // previous data in place. In other words, skipping should not mandate that - // previously received data is all of a sudden removed. Unfortunately, - // changing this is breaking, so we'll have to wait until Apollo Client 4.0 - // to address this. - internalState.result = internalState.skipStandbyResult; - } else if ( - internalState.result === internalState.ssrDisabledResult || - internalState.result === internalState.skipStandbyResult + hasRenderPromises && + (watchQueryOptions.fetchPolicy === "network-only" || + watchQueryOptions.fetchPolicy === "cache-and-network") ) { - internalState.result = void 0; + // this behavior was added to react-apollo without explanation in this PR + // https://github.com/apollographql/react-apollo/pull/1579 + watchQueryOptions.fetchPolicy = "cache-first"; } -} -function useObservableQuery( - internalState: InternalState -) { - // See if there is an existing observable that was used to fetch the same - // data and if so, use it instead since it will contain the proper queryId - // to fetch the result set. This is used during SSR. - const obsQuery = (internalState.observable = - (internalState.renderPromises && - internalState.renderPromises.getSSRObservable( - internalState.watchQueryOptions - )) || - internalState.observable || // Reuse this.observable if possible (and not SSR) - internalState.client.watchQuery(internalState.getObsQueryOptions())); - - internalState.obsQueryFields = React.useMemo( - () => ({ - refetch: obsQuery.refetch.bind(obsQuery), - reobserve: obsQuery.reobserve.bind(obsQuery), - fetchMore: obsQuery.fetchMore.bind(obsQuery), - updateQuery: obsQuery.updateQuery.bind(obsQuery), - startPolling: obsQuery.startPolling.bind(obsQuery), - stopPolling: obsQuery.stopPolling.bind(obsQuery), - subscribeToMore: obsQuery.subscribeToMore.bind(obsQuery), - }), - [obsQuery] - ); - - const ssrAllowed = !( - internalState.queryHookOptions.ssr === false || - internalState.queryHookOptions.skip - ); - - if (internalState.renderPromises && ssrAllowed) { - internalState.renderPromises.registerSSRObservable(obsQuery); + if (!watchQueryOptions.variables) { + watchQueryOptions.variables = {} as TVariables; + } - if (obsQuery.getCurrentResult().loading) { - // TODO: This is a legacy API which could probably be cleaned up - internalState.renderPromises.addObservableQueryPromise(obsQuery); - } + if (skip) { + const { + fetchPolicy = internalState.getDefaultFetchPolicy(), + initialFetchPolicy = fetchPolicy, + } = watchQueryOptions; + + // When skipping, we set watchQueryOptions.fetchPolicy initially to + // "standby", but we also need/want to preserve the initial non-standby + // fetchPolicy that would have been used if not skipping. + Object.assign(watchQueryOptions, { + initialFetchPolicy, + fetchPolicy: "standby", + }); + } else if (!watchQueryOptions.fetchPolicy) { + watchQueryOptions.fetchPolicy = + internalState.observable?.options.initialFetchPolicy || + internalState.getDefaultFetchPolicy(); } - return obsQuery; + return watchQueryOptions; } class InternalState { @@ -409,14 +452,17 @@ class InternalState { executeQuery( options: QueryHookOptions & { query?: DocumentNode; - } + }, + hasRenderPromises: boolean ) { if (options.query) { Object.assign(this, { query: options.query }); } - this.watchQueryOptions = this.createWatchQueryOptions( - (this.queryHookOptions = options) + this.watchQueryOptions = createWatchQueryOptions( + (this.queryHookOptions = options), + this, + hasRenderPromises ); const concast = this.observable.reobserveAsConcast( @@ -451,11 +497,6 @@ class InternalState { }); } - // These members (except for renderPromises) are all populated by the - // useOptions method, which is called unconditionally at the beginning of the - // useQuery method, so we can safely use these members in other/later methods - // without worrying they might be uninitialized. - public renderPromises: ApolloContextValue["renderPromises"]; public queryHookOptions!: QueryHookOptions; public watchQueryOptions!: WatchQueryOptions; @@ -503,62 +544,6 @@ class InternalState { networkStatus: NetworkStatus.ready, }); - // A function to massage options before passing them to ObservableQuery. - public createWatchQueryOptions({ - skip, - ssr, - onCompleted, - onError, - defaultOptions, - // The above options are useQuery-specific, so this ...otherOptions spread - // makes otherOptions almost a WatchQueryOptions object, except for the - // query property that we add below. - ...otherOptions - }: QueryHookOptions = {}): WatchQueryOptions< - TVariables, - TData - > { - // This Object.assign is safe because otherOptions is a fresh ...rest object - // that did not exist until just now, so modifications are still allowed. - const watchQueryOptions: WatchQueryOptions = - Object.assign(otherOptions, { query: this.query }); - - if ( - this.renderPromises && - (watchQueryOptions.fetchPolicy === "network-only" || - watchQueryOptions.fetchPolicy === "cache-and-network") - ) { - // this behavior was added to react-apollo without explanation in this PR - // https://github.com/apollographql/react-apollo/pull/1579 - watchQueryOptions.fetchPolicy = "cache-first"; - } - - if (!watchQueryOptions.variables) { - watchQueryOptions.variables = {} as TVariables; - } - - if (skip) { - const { - fetchPolicy = this.getDefaultFetchPolicy(), - initialFetchPolicy = fetchPolicy, - } = watchQueryOptions; - - // When skipping, we set watchQueryOptions.fetchPolicy initially to - // "standby", but we also need/want to preserve the initial non-standby - // fetchPolicy that would have been used if not skipping. - Object.assign(watchQueryOptions, { - initialFetchPolicy, - fetchPolicy: "standby", - }); - } else if (!watchQueryOptions.fetchPolicy) { - watchQueryOptions.fetchPolicy = - this.observable?.options.initialFetchPolicy || - this.getDefaultFetchPolicy(); - } - - return watchQueryOptions; - } - getDefaultFetchPolicy(): WatchQueryFetchPolicy { return ( this.queryHookOptions.defaultOptions?.fetchPolicy || From 10466e5c63defffee66f5c03a58daa0b02dd44d5 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Mon, 27 May 2024 18:12:29 +0200 Subject: [PATCH 03/32] move `executeQuery` to `useLazyQuery` --- src/react/hooks/useLazyQuery.ts | 83 ++++++++++++++++++++++-- src/react/hooks/useQuery.ts | 111 ++++++-------------------------- 2 files changed, 95 insertions(+), 99 deletions(-) diff --git a/src/react/hooks/useLazyQuery.ts b/src/react/hooks/useLazyQuery.ts index e44f79a8464..a6e3350a428 100644 --- a/src/react/hooks/useLazyQuery.ts +++ b/src/react/hooks/useLazyQuery.ts @@ -2,15 +2,25 @@ import type { DocumentNode } from "graphql"; import type { TypedDocumentNode } from "@graphql-typed-document-node/core"; import * as React from "rehackt"; -import type { OperationVariables } from "../../core/index.js"; +import type { + ApolloQueryResult, + OperationVariables, +} from "../../core/index.js"; import { mergeOptions } from "../../utilities/index.js"; import type { LazyQueryHookExecOptions, LazyQueryHookOptions, LazyQueryResultTuple, NoInfer, + QueryHookOptions, + QueryResult, } from "../types/types.js"; -import { useInternalState, useQueryWithInternalState } from "./useQuery.js"; +import type { InternalState } from "./useQuery.js"; +import { + createWatchQueryOptions, + useInternalState, + useQueryWithInternalState, +} from "./useQuery.js"; import { useApolloClient } from "./useApolloClient.js"; // The following methods, when called will execute the query, regardless of @@ -94,7 +104,8 @@ export function useLazyQuery< useQueryResult.observable.options.initialFetchPolicy || internalState.getDefaultFetchPolicy(); - const { forceUpdateState, obsQueryFields } = internalState; + const { obsQueryFields } = internalState; + const forceUpdateState = React.useReducer((tick) => tick + 1, 0)[1]; // We use useMemo here to make sure the eager methods have a stable identity. const eagerMethods = React.useMemo(() => { const eagerMethods: Record = {}; @@ -141,9 +152,12 @@ export function useLazyQuery< ...execOptionsRef.current, }); - const promise = internalState - .executeQuery({ ...options, skip: false }, false) - .then((queryResult) => Object.assign(queryResult, eagerMethods)); + const promise = executeQuery( + { ...options, skip: false }, + false, + internalState, + forceUpdateState + ).then((queryResult) => Object.assign(queryResult, eagerMethods)); // Because the return value of `useLazyQuery` is usually floated, we need // to catch the promise to prevent unhandled rejections. @@ -151,8 +165,63 @@ export function useLazyQuery< return promise; }, - [eagerMethods, initialFetchPolicy, internalState] + [eagerMethods, forceUpdateState, initialFetchPolicy, internalState] ); return [execute, result]; } + +function executeQuery( + options: QueryHookOptions & { + query?: DocumentNode; + }, + hasRenderPromises: boolean, + internalState: InternalState, + forceUpdate: () => void +) { + if (options.query) { + internalState.query = options.query; + } + + internalState.watchQueryOptions = createWatchQueryOptions( + (internalState.queryHookOptions = options), + internalState, + hasRenderPromises + ); + + const concast = internalState.observable.reobserveAsConcast( + internalState.getObsQueryOptions() + ); + + // Make sure getCurrentResult returns a fresh ApolloQueryResult, + // but save the current data as this.previousData, just like setResult + // usually does. + internalState.previousData = + internalState.result?.data || internalState.previousData; + internalState.result = void 0; + forceUpdate(); + + return new Promise>((resolve) => { + let result: ApolloQueryResult; + + // Subscribe to the concast independently of the ObservableQuery in case + // the component gets unmounted before the promise resolves. This prevents + // the concast from terminating early and resolving with `undefined` when + // there are no more subscribers for the concast. + concast.subscribe({ + next: (value) => { + result = value; + }, + error: () => { + resolve( + internalState.toQueryResult( + internalState.observable.getCurrentResult() + ) + ); + }, + complete: () => { + resolve(internalState.toQueryResult(result)); + }, + }); + }); +} diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index 4d6fa35d732..c2fdc84c186 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -238,8 +238,6 @@ export function useQueryWithInternalState< return () => {}; } - internalState.forceUpdate = handleStoreChange; - const onNext = () => { const previousResult = internalState.result; // We use `getCurrentResult()` instead of the onNext argument because @@ -256,7 +254,7 @@ export function useQueryWithInternalState< return; } - internalState.setResult(result); + internalState.setResult(result, handleStoreChange); }; const onError = (error: Error) => { @@ -274,12 +272,15 @@ export function useQueryWithInternalState< (previousResult && previousResult.loading) || !equal(error, previousResult.error) ) { - internalState.setResult({ - data: (previousResult && previousResult.data) as TData, - error: error as ApolloError, - loading: false, - networkStatus: NetworkStatus.error, - }); + internalState.setResult( + { + data: (previousResult && previousResult.data) as TData, + error: error as ApolloError, + loading: false, + networkStatus: NetworkStatus.error, + }, + handleStoreChange + ); } }; @@ -291,7 +292,6 @@ export function useQueryWithInternalState< // happen in very fast succession. return () => { setTimeout(() => subscription.unsubscribe()); - internalState.forceUpdate = () => internalState.forceUpdateState(); }; }, // eslint-disable-next-line react-compiler/react-compiler @@ -323,17 +323,8 @@ export function useInternalState( client: ApolloClient, query: DocumentNode | TypedDocumentNode ): InternalState { - // By default, InternalState.prototype.forceUpdate is an empty function, but - // we replace it here (before anyone has had a chance to see this state yet) - // with a function that unconditionally forces an update, using the latest - // setTick function. Updating this state by calling state.forceUpdate or the - // uSES notification callback are the only way we trigger React component updates. - const forceUpdateState = React.useReducer((tick) => tick + 1, 0)[1]; - function createInternalState(previous?: InternalState) { - return Object.assign(new InternalState(client, query, previous), { - forceUpdateState, - }); + return new InternalState(client, query, previous); } let [state, updateState] = React.useState(createInternalState); @@ -351,7 +342,7 @@ export function useInternalState( return state; } // A function to massage options before passing them to ObservableQuery. -function createWatchQueryOptions< +export function createWatchQueryOptions< TData = any, TVariables extends OperationVariables = OperationVariables, >( @@ -412,10 +403,11 @@ function createWatchQueryOptions< return watchQueryOptions; } +export { type InternalState }; class InternalState { constructor( public readonly client: ReturnType, - public readonly query: DocumentNode | TypedDocumentNode, + public query: DocumentNode | TypedDocumentNode, previous?: InternalState ) { verifyDocumentType(query, DocumentType.Query); @@ -429,74 +421,6 @@ class InternalState { } } - /** - * Forces an update using local component state. - * As this is not batched with `useSyncExternalStore` updates, - * this is only used as a fallback if the `useSyncExternalStore` "force update" - * method is not registered at the moment. - * See https://github.com/facebook/react/issues/25191 - * */ - forceUpdateState() { - // Replaced (in useInternalState) with a method that triggers an update. - invariant.warn( - "Calling default no-op implementation of InternalState#forceUpdate" - ); - } - - /** - * Will be overwritten by the `useSyncExternalStore` "force update" method - * whenever it is available and reset to `forceUpdateState` when it isn't. - */ - forceUpdate = () => this.forceUpdateState(); - - executeQuery( - options: QueryHookOptions & { - query?: DocumentNode; - }, - hasRenderPromises: boolean - ) { - if (options.query) { - Object.assign(this, { query: options.query }); - } - - this.watchQueryOptions = createWatchQueryOptions( - (this.queryHookOptions = options), - this, - hasRenderPromises - ); - - const concast = this.observable.reobserveAsConcast( - this.getObsQueryOptions() - ); - - // Make sure getCurrentResult returns a fresh ApolloQueryResult, - // but save the current data as this.previousData, just like setResult - // usually does. - this.previousData = this.result?.data || this.previousData; - this.result = void 0; - this.forceUpdate(); - - return new Promise>((resolve) => { - let result: ApolloQueryResult; - - // Subscribe to the concast independently of the ObservableQuery in case - // the component gets unmounted before the promise resolves. This prevents - // the concast from terminating early and resolving with `undefined` when - // there are no more subscribers for the concast. - concast.subscribe({ - next: (value) => { - result = value; - }, - error: () => { - resolve(this.toQueryResult(this.observable.getCurrentResult())); - }, - complete: () => { - resolve(this.toQueryResult(result)); - }, - }); - }); - } - public queryHookOptions!: QueryHookOptions; public watchQueryOptions!: WatchQueryOptions; @@ -569,7 +493,10 @@ class InternalState { public result: undefined | ApolloQueryResult; public previousData: undefined | TData; - public setResult(nextResult: ApolloQueryResult) { + public setResult( + nextResult: ApolloQueryResult, + forceUpdate: () => void + ) { const previousResult = this.result; if (previousResult && previousResult.data) { this.previousData = previousResult.data; @@ -577,7 +504,7 @@ class InternalState { this.result = nextResult; // Calling state.setResult always triggers an update, though some call sites // perform additional equality checks before committing to an update. - this.forceUpdate(); + forceUpdate(); this.handleErrorOrCompleted(nextResult, previousResult); } From ea77f601cdc3f2b4b6bb78c4e60762bb1e5e8746 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Mon, 27 May 2024 18:30:54 +0200 Subject: [PATCH 04/32] move more functions out --- src/react/hooks/useLazyQuery.ts | 8 +- src/react/hooks/useQuery.ts | 158 +++++++++++++++++--------------- 2 files changed, 87 insertions(+), 79 deletions(-) diff --git a/src/react/hooks/useLazyQuery.ts b/src/react/hooks/useLazyQuery.ts index a6e3350a428..40a95d13605 100644 --- a/src/react/hooks/useLazyQuery.ts +++ b/src/react/hooks/useLazyQuery.ts @@ -18,6 +18,7 @@ import type { import type { InternalState } from "./useQuery.js"; import { createWatchQueryOptions, + toQueryResult, useInternalState, useQueryWithInternalState, } from "./useQuery.js"; @@ -214,13 +215,14 @@ function executeQuery( }, error: () => { resolve( - internalState.toQueryResult( - internalState.observable.getCurrentResult() + toQueryResult( + internalState.observable.getCurrentResult(), + internalState ) ); }, complete: () => { - resolve(internalState.toQueryResult(result)); + resolve(toQueryResult(result, internalState)); }, }); }); diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index c2fdc84c186..3193117ec82 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -172,7 +172,7 @@ export function useQueryWithInternalState< ) { // If SSR has been explicitly disabled, and this function has been called // on the server side, return the default loading state. - internalState.result = internalState.ssrDisabledResult; + internalState.result = ssrDisabledResult; } else if ( internalState.queryHookOptions.skip || internalState.watchQueryOptions.fetchPolicy === "standby" @@ -187,10 +187,10 @@ export function useQueryWithInternalState< // previously received data is all of a sudden removed. Unfortunately, // changing this is breaking, so we'll have to wait until Apollo Client 4.0 // to address this. - internalState.result = internalState.skipStandbyResult; + internalState.result = skipStandbyResult; } else if ( - internalState.result === internalState.ssrDisabledResult || - internalState.result === internalState.skipStandbyResult + internalState.result === ssrDisabledResult || + internalState.result === skipStandbyResult ) { internalState.result = void 0; } @@ -314,9 +314,9 @@ export function useQueryWithInternalState< ); // TODO Remove this method when we remove support for options.partialRefetch. - internalState.unsafeHandlePartialRefetch(result); + unsafeHandlePartialRefetch(result, internalState); - return internalState.toQueryResult(result); + return toQueryResult(result, internalState); } export function useInternalState( @@ -454,20 +454,6 @@ class InternalState { return toMerge.reduce(mergeOptions) as WatchQueryOptions; } - public ssrDisabledResult = maybeDeepFreeze({ - loading: true, - data: void 0 as unknown as TData, - error: void 0, - networkStatus: NetworkStatus.loading, - }); - - public skipStandbyResult = maybeDeepFreeze({ - loading: false, - data: void 0 as unknown as TData, - error: void 0, - networkStatus: NetworkStatus.ready, - }); - getDefaultFetchPolicy(): WatchQueryFetchPolicy { return ( this.queryHookOptions.defaultOptions?.fetchPolicy || @@ -513,7 +499,7 @@ class InternalState { previousResult?: ApolloQueryResult ) { if (!result.loading) { - const error = this.toApolloError(result); + const error = toApolloError(result); // wait a tick in case we are in the middle of rendering a component Promise.resolve() @@ -534,14 +520,6 @@ class InternalState { } } - public toApolloError( - result: ApolloQueryResult - ): ApolloError | undefined { - return isNonEmptyArray(result.errors) ? - new ApolloError({ graphQLErrors: result.errors }) - : result.error; - } - public getCurrentResult(): ApolloQueryResult { // Using this.result as a cache ensures getCurrentResult continues returning // the same (===) result object, unless state.setResult has been called, or @@ -561,57 +539,85 @@ class InternalState { ApolloQueryResult, QueryResult >(); +} - toQueryResult( - result: ApolloQueryResult - ): QueryResult { - let queryResult = this.toQueryResultCache.get(result); - if (queryResult) return queryResult; - - const { data, partial, ...resultWithoutPartial } = result; - this.toQueryResultCache.set( - result, - (queryResult = { - data, // Ensure always defined, even if result.data is missing. - ...resultWithoutPartial, - ...this.obsQueryFields, - client: this.client, - observable: this.observable, - variables: this.observable.variables, - called: !this.queryHookOptions.skip, - previousData: this.previousData, - }) - ); +function toApolloError( + result: ApolloQueryResult +): ApolloError | undefined { + return isNonEmptyArray(result.errors) ? + new ApolloError({ graphQLErrors: result.errors }) + : result.error; +} - if (!queryResult.error && isNonEmptyArray(result.errors)) { - // Until a set naming convention for networkError and graphQLErrors is - // decided upon, we map errors (graphQLErrors) to the error options. - // TODO: Is it possible for both result.error and result.errors to be - // defined here? - queryResult.error = new ApolloError({ graphQLErrors: result.errors }); - } +export function toQueryResult( + result: ApolloQueryResult, + internalState: InternalState +): QueryResult { + let queryResult = internalState.toQueryResultCache.get(result); + if (queryResult) return queryResult; + + const { data, partial, ...resultWithoutPartial } = result; + internalState.toQueryResultCache.set( + result, + (queryResult = { + data, // Ensure always defined, even if result.data is missing. + ...resultWithoutPartial, + ...internalState.obsQueryFields, + client: internalState.client, + observable: internalState.observable, + variables: internalState.observable.variables, + called: !internalState.queryHookOptions.skip, + previousData: internalState.previousData, + }) + ); - return queryResult; + if (!queryResult.error && isNonEmptyArray(result.errors)) { + // Until a set naming convention for networkError and graphQLErrors is + // decided upon, we map errors (graphQLErrors) to the error options. + // TODO: Is it possible for both result.error and result.errors to be + // defined here? + queryResult.error = new ApolloError({ graphQLErrors: result.errors }); } - public unsafeHandlePartialRefetch(result: ApolloQueryResult) { - // WARNING: SIDE-EFFECTS IN THE RENDER FUNCTION - // - // TODO: This code should be removed when the partialRefetch option is - // removed. I was unable to get this hook to behave reasonably in certain - // edge cases when this block was put in an effect. - if ( - result.partial && - this.queryHookOptions.partialRefetch && - !result.loading && - (!result.data || Object.keys(result.data).length === 0) && - this.observable.options.fetchPolicy !== "cache-only" - ) { - Object.assign(result, { - loading: true, - networkStatus: NetworkStatus.refetch, - }); - this.observable.refetch(); - } + return queryResult; +} +function unsafeHandlePartialRefetch< + TData, + TVariables extends OperationVariables, +>( + result: ApolloQueryResult, + internalState: InternalState +) { + // WARNING: SIDE-EFFECTS IN THE RENDER FUNCTION + // + // TODO: This code should be removed when the partialRefetch option is + // removed. I was unable to get this hook to behave reasonably in certain + // edge cases when this block was put in an effect. + if ( + result.partial && + internalState.queryHookOptions.partialRefetch && + !result.loading && + (!result.data || Object.keys(result.data).length === 0) && + internalState.observable.options.fetchPolicy !== "cache-only" + ) { + Object.assign(result, { + loading: true, + networkStatus: NetworkStatus.refetch, + }); + internalState.observable.refetch(); } } + +const ssrDisabledResult = maybeDeepFreeze({ + loading: true, + data: void 0 as any, + error: void 0, + networkStatus: NetworkStatus.loading, +}); + +const skipStandbyResult = maybeDeepFreeze({ + loading: false, + data: void 0 as any, + error: void 0, + networkStatus: NetworkStatus.ready, +}); From fde4c4d5972a057eb52953810dca224cf1d74245 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Tue, 28 May 2024 16:06:03 +0200 Subject: [PATCH 05/32] move `unsafeHandlePartialRefetch` into `setResult` --- src/react/hooks/useQuery.ts | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index 3193117ec82..614c2d987c4 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -313,9 +313,6 @@ export function useQueryWithInternalState< () => internalState.getCurrentResult() ); - // TODO Remove this method when we remove support for options.partialRefetch. - unsafeHandlePartialRefetch(result, internalState); - return toQueryResult(result, internalState); } @@ -487,7 +484,7 @@ class InternalState { if (previousResult && previousResult.data) { this.previousData = previousResult.data; } - this.result = nextResult; + this.result = unsafeHandlePartialRefetch(nextResult, this); // Calling state.setResult always triggers an update, though some call sites // perform additional equality checks before committing to an update. forceUpdate(); @@ -525,11 +522,11 @@ class InternalState { // the same (===) result object, unless state.setResult has been called, or // we're doing server rendering and therefore override the result below. if (!this.result) { - this.handleErrorOrCompleted( - (this.result = this.observable.getCurrentResult()) - ); + // WARNING: SIDE-EFFECTS IN THE RENDER FUNCTION + // this could call unsafeHandlePartialRefetch + this.setResult(this.observable.getCurrentResult(), () => {}); } - return this.result; + return this.result!; } // This cache allows the referential stability of this.result (as returned by @@ -581,15 +578,14 @@ export function toQueryResult( return queryResult; } + function unsafeHandlePartialRefetch< TData, TVariables extends OperationVariables, >( result: ApolloQueryResult, internalState: InternalState -) { - // WARNING: SIDE-EFFECTS IN THE RENDER FUNCTION - // +): ApolloQueryResult { // TODO: This code should be removed when the partialRefetch option is // removed. I was unable to get this hook to behave reasonably in certain // edge cases when this block was put in an effect. @@ -600,12 +596,14 @@ function unsafeHandlePartialRefetch< (!result.data || Object.keys(result.data).length === 0) && internalState.observable.options.fetchPolicy !== "cache-only" ) { - Object.assign(result, { + internalState.observable.refetch(); + return { + ...result, loading: true, networkStatus: NetworkStatus.refetch, - }); - internalState.observable.refetch(); + }; } + return result; } const ssrDisabledResult = maybeDeepFreeze({ From e1b7ed789815ec866044cbecd397f6c7a7ff9038 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Wed, 29 May 2024 14:26:19 +0200 Subject: [PATCH 06/32] remove `toQueryResultCache`, save transformed result in `internalState.result` --- src/react/hooks/useQuery.ts | 109 +++++++++++++++++------------------- 1 file changed, 52 insertions(+), 57 deletions(-) diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index 614c2d987c4..974b934349c 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -30,7 +30,6 @@ import type { import { DocumentType, verifyDocumentType } from "../parser/index.js"; import { useApolloClient } from "./useApolloClient.js"; import { - canUseWeakMap, compact, isNonEmptyArray, maybeDeepFreeze, @@ -41,6 +40,12 @@ const { prototype: { hasOwnProperty }, } = Object; +const originalResult = Symbol(); +interface InternalQueryResult + extends QueryResult { + [originalResult]: ApolloQueryResult; +} + /** * A hook for executing queries in an Apollo application. * @@ -165,6 +170,28 @@ export function useQueryWithInternalState< options.onCompleted || InternalState.prototype.onCompleted; internalState.onError = options.onError || InternalState.prototype.onError; + // See if there is an existing observable that was used to fetch the same + // data and if so, use it instead since it will contain the proper queryId + // to fetch the result set. This is used during SSR. + const obsQuery = (internalState.observable = + (renderPromises && + renderPromises.getSSRObservable(internalState.watchQueryOptions)) || + internalState.observable || // Reuse this.observable if possible (and not SSR) + internalState.client.watchQuery(internalState.getObsQueryOptions())); + + internalState.obsQueryFields = React.useMemo( + () => ({ + refetch: obsQuery.refetch.bind(obsQuery), + reobserve: obsQuery.reobserve.bind(obsQuery), + fetchMore: obsQuery.fetchMore.bind(obsQuery), + updateQuery: obsQuery.updateQuery.bind(obsQuery), + startPolling: obsQuery.startPolling.bind(obsQuery), + stopPolling: obsQuery.stopPolling.bind(obsQuery), + subscribeToMore: obsQuery.subscribeToMore.bind(obsQuery), + }), + [obsQuery] + ); + if ( (renderPromises || internalState.client.disableNetworkFetches) && internalState.queryHookOptions.ssr === false && @@ -172,7 +199,7 @@ export function useQueryWithInternalState< ) { // If SSR has been explicitly disabled, and this function has been called // on the server side, return the default loading state. - internalState.result = ssrDisabledResult; + internalState.result = toQueryResult(ssrDisabledResult, internalState); } else if ( internalState.queryHookOptions.skip || internalState.watchQueryOptions.fetchPolicy === "standby" @@ -187,36 +214,14 @@ export function useQueryWithInternalState< // previously received data is all of a sudden removed. Unfortunately, // changing this is breaking, so we'll have to wait until Apollo Client 4.0 // to address this. - internalState.result = skipStandbyResult; + internalState.result = toQueryResult(skipStandbyResult, internalState); } else if ( - internalState.result === ssrDisabledResult || - internalState.result === skipStandbyResult + internalState.result?.[originalResult] === ssrDisabledResult || + internalState.result?.[originalResult] === skipStandbyResult ) { internalState.result = void 0; } - // See if there is an existing observable that was used to fetch the same - // data and if so, use it instead since it will contain the proper queryId - // to fetch the result set. This is used during SSR. - const obsQuery = (internalState.observable = - (renderPromises && - renderPromises.getSSRObservable(internalState.watchQueryOptions)) || - internalState.observable || // Reuse this.observable if possible (and not SSR) - internalState.client.watchQuery(internalState.getObsQueryOptions())); - - internalState.obsQueryFields = React.useMemo( - () => ({ - refetch: obsQuery.refetch.bind(obsQuery), - reobserve: obsQuery.reobserve.bind(obsQuery), - fetchMore: obsQuery.fetchMore.bind(obsQuery), - updateQuery: obsQuery.updateQuery.bind(obsQuery), - startPolling: obsQuery.startPolling.bind(obsQuery), - stopPolling: obsQuery.stopPolling.bind(obsQuery), - subscribeToMore: obsQuery.subscribeToMore.bind(obsQuery), - }), - [obsQuery] - ); - const ssrAllowed = !( internalState.queryHookOptions.ssr === false || internalState.queryHookOptions.skip @@ -313,7 +318,7 @@ export function useQueryWithInternalState< () => internalState.getCurrentResult() ); - return toQueryResult(result, internalState); + return result; } export function useInternalState( @@ -473,7 +478,7 @@ class InternalState { // These members are populated by getCurrentResult and setResult, and it's // okay/normal for them to be initially undefined. - public result: undefined | ApolloQueryResult; + public result: undefined | InternalQueryResult; public previousData: undefined | TData; public setResult( @@ -484,11 +489,14 @@ class InternalState { if (previousResult && previousResult.data) { this.previousData = previousResult.data; } - this.result = unsafeHandlePartialRefetch(nextResult, this); + this.result = toQueryResult( + unsafeHandlePartialRefetch(nextResult, this), + this + ); // Calling state.setResult always triggers an update, though some call sites // perform additional equality checks before committing to an update. forceUpdate(); - this.handleErrorOrCompleted(nextResult, previousResult); + this.handleErrorOrCompleted(nextResult, previousResult?.[originalResult]); } public handleErrorOrCompleted( @@ -517,7 +525,7 @@ class InternalState { } } - public getCurrentResult(): ApolloQueryResult { + public getCurrentResult(): QueryResult { // Using this.result as a cache ensures getCurrentResult continues returning // the same (===) result object, unless state.setResult has been called, or // we're doing server rendering and therefore override the result below. @@ -528,14 +536,6 @@ class InternalState { } return this.result!; } - - // This cache allows the referential stability of this.result (as returned by - // getCurrentResult) to translate into referential stability of the resulting - // QueryResult object returned by toQueryResult. - public toQueryResultCache = new (canUseWeakMap ? WeakMap : Map)< - ApolloQueryResult, - QueryResult - >(); } function toApolloError( @@ -549,24 +549,19 @@ function toApolloError( export function toQueryResult( result: ApolloQueryResult, internalState: InternalState -): QueryResult { - let queryResult = internalState.toQueryResultCache.get(result); - if (queryResult) return queryResult; - +): InternalQueryResult { const { data, partial, ...resultWithoutPartial } = result; - internalState.toQueryResultCache.set( - result, - (queryResult = { - data, // Ensure always defined, even if result.data is missing. - ...resultWithoutPartial, - ...internalState.obsQueryFields, - client: internalState.client, - observable: internalState.observable, - variables: internalState.observable.variables, - called: !internalState.queryHookOptions.skip, - previousData: internalState.previousData, - }) - ); + const queryResult: InternalQueryResult = { + [originalResult]: result, + data, // Ensure always defined, even if result.data is missing. + ...resultWithoutPartial, + ...internalState.obsQueryFields, + client: internalState.client, + observable: internalState.observable, + variables: internalState.observable.variables, + called: !internalState.queryHookOptions.skip, + previousData: internalState.previousData, + }; if (!queryResult.error && isNonEmptyArray(result.errors)) { // Until a set naming convention for networkError and graphQLErrors is From 9c865ca1a890d2b6a36ab1c79dcda02df42fcd54 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Wed, 29 May 2024 15:37:29 +0200 Subject: [PATCH 07/32] fixup test --- src/react/hooks/useQuery.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index 974b934349c..a0e328f9bce 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -552,7 +552,6 @@ export function toQueryResult( ): InternalQueryResult { const { data, partial, ...resultWithoutPartial } = result; const queryResult: InternalQueryResult = { - [originalResult]: result, data, // Ensure always defined, even if result.data is missing. ...resultWithoutPartial, ...internalState.obsQueryFields, @@ -561,7 +560,12 @@ export function toQueryResult( variables: internalState.observable.variables, called: !internalState.queryHookOptions.skip, previousData: internalState.previousData, - }; + } satisfies QueryResult as InternalQueryResult< + TData, + TVariables + >; + // non-enumerable property to hold the original result, for referential equality checks + Object.defineProperty(queryResult, originalResult, { value: result }); if (!queryResult.error && isNonEmptyArray(result.errors)) { // Until a set naming convention for networkError and graphQLErrors is From b5e16431494ff97109039cc17b359cee3fb117f9 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Wed, 29 May 2024 16:08:57 +0200 Subject: [PATCH 08/32] move `getDefaultFetchPolicy` out --- src/react/hooks/useLazyQuery.ts | 6 +++++- src/react/hooks/useQuery.ts | 33 +++++++++++++++++++++++---------- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/src/react/hooks/useLazyQuery.ts b/src/react/hooks/useLazyQuery.ts index 40a95d13605..0874af6ac32 100644 --- a/src/react/hooks/useLazyQuery.ts +++ b/src/react/hooks/useLazyQuery.ts @@ -18,6 +18,7 @@ import type { import type { InternalState } from "./useQuery.js"; import { createWatchQueryOptions, + getDefaultFetchPolicy, toQueryResult, useInternalState, useQueryWithInternalState, @@ -103,7 +104,10 @@ export function useLazyQuery< const initialFetchPolicy = useQueryResult.observable.options.initialFetchPolicy || - internalState.getDefaultFetchPolicy(); + getDefaultFetchPolicy( + internalState.queryHookOptions.defaultOptions, + internalState.client.defaultOptions + ); const { obsQueryFields } = internalState; const forceUpdateState = React.useReducer((tick) => tick + 1, 0)[1]; diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index a0e328f9bce..6a1c8e701f3 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -5,6 +5,7 @@ import { useSyncExternalStore } from "./useSyncExternalStore.js"; import { equal } from "@wry/equality"; import type { + DefaultOptions, OperationVariables, WatchQueryFetchPolicy, } from "../../core/index.js"; @@ -385,7 +386,10 @@ export function createWatchQueryOptions< if (skip) { const { - fetchPolicy = internalState.getDefaultFetchPolicy(), + fetchPolicy = getDefaultFetchPolicy( + internalState.queryHookOptions.defaultOptions, + internalState.client.defaultOptions + ), initialFetchPolicy = fetchPolicy, } = watchQueryOptions; @@ -399,7 +403,10 @@ export function createWatchQueryOptions< } else if (!watchQueryOptions.fetchPolicy) { watchQueryOptions.fetchPolicy = internalState.observable?.options.initialFetchPolicy || - internalState.getDefaultFetchPolicy(); + getDefaultFetchPolicy( + internalState.queryHookOptions.defaultOptions, + internalState.client.defaultOptions + ); } return watchQueryOptions; @@ -456,14 +463,6 @@ class InternalState { return toMerge.reduce(mergeOptions) as WatchQueryOptions; } - getDefaultFetchPolicy(): WatchQueryFetchPolicy { - return ( - this.queryHookOptions.defaultOptions?.fetchPolicy || - this.client.defaultOptions.watchQuery?.fetchPolicy || - "cache-first" - ); - } - // Defining these methods as no-ops on the prototype allows us to call // state.onCompleted and/or state.onError without worrying about whether a // callback was provided. @@ -538,6 +537,20 @@ class InternalState { } } +export function getDefaultFetchPolicy< + TData, + TVariables extends OperationVariables, +>( + queryHookDefaultOptions?: Partial>, + clientDefaultOptions?: DefaultOptions +): WatchQueryFetchPolicy { + return ( + queryHookDefaultOptions?.fetchPolicy || + clientDefaultOptions?.watchQuery?.fetchPolicy || + "cache-first" + ); +} + function toApolloError( result: ApolloQueryResult ): ApolloError | undefined { From b4c8bf91bc2ea5830c3a3e24dce52009fe0a6fcf Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Wed, 29 May 2024 16:33:19 +0200 Subject: [PATCH 09/32] move more functions out --- src/react/hooks/useQuery.ts | 80 ++++++++++++++++++++----------------- 1 file changed, 43 insertions(+), 37 deletions(-) diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index 6a1c8e701f3..4d74a746f1a 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -315,8 +315,8 @@ export function useQueryWithInternalState< ] ), - () => internalState.getCurrentResult(), - () => internalState.getCurrentResult() + () => getCurrentResult(internalState), + () => getCurrentResult(internalState) ); return result; @@ -495,46 +495,52 @@ class InternalState { // Calling state.setResult always triggers an update, though some call sites // perform additional equality checks before committing to an update. forceUpdate(); - this.handleErrorOrCompleted(nextResult, previousResult?.[originalResult]); + handleErrorOrCompleted(nextResult, previousResult?.[originalResult], this); } +} - public handleErrorOrCompleted( - result: ApolloQueryResult, - previousResult?: ApolloQueryResult - ) { - if (!result.loading) { - const error = toApolloError(result); - - // wait a tick in case we are in the middle of rendering a component - Promise.resolve() - .then(() => { - if (error) { - this.onError(error); - } else if ( - result.data && - previousResult?.networkStatus !== result.networkStatus && - result.networkStatus === NetworkStatus.ready - ) { - this.onCompleted(result.data); - } - }) - .catch((error) => { - invariant.warn(error); - }); - } +function handleErrorOrCompleted( + result: ApolloQueryResult, + previousResult: ApolloQueryResult | undefined, + internalState: InternalState +) { + if (!result.loading) { + const error = toApolloError(result); + + // wait a tick in case we are in the middle of rendering a component + Promise.resolve() + .then(() => { + if (error) { + internalState.onError(error); + } else if ( + result.data && + previousResult?.networkStatus !== result.networkStatus && + result.networkStatus === NetworkStatus.ready + ) { + internalState.onCompleted(result.data); + } + }) + .catch((error) => { + invariant.warn(error); + }); } +} - public getCurrentResult(): QueryResult { - // Using this.result as a cache ensures getCurrentResult continues returning - // the same (===) result object, unless state.setResult has been called, or - // we're doing server rendering and therefore override the result below. - if (!this.result) { - // WARNING: SIDE-EFFECTS IN THE RENDER FUNCTION - // this could call unsafeHandlePartialRefetch - this.setResult(this.observable.getCurrentResult(), () => {}); - } - return this.result!; +function getCurrentResult( + internalState: InternalState +): QueryResult { + // Using this.result as a cache ensures getCurrentResult continues returning + // the same (===) result object, unless state.setResult has been called, or + // we're doing server rendering and therefore override the result below. + if (!internalState.result) { + // WARNING: SIDE-EFFECTS IN THE RENDER FUNCTION + // this could call unsafeHandlePartialRefetch + internalState.setResult( + internalState.observable.getCurrentResult(), + () => {} + ); } + return internalState.result!; } export function getDefaultFetchPolicy< From 12e19f99254d7cb39f0ee185809d28b4ca1270d3 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Wed, 29 May 2024 16:40:08 +0200 Subject: [PATCH 10/32] moved all class methods out --- src/react/hooks/useLazyQuery.ts | 3 +- src/react/hooks/useQuery.ts | 118 ++++++++++++++++++-------------- 2 files changed, 67 insertions(+), 54 deletions(-) diff --git a/src/react/hooks/useLazyQuery.ts b/src/react/hooks/useLazyQuery.ts index 0874af6ac32..d13d7596664 100644 --- a/src/react/hooks/useLazyQuery.ts +++ b/src/react/hooks/useLazyQuery.ts @@ -19,6 +19,7 @@ import type { InternalState } from "./useQuery.js"; import { createWatchQueryOptions, getDefaultFetchPolicy, + getObsQueryOptions, toQueryResult, useInternalState, useQueryWithInternalState, @@ -195,7 +196,7 @@ function executeQuery( ); const concast = internalState.observable.reobserveAsConcast( - internalState.getObsQueryOptions() + getObsQueryOptions(internalState) ); // Make sure getCurrentResult returns a fresh ApolloQueryResult, diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index 4d74a746f1a..1140314db41 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -150,7 +150,7 @@ export function useQueryWithInternalState< // 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. - internalState.observable.reobserve(internalState.getObsQueryOptions()); + internalState.observable.reobserve(getObsQueryOptions(internalState)); // Make sure getCurrentResult returns a fresh ApolloQueryResult, // but save the current data as this.previousData, just like setResult @@ -178,7 +178,7 @@ export function useQueryWithInternalState< (renderPromises && renderPromises.getSSRObservable(internalState.watchQueryOptions)) || internalState.observable || // Reuse this.observable if possible (and not SSR) - internalState.client.watchQuery(internalState.getObsQueryOptions())); + internalState.client.watchQuery(getObsQueryOptions(internalState))); internalState.obsQueryFields = React.useMemo( () => ({ @@ -260,7 +260,7 @@ export function useQueryWithInternalState< return; } - internalState.setResult(result, handleStoreChange); + setResult(result, handleStoreChange, internalState); }; const onError = (error: Error) => { @@ -278,14 +278,15 @@ export function useQueryWithInternalState< (previousResult && previousResult.loading) || !equal(error, previousResult.error) ) { - internalState.setResult( + setResult( { data: (previousResult && previousResult.data) as TData, error: error as ApolloError, loading: false, networkStatus: NetworkStatus.error, }, - handleStoreChange + handleStoreChange, + internalState ); } }; @@ -433,36 +434,6 @@ class InternalState { public queryHookOptions!: QueryHookOptions; public watchQueryOptions!: WatchQueryOptions; - public getObsQueryOptions(): WatchQueryOptions { - const toMerge: Array>> = []; - - const globalDefaults = this.client.defaultOptions.watchQuery; - if (globalDefaults) toMerge.push(globalDefaults); - - if (this.queryHookOptions.defaultOptions) { - toMerge.push(this.queryHookOptions.defaultOptions); - } - - // We use compact rather than mergeOptions for this part of the merge, - // because we want watchQueryOptions.variables (if defined) to replace - // this.observable.options.variables whole. This replacement allows - // removing variables by removing them from the variables input to - // useQuery. If the variables were always merged together (rather than - // replaced), there would be no way to remove existing variables. - // However, the variables from options.defaultOptions and globalDefaults - // (if provided) should be merged, to ensure individual defaulted - // variables always have values, if not otherwise defined in - // observable.options or watchQueryOptions. - toMerge.push( - compact( - this.observable && this.observable.options, - this.watchQueryOptions - ) - ); - - return toMerge.reduce(mergeOptions) as WatchQueryOptions; - } - // Defining these methods as no-ops on the prototype allows us to call // state.onCompleted and/or state.onError without worrying about whether a // callback was provided. @@ -479,24 +450,64 @@ class InternalState { // okay/normal for them to be initially undefined. public result: undefined | InternalQueryResult; public previousData: undefined | TData; +} - public setResult( - nextResult: ApolloQueryResult, - forceUpdate: () => void - ) { - const previousResult = this.result; - if (previousResult && previousResult.data) { - this.previousData = previousResult.data; - } - this.result = toQueryResult( - unsafeHandlePartialRefetch(nextResult, this), - this - ); - // Calling state.setResult always triggers an update, though some call sites - // perform additional equality checks before committing to an update. - forceUpdate(); - handleErrorOrCompleted(nextResult, previousResult?.[originalResult], this); +export function getObsQueryOptions< + TData, + TVariables extends OperationVariables, +>( + internalState: InternalState +): WatchQueryOptions { + const toMerge: Array>> = []; + + const globalDefaults = internalState.client.defaultOptions.watchQuery; + if (globalDefaults) toMerge.push(globalDefaults); + + if (internalState.queryHookOptions.defaultOptions) { + toMerge.push(internalState.queryHookOptions.defaultOptions); + } + + // We use compact rather than mergeOptions for this part of the merge, + // because we want watchQueryOptions.variables (if defined) to replace + // this.observable.options.variables whole. This replacement allows + // removing variables by removing them from the variables input to + // useQuery. If the variables were always merged together (rather than + // replaced), there would be no way to remove existing variables. + // However, the variables from options.defaultOptions and globalDefaults + // (if provided) should be merged, to ensure individual defaulted + // variables always have values, if not otherwise defined in + // observable.options or watchQueryOptions. + toMerge.push( + compact( + internalState.observable && internalState.observable.options, + internalState.watchQueryOptions + ) + ); + + return toMerge.reduce(mergeOptions) as WatchQueryOptions; +} + +function setResult( + nextResult: ApolloQueryResult, + forceUpdate: () => void, + internalState: InternalState +) { + const previousResult = internalState.result; + if (previousResult && previousResult.data) { + internalState.previousData = previousResult.data; } + internalState.result = toQueryResult( + unsafeHandlePartialRefetch(nextResult, internalState), + internalState + ); + // Calling state.setResult always triggers an update, though some call sites + // perform additional equality checks before committing to an update. + forceUpdate(); + handleErrorOrCompleted( + nextResult, + previousResult?.[originalResult], + internalState + ); } function handleErrorOrCompleted( @@ -535,9 +546,10 @@ function getCurrentResult( if (!internalState.result) { // WARNING: SIDE-EFFECTS IN THE RENDER FUNCTION // this could call unsafeHandlePartialRefetch - internalState.setResult( + setResult( internalState.observable.getCurrentResult(), - () => {} + () => {}, + internalState ); } return internalState.result!; From 24e4491876e44bc11d1335678c699653c88d9ba4 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Wed, 29 May 2024 17:00:29 +0200 Subject: [PATCH 11/32] replace class with single mutable object --- src/react/hooks/useQuery.ts | 85 ++++++++++++++++++------------------- 1 file changed, 42 insertions(+), 43 deletions(-) diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index 1140314db41..9c4cede7269 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -47,6 +47,29 @@ interface InternalQueryResult [originalResult]: ApolloQueryResult; } +const noop = () => {}; + +export interface InternalState { + readonly client: ReturnType; + query: DocumentNode | TypedDocumentNode; + + queryHookOptions: QueryHookOptions; + watchQueryOptions: WatchQueryOptions; + + // Defining these methods as no-ops on the prototype allows us to call + // state.onCompleted and/or state.onError without worrying about whether a + // callback was provided. + onCompleted(data: TData): void; + onError(error: ApolloError): void; + + observable: ObservableQuery; + obsQueryFields: Omit, "variables">; + // These members are populated by getCurrentResult and setResult, and it's + // okay/normal for them to be initially undefined. + result: undefined | InternalQueryResult; + previousData: undefined | TData; +} + /** * A hook for executing queries in an Apollo application. * @@ -167,9 +190,8 @@ export function useQueryWithInternalState< // Like the forceUpdate method, the versions of these methods inherited from // InternalState.prototype are empty no-ops, but we can override them on the // base state object (without modifying the prototype). - internalState.onCompleted = - options.onCompleted || InternalState.prototype.onCompleted; - internalState.onError = options.onError || InternalState.prototype.onError; + internalState.onCompleted = options.onCompleted || noop; + internalState.onError = options.onError || noop; // See if there is an existing observable that was used to fetch the same // data and if so, use it instead since it will contain the proper queryId @@ -328,7 +350,23 @@ export function useInternalState( query: DocumentNode | TypedDocumentNode ): InternalState { function createInternalState(previous?: InternalState) { - return new InternalState(client, query, previous); + verifyDocumentType(query, DocumentType.Query); + + // Reuse previousData from previous InternalState (if any) to provide + // continuity of previousData even if/when the query or client changes. + const previousResult = previous && previous.result; + const previousData = previousResult && previousResult.data; + const internalState: Partial> = { + client, + query, + onCompleted: noop, + onError: noop, + }; + if (previousData) { + internalState.previousData = previousData; + } + + return internalState as InternalState; } let [state, updateState] = React.useState(createInternalState); @@ -413,45 +451,6 @@ export function createWatchQueryOptions< return watchQueryOptions; } -export { type InternalState }; -class InternalState { - constructor( - public readonly client: ReturnType, - public query: DocumentNode | TypedDocumentNode, - previous?: InternalState - ) { - verifyDocumentType(query, DocumentType.Query); - - // Reuse previousData from previous InternalState (if any) to provide - // continuity of previousData even if/when the query or client changes. - const previousResult = previous && previous.result; - const previousData = previousResult && previousResult.data; - if (previousData) { - this.previousData = previousData; - } - } - - public queryHookOptions!: QueryHookOptions; - public watchQueryOptions!: WatchQueryOptions; - - // Defining these methods as no-ops on the prototype allows us to call - // state.onCompleted and/or state.onError without worrying about whether a - // callback was provided. - public onCompleted(data: TData) {} - public onError(error: ApolloError) {} - - public observable!: ObservableQuery; - public obsQueryFields!: Omit< - ObservableQueryFields, - "variables" - >; - - // These members are populated by getCurrentResult and setResult, and it's - // okay/normal for them to be initially undefined. - public result: undefined | InternalQueryResult; - public previousData: undefined | TData; -} - export function getObsQueryOptions< TData, TVariables extends OperationVariables, From e31d953b085c78e0d1ef5381a00a12d99099c8ee Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Wed, 29 May 2024 17:08:31 +0200 Subject: [PATCH 12/32] move callbacks into their own ref --- src/react/hooks/useQuery.ts | 70 +++++++++++++++++++++++-------------- 1 file changed, 43 insertions(+), 27 deletions(-) diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index 9c4cede7269..4683c749bd5 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -56,12 +56,6 @@ export interface InternalState { queryHookOptions: QueryHookOptions; watchQueryOptions: WatchQueryOptions; - // Defining these methods as no-ops on the prototype allows us to call - // state.onCompleted and/or state.onError without worrying about whether a - // callback was provided. - onCompleted(data: TData): void; - onError(error: ApolloError): void; - observable: ObservableQuery; obsQueryFields: Omit, "variables">; // These members are populated by getCurrentResult and setResult, and it's @@ -70,6 +64,14 @@ export interface InternalState { previousData: undefined | TData; } +interface Callbacks { + // Defining these methods as no-ops on the prototype allows us to call + // state.onCompleted and/or state.onError without worrying about whether a + // callback was provided. + onCompleted(data: TData): void; + onError(error: ApolloError): void; +} + /** * A hook for executing queries in an Apollo application. * @@ -184,14 +186,20 @@ export function useQueryWithInternalState< } } - // Make sure state.onCompleted and state.onError always reflect the latest - // options.onCompleted and options.onError callbacks provided to useQuery, - // since those functions are often recreated every time useQuery is called. - // Like the forceUpdate method, the versions of these methods inherited from - // InternalState.prototype are empty no-ops, but we can override them on the - // base state object (without modifying the prototype). - internalState.onCompleted = options.onCompleted || noop; - internalState.onError = options.onError || noop; + const _callbacks = { + onCompleted: options.onCompleted || noop, + onError: options.onError || noop, + }; + const callbackRef = React.useRef>(_callbacks); + React.useEffect(() => { + // Make sure state.onCompleted and state.onError always reflect the latest + // options.onCompleted and options.onError callbacks provided to useQuery, + // since those functions are often recreated every time useQuery is called. + // Like the forceUpdate method, the versions of these methods inherited from + // InternalState.prototype are empty no-ops, but we can override them on the + // base state object (without modifying the prototype). + callbackRef.current = _callbacks; + }); // See if there is an existing observable that was used to fetch the same // data and if so, use it instead since it will contain the proper queryId @@ -282,7 +290,12 @@ export function useQueryWithInternalState< return; } - setResult(result, handleStoreChange, internalState); + setResult( + result, + handleStoreChange, + internalState, + callbackRef.current + ); }; const onError = (error: Error) => { @@ -308,7 +321,8 @@ export function useQueryWithInternalState< networkStatus: NetworkStatus.error, }, handleStoreChange, - internalState + internalState, + callbackRef.current ); } }; @@ -338,8 +352,8 @@ export function useQueryWithInternalState< ] ), - () => getCurrentResult(internalState), - () => getCurrentResult(internalState) + () => getCurrentResult(internalState, callbackRef.current), + () => getCurrentResult(internalState, callbackRef.current) ); return result; @@ -359,8 +373,6 @@ export function useInternalState( const internalState: Partial> = { client, query, - onCompleted: noop, - onError: noop, }; if (previousData) { internalState.previousData = previousData; @@ -383,6 +395,7 @@ export function useInternalState( return state; } + // A function to massage options before passing them to ObservableQuery. export function createWatchQueryOptions< TData = any, @@ -489,7 +502,8 @@ export function getObsQueryOptions< function setResult( nextResult: ApolloQueryResult, forceUpdate: () => void, - internalState: InternalState + internalState: InternalState, + callbacks: Callbacks ) { const previousResult = internalState.result; if (previousResult && previousResult.data) { @@ -505,14 +519,14 @@ function setResult( handleErrorOrCompleted( nextResult, previousResult?.[originalResult], - internalState + callbacks ); } function handleErrorOrCompleted( result: ApolloQueryResult, previousResult: ApolloQueryResult | undefined, - internalState: InternalState + callbacks: Callbacks ) { if (!result.loading) { const error = toApolloError(result); @@ -521,13 +535,13 @@ function handleErrorOrCompleted( Promise.resolve() .then(() => { if (error) { - internalState.onError(error); + callbacks.onError(error); } else if ( result.data && previousResult?.networkStatus !== result.networkStatus && result.networkStatus === NetworkStatus.ready ) { - internalState.onCompleted(result.data); + callbacks.onCompleted(result.data); } }) .catch((error) => { @@ -537,7 +551,8 @@ function handleErrorOrCompleted( } function getCurrentResult( - internalState: InternalState + internalState: InternalState, + callbacks: Callbacks ): QueryResult { // Using this.result as a cache ensures getCurrentResult continues returning // the same (===) result object, unless state.setResult has been called, or @@ -548,7 +563,8 @@ function getCurrentResult( setResult( internalState.observable.getCurrentResult(), () => {}, - internalState + internalState, + callbacks ); } return internalState.result!; From 891e2113182542c46a09c6b17e605d24afcb2fdb Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Mon, 3 Jun 2024 15:40:32 +0200 Subject: [PATCH 13/32] move `obsQueryFields` out of `internalState` --- src/react/hooks/useLazyQuery.ts | 32 ++++++++++++++++++----------- src/react/hooks/useQuery.ts | 36 ++++++++++++++++++++------------- 2 files changed, 42 insertions(+), 26 deletions(-) diff --git a/src/react/hooks/useLazyQuery.ts b/src/react/hooks/useLazyQuery.ts index d13d7596664..c07c8ab34eb 100644 --- a/src/react/hooks/useLazyQuery.ts +++ b/src/react/hooks/useLazyQuery.ts @@ -21,10 +21,8 @@ import { getDefaultFetchPolicy, getObsQueryOptions, toQueryResult, - useInternalState, useQueryWithInternalState, } from "./useQuery.js"; -import { useApolloClient } from "./useApolloClient.js"; // The following methods, when called will execute the query, regardless of // whether the useLazyQuery execute function was called before. @@ -34,6 +32,7 @@ const EAGER_METHODS = [ "fetchMore", "updateQuery", "startPolling", + "stopPolling", "subscribeToMore", ] as const; @@ -93,12 +92,11 @@ export function useLazyQuery< optionsRef.current = options; queryRef.current = document; - const internalState = useInternalState( - useApolloClient(options && options.client), - document - ); - - const useQueryResult = useQueryWithInternalState(internalState, { + const { + internalState, + obsQueryFields, + result: useQueryResult, + } = useQueryWithInternalState(document, { ...merged, skip: !execOptionsRef.current, }); @@ -110,7 +108,6 @@ export function useLazyQuery< internalState.client.defaultOptions ); - const { obsQueryFields } = internalState; const forceUpdateState = React.useReducer((tick) => tick + 1, 0)[1]; // We use useMemo here to make sure the eager methods have a stable identity. const eagerMethods = React.useMemo(() => { @@ -128,7 +125,7 @@ export function useLazyQuery< }; } - return eagerMethods; + return eagerMethods as typeof obsQueryFields; }, [forceUpdateState, obsQueryFields]); const called = !!execOptionsRef.current; @@ -174,7 +171,16 @@ export function useLazyQuery< [eagerMethods, forceUpdateState, initialFetchPolicy, internalState] ); - return [execute, result]; + const executeRef = React.useRef(execute); + React.useLayoutEffect(() => { + executeRef.current = execute; + }); + + const stableExecute = React.useCallback( + (...args) => executeRef.current(...args), + [] + ); + return [stableExecute, result]; } function executeQuery( @@ -207,7 +213,9 @@ function executeQuery( internalState.result = void 0; forceUpdate(); - return new Promise>((resolve) => { + return new Promise< + Omit, (typeof EAGER_METHODS)[number]> + >((resolve) => { let result: ApolloQueryResult; // Subscribe to the concast independently of the ObservableQuery in case diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index 4683c749bd5..de1b7e2515c 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -43,7 +43,10 @@ const { const originalResult = Symbol(); interface InternalQueryResult - extends QueryResult { + extends Omit< + QueryResult, + Exclude, "variables"> + > { [originalResult]: ApolloQueryResult; } @@ -57,7 +60,6 @@ export interface InternalState { watchQueryOptions: WatchQueryOptions; observable: ObservableQuery; - obsQueryFields: Omit, "variables">; // These members are populated by getCurrentResult and setResult, and it's // okay/normal for them to be initially undefined. result: undefined | InternalQueryResult; @@ -130,9 +132,10 @@ function _useQuery< query: DocumentNode | TypedDocumentNode, options: QueryHookOptions, NoInfer> ) { - return useQueryWithInternalState( - useInternalState(useApolloClient(options.client), query), - options + const { result, obsQueryFields } = useQueryWithInternalState(query, options); + return React.useMemo( + () => ({ ...result, ...obsQueryFields }), + [result, obsQueryFields] ); } @@ -140,9 +143,13 @@ export function useQueryWithInternalState< TData = any, TVariables extends OperationVariables = OperationVariables, >( - internalState: InternalState, + query: DocumentNode | TypedDocumentNode, options: QueryHookOptions, NoInfer> ) { + const internalState = useInternalState( + useApolloClient(options.client), + query + ); // The renderPromises field gets initialized here in the useQuery method, at // the beginning of everything (for a given component rendering, at least), // so we can safely use this.renderPromises in other/later InternalState @@ -210,7 +217,9 @@ export function useQueryWithInternalState< internalState.observable || // Reuse this.observable if possible (and not SSR) internalState.client.watchQuery(getObsQueryOptions(internalState))); - internalState.obsQueryFields = React.useMemo( + const obsQueryFields = React.useMemo< + Omit, "variables"> + >( () => ({ refetch: obsQuery.refetch.bind(obsQuery), reobserve: obsQuery.reobserve.bind(obsQuery), @@ -356,7 +365,7 @@ export function useQueryWithInternalState< () => getCurrentResult(internalState, callbackRef.current) ); - return result; + return { result, obsQueryFields, internalState }; } export function useInternalState( @@ -553,7 +562,7 @@ function handleErrorOrCompleted( function getCurrentResult( internalState: InternalState, callbacks: Callbacks -): QueryResult { +): InternalQueryResult { // Using this.result as a cache ensures getCurrentResult continues returning // the same (===) result object, unless state.setResult has been called, or // we're doing server rendering and therefore override the result below. @@ -600,16 +609,15 @@ export function toQueryResult( const queryResult: InternalQueryResult = { data, // Ensure always defined, even if result.data is missing. ...resultWithoutPartial, - ...internalState.obsQueryFields, client: internalState.client, observable: internalState.observable, variables: internalState.observable.variables, called: !internalState.queryHookOptions.skip, previousData: internalState.previousData, - } satisfies QueryResult as InternalQueryResult< - TData, - TVariables - >; + } satisfies Omit< + InternalQueryResult, + typeof originalResult + > as InternalQueryResult; // non-enumerable property to hold the original result, for referential equality checks Object.defineProperty(queryResult, originalResult, { value: result }); From ec1c7a92d23ee7c0f541c2944fcc1c88fe986705 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Tue, 4 Jun 2024 11:06:54 +0200 Subject: [PATCH 14/32] inline `useInternalState` --- src/react/hooks/useQuery.ts | 72 ++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 41 deletions(-) diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index de1b7e2515c..caa2e9e69b9 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -146,10 +146,37 @@ export function useQueryWithInternalState< query: DocumentNode | TypedDocumentNode, options: QueryHookOptions, NoInfer> ) { - const internalState = useInternalState( - useApolloClient(options.client), - query - ); + const client = useApolloClient(options.client); + function createInternalState(previous?: InternalState) { + verifyDocumentType(query, DocumentType.Query); + + // Reuse previousData from previous InternalState (if any) to provide + // continuity of previousData even if/when the query or client changes. + const previousResult = previous && previous.result; + const previousData = previousResult && previousResult.data; + const internalState: Partial> = { + client, + query, + }; + if (previousData) { + internalState.previousData = previousData; + } + + return internalState as InternalState; + } + + let [internalState, updateState] = React.useState(createInternalState); + + if (client !== internalState.client || query !== internalState.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 + // to run the current render function to completion. + // Since we sometimes trigger some side-effects in the render function, we + // re-assign `state` to the new state to ensure that those side-effects are + // triggered with the new state. + updateState((internalState = createInternalState(internalState))); + } + // The renderPromises field gets initialized here in the useQuery method, at // the beginning of everything (for a given component rendering, at least), // so we can safely use this.renderPromises in other/later InternalState @@ -368,43 +395,6 @@ export function useQueryWithInternalState< return { result, obsQueryFields, internalState }; } -export function useInternalState( - client: ApolloClient, - query: DocumentNode | TypedDocumentNode -): InternalState { - function createInternalState(previous?: InternalState) { - verifyDocumentType(query, DocumentType.Query); - - // Reuse previousData from previous InternalState (if any) to provide - // continuity of previousData even if/when the query or client changes. - const previousResult = previous && previous.result; - const previousData = previousResult && previousResult.data; - const internalState: Partial> = { - client, - query, - }; - if (previousData) { - internalState.previousData = previousData; - } - - return internalState as InternalState; - } - - let [state, updateState] = React.useState(createInternalState); - - 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 - // to run the current render function to completion. - // Since we sometimes trigger some side-effects in the render function, we - // re-assign `state` to the new state to ensure that those side-effects are - // triggered with the new state. - updateState((state = createInternalState(state))); - } - - return state; -} - // A function to massage options before passing them to ObservableQuery. export function createWatchQueryOptions< TData = any, From 79566a80825a5bcce7152d3b7fdacca4fd6a056f Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Tue, 4 Jun 2024 13:02:01 +0200 Subject: [PATCH 15/32] redactor away `internalState.queryHookOptions` --- src/react/hooks/useLazyQuery.ts | 15 ++++---- src/react/hooks/useQuery.ts | 64 ++++++++++++++++++--------------- 2 files changed, 44 insertions(+), 35 deletions(-) diff --git a/src/react/hooks/useLazyQuery.ts b/src/react/hooks/useLazyQuery.ts index c07c8ab34eb..cce41ebf3cb 100644 --- a/src/react/hooks/useLazyQuery.ts +++ b/src/react/hooks/useLazyQuery.ts @@ -92,19 +92,20 @@ export function useLazyQuery< optionsRef.current = options; queryRef.current = document; + const queryHookOptions = { + ...merged, + skip: !execOptionsRef.current, + }; const { internalState, obsQueryFields, result: useQueryResult, - } = useQueryWithInternalState(document, { - ...merged, - skip: !execOptionsRef.current, - }); + } = useQueryWithInternalState(document, queryHookOptions); const initialFetchPolicy = useQueryResult.observable.options.initialFetchPolicy || getDefaultFetchPolicy( - internalState.queryHookOptions.defaultOptions, + queryHookOptions.defaultOptions, internalState.client.defaultOptions ); @@ -196,13 +197,13 @@ function executeQuery( } internalState.watchQueryOptions = createWatchQueryOptions( - (internalState.queryHookOptions = options), + options, internalState, hasRenderPromises ); const concast = internalState.observable.reobserveAsConcast( - getObsQueryOptions(internalState) + getObsQueryOptions(internalState, options) ); // Make sure getCurrentResult returns a fresh ApolloQueryResult, diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index caa2e9e69b9..6a4098f3116 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -13,7 +13,6 @@ import { mergeOptions } from "../../utilities/index.js"; import { getApolloContext } from "../context/index.js"; import { ApolloError } from "../../errors/index.js"; import type { - ApolloClient, ApolloQueryResult, ObservableQuery, DocumentNode, @@ -56,7 +55,6 @@ export interface InternalState { readonly client: ReturnType; query: DocumentNode | TypedDocumentNode; - queryHookOptions: QueryHookOptions; watchQueryOptions: WatchQueryOptions; observable: ObservableQuery; @@ -187,7 +185,7 @@ export function useQueryWithInternalState< const renderPromises = React.useContext(getApolloContext()).renderPromises; const watchQueryOptions = createWatchQueryOptions( - (internalState.queryHookOptions = options), + options, internalState, !!renderPromises ); @@ -209,7 +207,9 @@ export function useQueryWithInternalState< // 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. - internalState.observable.reobserve(getObsQueryOptions(internalState)); + internalState.observable.reobserve( + getObsQueryOptions(internalState, options) + ); // Make sure getCurrentResult returns a fresh ApolloQueryResult, // but save the current data as this.previousData, just like setResult @@ -242,7 +242,9 @@ export function useQueryWithInternalState< (renderPromises && renderPromises.getSSRObservable(internalState.watchQueryOptions)) || internalState.observable || // Reuse this.observable if possible (and not SSR) - internalState.client.watchQuery(getObsQueryOptions(internalState))); + internalState.client.watchQuery( + getObsQueryOptions(internalState, options) + )); const obsQueryFields = React.useMemo< Omit, "variables"> @@ -261,14 +263,14 @@ export function useQueryWithInternalState< if ( (renderPromises || internalState.client.disableNetworkFetches) && - internalState.queryHookOptions.ssr === false && - !internalState.queryHookOptions.skip + options.ssr === false && + !options.skip ) { // If SSR has been explicitly disabled, and this function has been called // on the server side, return the default loading state. internalState.result = toQueryResult(ssrDisabledResult, internalState); } else if ( - internalState.queryHookOptions.skip || + options.skip || internalState.watchQueryOptions.fetchPolicy === "standby" ) { // When skipping a query (ie. we're not querying for data but still want to @@ -289,10 +291,7 @@ export function useQueryWithInternalState< internalState.result = void 0; } - const ssrAllowed = !( - internalState.queryHookOptions.ssr === false || - internalState.queryHookOptions.skip - ); + const ssrAllowed = !(options.ssr === false || options.skip); if (renderPromises && ssrAllowed) { renderPromises.registerSSRObservable(obsQuery); @@ -303,6 +302,7 @@ export function useQueryWithInternalState< } } + const partialRefetch = options.partialRefetch; const result = useSyncExternalStore( React.useCallback( (handleStoreChange) => { @@ -330,7 +330,8 @@ export function useQueryWithInternalState< result, handleStoreChange, internalState, - callbackRef.current + callbackRef.current, + partialRefetch ); }; @@ -358,7 +359,8 @@ export function useQueryWithInternalState< }, handleStoreChange, internalState, - callbackRef.current + callbackRef.current, + partialRefetch ); } }; @@ -385,11 +387,12 @@ export function useQueryWithInternalState< obsQuery, renderPromises, internalState.client.disableNetworkFetches, + partialRefetch, ] ), - () => getCurrentResult(internalState, callbackRef.current), - () => getCurrentResult(internalState, callbackRef.current) + () => getCurrentResult(internalState, callbackRef.current, partialRefetch), + () => getCurrentResult(internalState, callbackRef.current, partialRefetch) ); return { result, obsQueryFields, internalState }; @@ -438,7 +441,7 @@ export function createWatchQueryOptions< if (skip) { const { fetchPolicy = getDefaultFetchPolicy( - internalState.queryHookOptions.defaultOptions, + defaultOptions, internalState.client.defaultOptions ), initialFetchPolicy = fetchPolicy, @@ -455,7 +458,7 @@ export function createWatchQueryOptions< watchQueryOptions.fetchPolicy = internalState.observable?.options.initialFetchPolicy || getDefaultFetchPolicy( - internalState.queryHookOptions.defaultOptions, + defaultOptions, internalState.client.defaultOptions ); } @@ -467,15 +470,16 @@ export function getObsQueryOptions< TData, TVariables extends OperationVariables, >( - internalState: InternalState + internalState: InternalState, + queryHookOptions: QueryHookOptions ): WatchQueryOptions { const toMerge: Array>> = []; const globalDefaults = internalState.client.defaultOptions.watchQuery; if (globalDefaults) toMerge.push(globalDefaults); - if (internalState.queryHookOptions.defaultOptions) { - toMerge.push(internalState.queryHookOptions.defaultOptions); + if (queryHookOptions.defaultOptions) { + toMerge.push(queryHookOptions.defaultOptions); } // We use compact rather than mergeOptions for this part of the merge, @@ -502,14 +506,15 @@ function setResult( nextResult: ApolloQueryResult, forceUpdate: () => void, internalState: InternalState, - callbacks: Callbacks + callbacks: Callbacks, + partialRefetch: boolean | undefined ) { const previousResult = internalState.result; if (previousResult && previousResult.data) { internalState.previousData = previousResult.data; } internalState.result = toQueryResult( - unsafeHandlePartialRefetch(nextResult, internalState), + unsafeHandlePartialRefetch(nextResult, internalState, partialRefetch), internalState ); // Calling state.setResult always triggers an update, though some call sites @@ -551,7 +556,8 @@ function handleErrorOrCompleted( function getCurrentResult( internalState: InternalState, - callbacks: Callbacks + callbacks: Callbacks, + partialRefetch: boolean | undefined ): InternalQueryResult { // Using this.result as a cache ensures getCurrentResult continues returning // the same (===) result object, unless state.setResult has been called, or @@ -563,7 +569,8 @@ function getCurrentResult( internalState.observable.getCurrentResult(), () => {}, internalState, - callbacks + callbacks, + partialRefetch ); } return internalState.result!; @@ -602,7 +609,7 @@ export function toQueryResult( client: internalState.client, observable: internalState.observable, variables: internalState.observable.variables, - called: !internalState.queryHookOptions.skip, + called: result !== ssrDisabledResult && result !== skipStandbyResult, previousData: internalState.previousData, } satisfies Omit< InternalQueryResult, @@ -627,14 +634,15 @@ function unsafeHandlePartialRefetch< TVariables extends OperationVariables, >( result: ApolloQueryResult, - internalState: InternalState + internalState: InternalState, + partialRefetch: boolean | undefined ): ApolloQueryResult { // TODO: This code should be removed when the partialRefetch option is // removed. I was unable to get this hook to behave reasonably in certain // edge cases when this block was put in an effect. if ( result.partial && - internalState.queryHookOptions.partialRefetch && + partialRefetch && !result.loading && (!result.data || Object.keys(result.data).length === 0) && internalState.observable.options.fetchPolicy !== "cache-only" From 873f24d0688a6ebf65f6a1e3e787b11f8124638d Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Tue, 4 Jun 2024 14:17:44 +0200 Subject: [PATCH 16/32] make function arguments more explicit --- src/react/hooks/useLazyQuery.ts | 13 +++-- src/react/hooks/useQuery.ts | 87 ++++++++++++++++++--------------- 2 files changed, 57 insertions(+), 43 deletions(-) diff --git a/src/react/hooks/useLazyQuery.ts b/src/react/hooks/useLazyQuery.ts index cce41ebf3cb..1bf7c4007cd 100644 --- a/src/react/hooks/useLazyQuery.ts +++ b/src/react/hooks/useLazyQuery.ts @@ -197,13 +197,20 @@ function executeQuery( } internalState.watchQueryOptions = createWatchQueryOptions( + internalState.client, + internalState.query, options, - internalState, - hasRenderPromises + hasRenderPromises, + internalState.observable ); const concast = internalState.observable.reobserveAsConcast( - getObsQueryOptions(internalState, options) + getObsQueryOptions( + internalState.client, + options, + internalState.watchQueryOptions, + internalState.observable + ) ); // Make sure getCurrentResult returns a fresh ApolloQueryResult, diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index 6a4098f3116..fb397bbe743 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -5,6 +5,7 @@ import { useSyncExternalStore } from "./useSyncExternalStore.js"; import { equal } from "@wry/equality"; import type { + ApolloClient, DefaultOptions, OperationVariables, WatchQueryFetchPolicy, @@ -184,11 +185,14 @@ export function useQueryWithInternalState< // rather than left uninitialized. const renderPromises = React.useContext(getApolloContext()).renderPromises; - const watchQueryOptions = createWatchQueryOptions( - options, - internalState, - !!renderPromises - ); + const watchQueryOptions: Readonly> = + createWatchQueryOptions( + internalState.client, + internalState.query, + options, + !!renderPromises, + internalState.observable + ); // Update this.watchQueryOptions, but only when they have changed, which // allows us to depend on the referential stability of @@ -208,7 +212,12 @@ export function useQueryWithInternalState< // (potentially) kicks off a network request (for example, when the // variables have changed), which is technically a side-effect. internalState.observable.reobserve( - getObsQueryOptions(internalState, options) + getObsQueryOptions( + internalState.client, + options, + internalState.watchQueryOptions, + internalState.observable + ) ); // Make sure getCurrentResult returns a fresh ApolloQueryResult, @@ -243,7 +252,12 @@ export function useQueryWithInternalState< renderPromises.getSSRObservable(internalState.watchQueryOptions)) || internalState.observable || // Reuse this.observable if possible (and not SSR) internalState.client.watchQuery( - getObsQueryOptions(internalState, options) + getObsQueryOptions( + internalState.client, + options, + internalState.watchQueryOptions, + undefined + ) )); const obsQueryFields = React.useMemo< @@ -403,6 +417,8 @@ export function createWatchQueryOptions< TData = any, TVariables extends OperationVariables = OperationVariables, >( + client: ApolloClient, + query: DocumentNode | TypedDocumentNode, { skip, ssr, @@ -414,14 +430,14 @@ export function createWatchQueryOptions< // query property that we add below. ...otherOptions }: QueryHookOptions = {}, - internalState: InternalState, - hasRenderPromises: boolean + hasRenderPromises: boolean, + observable: ObservableQuery | undefined ): WatchQueryOptions { // This Object.assign is safe because otherOptions is a fresh ...rest object // that did not exist until just now, so modifications are still allowed. const watchQueryOptions: WatchQueryOptions = Object.assign( otherOptions, - { query: internalState.query } + { query } ); if ( @@ -439,28 +455,18 @@ export function createWatchQueryOptions< } if (skip) { - const { - fetchPolicy = getDefaultFetchPolicy( - defaultOptions, - internalState.client.defaultOptions - ), - initialFetchPolicy = fetchPolicy, - } = watchQueryOptions; - // When skipping, we set watchQueryOptions.fetchPolicy initially to // "standby", but we also need/want to preserve the initial non-standby // fetchPolicy that would have been used if not skipping. - Object.assign(watchQueryOptions, { - initialFetchPolicy, - fetchPolicy: "standby", - }); + watchQueryOptions.initialFetchPolicy = + watchQueryOptions.initialFetchPolicy || + watchQueryOptions.fetchPolicy || + getDefaultFetchPolicy(defaultOptions, client.defaultOptions); + watchQueryOptions.fetchPolicy = "standby"; } else if (!watchQueryOptions.fetchPolicy) { watchQueryOptions.fetchPolicy = - internalState.observable?.options.initialFetchPolicy || - getDefaultFetchPolicy( - defaultOptions, - internalState.client.defaultOptions - ); + observable?.options.initialFetchPolicy || + getDefaultFetchPolicy(defaultOptions, client.defaultOptions); } return watchQueryOptions; @@ -470,12 +476,14 @@ export function getObsQueryOptions< TData, TVariables extends OperationVariables, >( - internalState: InternalState, - queryHookOptions: QueryHookOptions + client: ApolloClient, + queryHookOptions: QueryHookOptions, + watchQueryOptions: Partial>, + observable: ObservableQuery | undefined ): WatchQueryOptions { const toMerge: Array>> = []; - const globalDefaults = internalState.client.defaultOptions.watchQuery; + const globalDefaults = client.defaultOptions.watchQuery; if (globalDefaults) toMerge.push(globalDefaults); if (queryHookOptions.defaultOptions) { @@ -492,12 +500,7 @@ export function getObsQueryOptions< // (if provided) should be merged, to ensure individual defaulted // variables always have values, if not otherwise defined in // observable.options or watchQueryOptions. - toMerge.push( - compact( - internalState.observable && internalState.observable.options, - internalState.watchQueryOptions - ) - ); + toMerge.push(compact(observable && observable.options, watchQueryOptions)); return toMerge.reduce(mergeOptions) as WatchQueryOptions; } @@ -514,7 +517,11 @@ function setResult( internalState.previousData = previousResult.data; } internalState.result = toQueryResult( - unsafeHandlePartialRefetch(nextResult, internalState, partialRefetch), + unsafeHandlePartialRefetch( + nextResult, + internalState.observable, + partialRefetch + ), internalState ); // Calling state.setResult always triggers an update, though some call sites @@ -634,7 +641,7 @@ function unsafeHandlePartialRefetch< TVariables extends OperationVariables, >( result: ApolloQueryResult, - internalState: InternalState, + observable: ObservableQuery, partialRefetch: boolean | undefined ): ApolloQueryResult { // TODO: This code should be removed when the partialRefetch option is @@ -645,9 +652,9 @@ function unsafeHandlePartialRefetch< partialRefetch && !result.loading && (!result.data || Object.keys(result.data).length === 0) && - internalState.observable.options.fetchPolicy !== "cache-only" + observable.options.fetchPolicy !== "cache-only" ) { - internalState.observable.refetch(); + observable.refetch(); return { ...result, loading: true, From 8bb445c7ff6aac518b39b701ac73ede16ea70044 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Wed, 5 Jun 2024 12:43:40 +0200 Subject: [PATCH 17/32] replace `internalState.watchQueryOptions` with `observable[lastWatchOptions]` --- src/react/hooks/useLazyQuery.ts | 6 ++-- src/react/hooks/useQuery.ts | 64 ++++++++++++++++----------------- 2 files changed, 36 insertions(+), 34 deletions(-) diff --git a/src/react/hooks/useLazyQuery.ts b/src/react/hooks/useLazyQuery.ts index 1bf7c4007cd..d9107569e8f 100644 --- a/src/react/hooks/useLazyQuery.ts +++ b/src/react/hooks/useLazyQuery.ts @@ -20,6 +20,7 @@ import { createWatchQueryOptions, getDefaultFetchPolicy, getObsQueryOptions, + lastWatchOptions, toQueryResult, useQueryWithInternalState, } from "./useQuery.js"; @@ -196,7 +197,7 @@ function executeQuery( internalState.query = options.query; } - internalState.watchQueryOptions = createWatchQueryOptions( + const watchQueryOptions = createWatchQueryOptions( internalState.client, internalState.query, options, @@ -208,10 +209,11 @@ function executeQuery( getObsQueryOptions( internalState.client, options, - internalState.watchQueryOptions, + watchQueryOptions, internalState.observable ) ); + internalState.observable[lastWatchOptions] = watchQueryOptions; // Make sure getCurrentResult returns a fresh ApolloQueryResult, // but save the current data as this.previousData, just like setResult diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index fb397bbe743..3178029cbe4 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -51,14 +51,17 @@ interface InternalQueryResult } const noop = () => {}; +export const lastWatchOptions = Symbol(); +interface ObsQueryWithMeta + extends ObservableQuery { + [lastWatchOptions]?: WatchQueryOptions; +} export interface InternalState { readonly client: ReturnType; query: DocumentNode | TypedDocumentNode; - watchQueryOptions: WatchQueryOptions; - - observable: ObservableQuery; + observable: ObsQueryWithMeta; // These members are populated by getCurrentResult and setResult, and it's // okay/normal for them to be initially undefined. result: undefined | InternalQueryResult; @@ -194,15 +197,30 @@ export function useQueryWithInternalState< internalState.observable ); - // Update this.watchQueryOptions, but only when they have changed, which - // allows us to depend on the referential stability of - // this.watchQueryOptions elsewhere. - const currentWatchQueryOptions = internalState.watchQueryOptions; - - if (!equal(watchQueryOptions, currentWatchQueryOptions)) { - internalState.watchQueryOptions = watchQueryOptions; + // See if there is an existing observable that was used to fetch the same + // data and if so, use it instead since it will contain the proper queryId + // to fetch the result set. This is used during SSR. + const obsQuery: ObsQueryWithMeta = + (internalState.observable = + (renderPromises && renderPromises.getSSRObservable(watchQueryOptions)) || + internalState.observable || // Reuse this.observable if possible (and not SSR) + internalState.client.watchQuery( + getObsQueryOptions( + internalState.client, + options, + watchQueryOptions, + undefined + ) + )); - if (currentWatchQueryOptions && internalState.observable) { + // TODO: this part is not compatible with any rules of React, and there's + // no good way to rewrite it. + // it should be moved out into a separate hook that will not be optimized by the compiler + { + if ( + obsQuery[lastWatchOptions] && + !equal(obsQuery[lastWatchOptions], watchQueryOptions) + ) { // Though it might be tempting to postpone this reobserve call to the // useEffect block, we need getCurrentResult to return an appropriate // loading:true result synchronously (later within the same call to @@ -215,7 +233,7 @@ export function useQueryWithInternalState< getObsQueryOptions( internalState.client, options, - internalState.watchQueryOptions, + watchQueryOptions, internalState.observable ) ); @@ -227,6 +245,7 @@ export function useQueryWithInternalState< internalState.result?.data || internalState.previousData; internalState.result = void 0; } + obsQuery[lastWatchOptions] = watchQueryOptions; } const _callbacks = { @@ -244,22 +263,6 @@ export function useQueryWithInternalState< callbackRef.current = _callbacks; }); - // See if there is an existing observable that was used to fetch the same - // data and if so, use it instead since it will contain the proper queryId - // to fetch the result set. This is used during SSR. - const obsQuery = (internalState.observable = - (renderPromises && - renderPromises.getSSRObservable(internalState.watchQueryOptions)) || - internalState.observable || // Reuse this.observable if possible (and not SSR) - internalState.client.watchQuery( - getObsQueryOptions( - internalState.client, - options, - internalState.watchQueryOptions, - undefined - ) - )); - const obsQueryFields = React.useMemo< Omit, "variables"> >( @@ -283,10 +286,7 @@ export function useQueryWithInternalState< // If SSR has been explicitly disabled, and this function has been called // on the server side, return the default loading state. internalState.result = toQueryResult(ssrDisabledResult, internalState); - } else if ( - options.skip || - internalState.watchQueryOptions.fetchPolicy === "standby" - ) { + } else if (options.skip || watchQueryOptions.fetchPolicy === "standby") { // When skipping a query (ie. we're not querying for data but still want to // render children), make sure the `data` is cleared out and `loading` is // set to `false` (since we aren't loading anything). From 635a32ba1e35b312bee15ee589540c1fccd34d37 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Wed, 5 Jun 2024 14:36:38 +0200 Subject: [PATCH 18/32] move observable fully into a readonly prop on internalState --- src/react/hooks/useQuery.ts | 139 ++++++++++++++++++------------------ 1 file changed, 68 insertions(+), 71 deletions(-) diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index 3178029cbe4..9441b67e4c9 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -61,11 +61,11 @@ export interface InternalState { readonly client: ReturnType; query: DocumentNode | TypedDocumentNode; - observable: ObsQueryWithMeta; + readonly observable: ObsQueryWithMeta; // These members are populated by getCurrentResult and setResult, and it's // okay/normal for them to be initially undefined. - result: undefined | InternalQueryResult; - previousData: undefined | TData; + result?: undefined | InternalQueryResult; + previousData?: undefined | TData; } interface Callbacks { @@ -149,25 +149,57 @@ export function useQueryWithInternalState< options: QueryHookOptions, NoInfer> ) { const client = useApolloClient(options.client); + + // The renderPromises field gets initialized here in the useQuery method, at + // the beginning of everything (for a given component rendering, at least), + // so we can safely use this.renderPromises in other/later InternalState + // methods without worrying it might be uninitialized. Even after + // initialization, this.renderPromises is usually undefined (unless SSR is + // happening), but that's fine as long as it has been initialized that way, + // rather than left uninitialized. + const renderPromises = React.useContext(getApolloContext()).renderPromises; + + const makeWatchQueryOptions = ( + observable?: ObservableQuery + ) => + createWatchQueryOptions( + client, + query, + options, + !!renderPromises, + observable + ); + function createInternalState(previous?: InternalState) { verifyDocumentType(query, DocumentType.Query); - // Reuse previousData from previous InternalState (if any) to provide - // continuity of previousData even if/when the query or client changes. - const previousResult = previous && previous.result; - const previousData = previousResult && previousResult.data; - const internalState: Partial> = { + const internalState: InternalState = { client, query, + observable: + // See if there is an existing observable that was used to fetch the same + // data and if so, use it instead since it will contain the proper queryId + // to fetch the result set. This is used during SSR. + (renderPromises && + renderPromises.getSSRObservable(makeWatchQueryOptions())) || + client.watchQuery( + getObsQueryOptions( + client, + options, + makeWatchQueryOptions(), + undefined + ) + ), + // Reuse previousData from previous InternalState (if any) to provide + // continuity of previousData even if/when the query or client changes. + previousData: previous?.result?.data, }; - if (previousData) { - internalState.previousData = previousData; - } return internalState as InternalState; } - let [internalState, updateState] = React.useState(createInternalState); + let [internalState, updateInternalState] = + React.useState(createInternalState); if (client !== internalState.client || query !== internalState.query) { // If the client or query have changed, we need to create a new InternalState. @@ -176,50 +208,20 @@ export function useQueryWithInternalState< // Since we sometimes trigger some side-effects in the render function, we // re-assign `state` to the new state to ensure that those side-effects are // triggered with the new state. - updateState((internalState = createInternalState(internalState))); + updateInternalState((internalState = createInternalState(internalState))); } - // The renderPromises field gets initialized here in the useQuery method, at - // the beginning of everything (for a given component rendering, at least), - // so we can safely use this.renderPromises in other/later InternalState - // methods without worrying it might be uninitialized. Even after - // initialization, this.renderPromises is usually undefined (unless SSR is - // happening), but that's fine as long as it has been initialized that way, - // rather than left uninitialized. - const renderPromises = React.useContext(getApolloContext()).renderPromises; - + const observable = internalState.observable; const watchQueryOptions: Readonly> = - createWatchQueryOptions( - internalState.client, - internalState.query, - options, - !!renderPromises, - internalState.observable - ); - - // See if there is an existing observable that was used to fetch the same - // data and if so, use it instead since it will contain the proper queryId - // to fetch the result set. This is used during SSR. - const obsQuery: ObsQueryWithMeta = - (internalState.observable = - (renderPromises && renderPromises.getSSRObservable(watchQueryOptions)) || - internalState.observable || // Reuse this.observable if possible (and not SSR) - internalState.client.watchQuery( - getObsQueryOptions( - internalState.client, - options, - watchQueryOptions, - undefined - ) - )); + makeWatchQueryOptions(observable); // TODO: this part is not compatible with any rules of React, and there's // no good way to rewrite it. // it should be moved out into a separate hook that will not be optimized by the compiler { if ( - obsQuery[lastWatchOptions] && - !equal(obsQuery[lastWatchOptions], watchQueryOptions) + observable[lastWatchOptions] && + !equal(observable[lastWatchOptions], watchQueryOptions) ) { // Though it might be tempting to postpone this reobserve call to the // useEffect block, we need getCurrentResult to return an appropriate @@ -229,13 +231,8 @@ export function useQueryWithInternalState< // 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. - internalState.observable.reobserve( - getObsQueryOptions( - internalState.client, - options, - watchQueryOptions, - internalState.observable - ) + observable.reobserve( + getObsQueryOptions(client, options, watchQueryOptions, observable) ); // Make sure getCurrentResult returns a fresh ApolloQueryResult, @@ -245,7 +242,7 @@ export function useQueryWithInternalState< internalState.result?.data || internalState.previousData; internalState.result = void 0; } - obsQuery[lastWatchOptions] = watchQueryOptions; + observable[lastWatchOptions] = watchQueryOptions; } const _callbacks = { @@ -267,19 +264,19 @@ export function useQueryWithInternalState< Omit, "variables"> >( () => ({ - refetch: obsQuery.refetch.bind(obsQuery), - reobserve: obsQuery.reobserve.bind(obsQuery), - fetchMore: obsQuery.fetchMore.bind(obsQuery), - updateQuery: obsQuery.updateQuery.bind(obsQuery), - startPolling: obsQuery.startPolling.bind(obsQuery), - stopPolling: obsQuery.stopPolling.bind(obsQuery), - subscribeToMore: obsQuery.subscribeToMore.bind(obsQuery), + refetch: observable.refetch.bind(observable), + reobserve: observable.reobserve.bind(observable), + fetchMore: observable.fetchMore.bind(observable), + updateQuery: observable.updateQuery.bind(observable), + startPolling: observable.startPolling.bind(observable), + stopPolling: observable.stopPolling.bind(observable), + subscribeToMore: observable.subscribeToMore.bind(observable), }), - [obsQuery] + [observable] ); if ( - (renderPromises || internalState.client.disableNetworkFetches) && + (renderPromises || client.disableNetworkFetches) && options.ssr === false && !options.skip ) { @@ -308,11 +305,11 @@ export function useQueryWithInternalState< const ssrAllowed = !(options.ssr === false || options.skip); if (renderPromises && ssrAllowed) { - renderPromises.registerSSRObservable(obsQuery); + renderPromises.registerSSRObservable(observable); - if (obsQuery.getCurrentResult().loading) { + if (observable.getCurrentResult().loading) { // TODO: This is a legacy API which could probably be cleaned up - renderPromises.addObservableQueryPromise(obsQuery); + renderPromises.addObservableQueryPromise(observable); } } @@ -329,7 +326,7 @@ export function useQueryWithInternalState< // We use `getCurrentResult()` instead of the onNext argument because // the values differ slightly. Specifically, loading results will have // an empty object for data instead of `undefined` for some reason. - const result = obsQuery.getCurrentResult(); + const result = observable.getCurrentResult(); // Make sure we're not attempting to re-render similar results if ( previousResult && @@ -351,7 +348,7 @@ export function useQueryWithInternalState< const onError = (error: Error) => { subscription.unsubscribe(); - subscription = obsQuery.resubscribeAfterError(onNext, onError); + subscription = observable.resubscribeAfterError(onNext, onError); if (!hasOwnProperty.call(error, "graphQLErrors")) { // The error is not a GraphQL error @@ -379,7 +376,7 @@ export function useQueryWithInternalState< } }; - let subscription = obsQuery.subscribe(onNext, onError); + let subscription = observable.subscribe(onNext, onError); // Do the "unsubscribe" with a short delay. // This way, an existing subscription can be reused without an additional @@ -398,7 +395,7 @@ export function useQueryWithInternalState< // useEffect ultimately responsible for the subscription, so we are // effectively passing this dependency array to that useEffect buried // inside useSyncExternalStore, as desired. - obsQuery, + observable, renderPromises, internalState.client.disableNetworkFetches, partialRefetch, From d6de17f7911e5ddf169f96b2d6b6688f42a8c28e Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Wed, 5 Jun 2024 17:24:40 +0200 Subject: [PATCH 19/32] remove all direct access to `internalState` after initializing --- src/react/hooks/useLazyQuery.ts | 93 ++++++++++------- src/react/hooks/useQuery.ts | 174 +++++++++++++++++++++----------- 2 files changed, 171 insertions(+), 96 deletions(-) diff --git a/src/react/hooks/useLazyQuery.ts b/src/react/hooks/useLazyQuery.ts index d9107569e8f..6f2e74c0663 100644 --- a/src/react/hooks/useLazyQuery.ts +++ b/src/react/hooks/useLazyQuery.ts @@ -3,6 +3,7 @@ import type { TypedDocumentNode } from "@graphql-typed-document-node/core"; import * as React from "rehackt"; import type { + ApolloClient, ApolloQueryResult, OperationVariables, } from "../../core/index.js"; @@ -15,7 +16,11 @@ import type { QueryHookOptions, QueryResult, } from "../types/types.js"; -import type { InternalState } from "./useQuery.js"; +import type { + InternalResult, + ObsQueryWithMeta, + UpdateInternalState, +} from "./useQuery.js"; import { createWatchQueryOptions, getDefaultFetchPolicy, @@ -98,16 +103,19 @@ export function useLazyQuery< skip: !execOptionsRef.current, }; const { - internalState, obsQueryFields, result: useQueryResult, + client, + resultData, + observable, + updateInternalState, } = useQueryWithInternalState(document, queryHookOptions); const initialFetchPolicy = - useQueryResult.observable.options.initialFetchPolicy || + observable.options.initialFetchPolicy || getDefaultFetchPolicy( queryHookOptions.defaultOptions, - internalState.client.defaultOptions + client.defaultOptions ); const forceUpdateState = React.useReducer((tick) => tick + 1, 0)[1]; @@ -160,8 +168,11 @@ export function useLazyQuery< const promise = executeQuery( { ...options, skip: false }, false, - internalState, - forceUpdateState + document, + resultData, + observable, + client, + updateInternalState ).then((queryResult) => Object.assign(queryResult, eagerMethods)); // Because the return value of `useLazyQuery` is usually floated, we need @@ -170,7 +181,15 @@ export function useLazyQuery< return promise; }, - [eagerMethods, forceUpdateState, initialFetchPolicy, internalState] + [ + client, + document, + eagerMethods, + initialFetchPolicy, + observable, + resultData, + updateInternalState, + ] ); const executeRef = React.useRef(execute); @@ -190,38 +209,40 @@ function executeQuery( query?: DocumentNode; }, hasRenderPromises: boolean, - internalState: InternalState, - forceUpdate: () => void + currentQuery: DocumentNode, + resultData: InternalResult, + observable: ObsQueryWithMeta, + client: ApolloClient, + updateInternalState: UpdateInternalState ) { - if (options.query) { - internalState.query = options.query; - } - + const query = options.query || currentQuery; const watchQueryOptions = createWatchQueryOptions( - internalState.client, - internalState.query, + client, + query, options, hasRenderPromises, - internalState.observable + observable ); - const concast = internalState.observable.reobserveAsConcast( - getObsQueryOptions( - internalState.client, - options, - watchQueryOptions, - internalState.observable - ) + const concast = observable.reobserveAsConcast( + getObsQueryOptions(client, options, watchQueryOptions, observable) ); - internalState.observable[lastWatchOptions] = watchQueryOptions; - - // Make sure getCurrentResult returns a fresh ApolloQueryResult, - // but save the current data as this.previousData, just like setResult - // usually does. - internalState.previousData = - internalState.result?.data || internalState.previousData; - internalState.result = void 0; - forceUpdate(); + // this needs to be set to prevent an immediate `resubscribe` in the + // next rerender of the `useQuery` internals + observable[lastWatchOptions] = watchQueryOptions; + updateInternalState({ + client, + observable, + // might be a different query + query, + resultData: Object.assign(resultData, { + // Make sure getCurrentResult returns a fresh ApolloQueryResult, + // but save the current data as this.previousData, just like setResult + // usually does. + previousData: resultData.current?.data || resultData.previousData, + current: undefined, + }), + }); return new Promise< Omit, (typeof EAGER_METHODS)[number]> @@ -239,13 +260,15 @@ function executeQuery( error: () => { resolve( toQueryResult( - internalState.observable.getCurrentResult(), - internalState + observable.getCurrentResult(), + resultData, + observable, + client ) ); }, complete: () => { - resolve(toQueryResult(result, internalState)); + resolve(toQueryResult(result, resultData, observable, client)); }, }); }); diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index 9441b67e4c9..54055b62799 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -53,21 +53,30 @@ interface InternalQueryResult const noop = () => {}; export const lastWatchOptions = Symbol(); -interface ObsQueryWithMeta +export interface ObsQueryWithMeta extends ObservableQuery { [lastWatchOptions]?: WatchQueryOptions; } -export interface InternalState { - readonly client: ReturnType; - query: DocumentNode | TypedDocumentNode; - readonly observable: ObsQueryWithMeta; +export interface InternalResult { // These members are populated by getCurrentResult and setResult, and it's // okay/normal for them to be initially undefined. - result?: undefined | InternalQueryResult; + current?: undefined | InternalQueryResult; previousData?: undefined | TData; } +interface InternalState { + client: ReturnType; + query: DocumentNode | TypedDocumentNode; + observable: ObsQueryWithMeta; + resultData: InternalResult; +} + +export type UpdateInternalState< + TData, + TVariables extends OperationVariables, +> = (state: InternalState) => void; + interface Callbacks { // Defining these methods as no-ops on the prototype allows us to call // state.onCompleted and/or state.onError without worrying about whether a @@ -190,9 +199,11 @@ export function useQueryWithInternalState< undefined ) ), - // Reuse previousData from previous InternalState (if any) to provide - // continuity of previousData even if/when the query or client changes. - previousData: previous?.result?.data, + resultData: { + // Reuse previousData from previous InternalState (if any) to provide + // continuity of previousData even if/when the query or client changes. + previousData: previous?.resultData.current?.data, + }, }; return internalState as InternalState; @@ -211,7 +222,7 @@ export function useQueryWithInternalState< updateInternalState((internalState = createInternalState(internalState))); } - const observable = internalState.observable; + const { observable, resultData } = internalState; const watchQueryOptions: Readonly> = makeWatchQueryOptions(observable); @@ -238,9 +249,9 @@ export function useQueryWithInternalState< // Make sure getCurrentResult returns a fresh ApolloQueryResult, // but save the current data as this.previousData, just like setResult // usually does. - internalState.previousData = - internalState.result?.data || internalState.previousData; - internalState.result = void 0; + resultData.previousData = + resultData.current?.data || resultData.previousData; + resultData.current = void 0; } observable[lastWatchOptions] = watchQueryOptions; } @@ -282,7 +293,12 @@ export function useQueryWithInternalState< ) { // If SSR has been explicitly disabled, and this function has been called // on the server side, return the default loading state. - internalState.result = toQueryResult(ssrDisabledResult, internalState); + resultData.current = toQueryResult( + ssrDisabledResult, + resultData, + observable, + client + ); } else if (options.skip || watchQueryOptions.fetchPolicy === "standby") { // When skipping a query (ie. we're not querying for data but still want to // render children), make sure the `data` is cleared out and `loading` is @@ -294,12 +310,18 @@ export function useQueryWithInternalState< // previously received data is all of a sudden removed. Unfortunately, // changing this is breaking, so we'll have to wait until Apollo Client 4.0 // to address this. - internalState.result = toQueryResult(skipStandbyResult, internalState); + resultData.current = toQueryResult( + skipStandbyResult, + resultData, + observable, + client + ); } else if ( - internalState.result?.[originalResult] === ssrDisabledResult || - internalState.result?.[originalResult] === skipStandbyResult + resultData.current && + (resultData.current[originalResult] === ssrDisabledResult || + resultData.current[originalResult] === skipStandbyResult) ) { - internalState.result = void 0; + resultData.current = void 0; } const ssrAllowed = !(options.ssr === false || options.skip); @@ -313,16 +335,21 @@ export function useQueryWithInternalState< } } + const disableNetworkFetches = client.disableNetworkFetches; const partialRefetch = options.partialRefetch; const result = useSyncExternalStore( React.useCallback( (handleStoreChange) => { + // reference `disableNetworkFetches` here to ensure that the rules of hooks + // keep it as a dependency of this effect, even though it's not used + disableNetworkFetches; + if (renderPromises) { return () => {}; } const onNext = () => { - const previousResult = internalState.result; + const previousResult = resultData.current; // We use `getCurrentResult()` instead of the onNext argument because // the values differ slightly. Specifically, loading results will have // an empty object for data instead of `undefined` for some reason. @@ -338,11 +365,13 @@ export function useQueryWithInternalState< } setResult( + resultData, result, handleStoreChange, - internalState, callbackRef.current, - partialRefetch + partialRefetch, + observable, + client ); }; @@ -355,13 +384,14 @@ export function useQueryWithInternalState< throw error; } - const previousResult = internalState.result; + const previousResult = resultData.current; if ( !previousResult || (previousResult && previousResult.loading) || !equal(error, previousResult.error) ) { setResult( + resultData, { data: (previousResult && previousResult.data) as TData, error: error as ApolloError, @@ -369,9 +399,10 @@ export function useQueryWithInternalState< networkStatus: NetworkStatus.error, }, handleStoreChange, - internalState, callbackRef.current, - partialRefetch + partialRefetch, + observable, + client ); } }; @@ -386,27 +417,42 @@ export function useQueryWithInternalState< setTimeout(() => subscription.unsubscribe()); }; }, - // eslint-disable-next-line react-compiler/react-compiler - // eslint-disable-next-line react-hooks/exhaustive-deps + [ - // We memoize the subscribe function using useCallback and the following - // dependency keys, because the subscribe function reference is all that - // useSyncExternalStore uses internally as a dependency key for the - // useEffect ultimately responsible for the subscription, so we are - // effectively passing this dependency array to that useEffect buried - // inside useSyncExternalStore, as desired. - observable, + disableNetworkFetches, renderPromises, - internalState.client.disableNetworkFetches, + observable, + resultData, partialRefetch, + client, ] ), - - () => getCurrentResult(internalState, callbackRef.current, partialRefetch), - () => getCurrentResult(internalState, callbackRef.current, partialRefetch) + () => + getCurrentResult( + resultData, + observable, + callbackRef.current, + partialRefetch, + client + ), + () => + getCurrentResult( + resultData, + observable, + callbackRef.current, + partialRefetch, + client + ) ); - return { result, obsQueryFields, internalState }; + return { + result, + obsQueryFields, + observable, + resultData, + client, + updateInternalState, + }; } // A function to massage options before passing them to ObservableQuery. @@ -503,23 +549,23 @@ export function getObsQueryOptions< } function setResult( + resultData: InternalResult, nextResult: ApolloQueryResult, forceUpdate: () => void, - internalState: InternalState, callbacks: Callbacks, - partialRefetch: boolean | undefined + partialRefetch: boolean | undefined, + observable: ObservableQuery, + client: ApolloClient ) { - const previousResult = internalState.result; + const previousResult = resultData.current; if (previousResult && previousResult.data) { - internalState.previousData = previousResult.data; + resultData.previousData = previousResult.data; } - internalState.result = toQueryResult( - unsafeHandlePartialRefetch( - nextResult, - internalState.observable, - partialRefetch - ), - internalState + resultData.current = toQueryResult( + unsafeHandlePartialRefetch(nextResult, observable, partialRefetch), + resultData, + observable, + client ); // Calling state.setResult always triggers an update, though some call sites // perform additional equality checks before committing to an update. @@ -559,25 +605,29 @@ function handleErrorOrCompleted( } function getCurrentResult( - internalState: InternalState, + resultData: InternalResult, + observable: ObservableQuery, callbacks: Callbacks, - partialRefetch: boolean | undefined + partialRefetch: boolean | undefined, + client: ApolloClient ): InternalQueryResult { // Using this.result as a cache ensures getCurrentResult continues returning // the same (===) result object, unless state.setResult has been called, or // we're doing server rendering and therefore override the result below. - if (!internalState.result) { + if (!resultData.current) { // WARNING: SIDE-EFFECTS IN THE RENDER FUNCTION // this could call unsafeHandlePartialRefetch setResult( - internalState.observable.getCurrentResult(), + resultData, + observable.getCurrentResult(), () => {}, - internalState, callbacks, - partialRefetch + partialRefetch, + observable, + client ); } - return internalState.result!; + return resultData.current!; } export function getDefaultFetchPolicy< @@ -604,17 +654,19 @@ function toApolloError( export function toQueryResult( result: ApolloQueryResult, - internalState: InternalState + resultData: InternalResult, + observable: ObservableQuery, + client: ApolloClient ): InternalQueryResult { const { data, partial, ...resultWithoutPartial } = result; const queryResult: InternalQueryResult = { data, // Ensure always defined, even if result.data is missing. ...resultWithoutPartial, - client: internalState.client, - observable: internalState.observable, - variables: internalState.observable.variables, + client: client, + observable: observable, + variables: observable.variables, called: result !== ssrDisabledResult && result !== skipStandbyResult, - previousData: internalState.previousData, + previousData: resultData.previousData, } satisfies Omit< InternalQueryResult, typeof originalResult From 06d98acb3d7cf758220675297d4f02c24a3981c1 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Wed, 5 Jun 2024 17:58:41 +0200 Subject: [PATCH 20/32] extract new `useInternalState` hook --- src/react/hooks/useLazyQuery.ts | 4 +- src/react/hooks/useQuery.ts | 79 +++++++++++++++++++++------------ 2 files changed, 53 insertions(+), 30 deletions(-) diff --git a/src/react/hooks/useLazyQuery.ts b/src/react/hooks/useLazyQuery.ts index 6f2e74c0663..35662ecdd63 100644 --- a/src/react/hooks/useLazyQuery.ts +++ b/src/react/hooks/useLazyQuery.ts @@ -27,7 +27,7 @@ import { getObsQueryOptions, lastWatchOptions, toQueryResult, - useQueryWithInternalState, + useQueryInternals, } from "./useQuery.js"; // The following methods, when called will execute the query, regardless of @@ -109,7 +109,7 @@ export function useLazyQuery< resultData, observable, updateInternalState, - } = useQueryWithInternalState(document, queryHookOptions); + } = useQueryInternals(document, queryHookOptions); const initialFetchPolicy = observable.options.initialFetchPolicy || diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index 54055b62799..6233b414c0e 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -36,6 +36,7 @@ import { maybeDeepFreeze, } from "../../utilities/index.js"; import { wrapHook } from "./internal/index.js"; +import type { RenderPromises } from "../ssr/RenderPromises.js"; const { prototype: { hasOwnProperty }, @@ -143,42 +144,23 @@ function _useQuery< query: DocumentNode | TypedDocumentNode, options: QueryHookOptions, NoInfer> ) { - const { result, obsQueryFields } = useQueryWithInternalState(query, options); + const { result, obsQueryFields } = useQueryInternals(query, options); return React.useMemo( () => ({ ...result, ...obsQueryFields }), [result, obsQueryFields] ); } -export function useQueryWithInternalState< +function useInternalState< TData = any, TVariables extends OperationVariables = OperationVariables, >( - query: DocumentNode | TypedDocumentNode, - options: QueryHookOptions, NoInfer> + client: ApolloClient, + query: DocumentNode | TypedDocumentNode, + options: QueryHookOptions, NoInfer>, + renderPromises: RenderPromises | undefined, + makeWatchQueryOptions: () => WatchQueryOptions ) { - const client = useApolloClient(options.client); - - // The renderPromises field gets initialized here in the useQuery method, at - // the beginning of everything (for a given component rendering, at least), - // so we can safely use this.renderPromises in other/later InternalState - // methods without worrying it might be uninitialized. Even after - // initialization, this.renderPromises is usually undefined (unless SSR is - // happening), but that's fine as long as it has been initialized that way, - // rather than left uninitialized. - const renderPromises = React.useContext(getApolloContext()).renderPromises; - - const makeWatchQueryOptions = ( - observable?: ObservableQuery - ) => - createWatchQueryOptions( - client, - query, - options, - !!renderPromises, - observable - ); - function createInternalState(previous?: InternalState) { verifyDocumentType(query, DocumentType.Query); @@ -219,10 +201,51 @@ export function useQueryWithInternalState< // Since we sometimes trigger some side-effects in the render function, we // re-assign `state` to the new state to ensure that those side-effects are // triggered with the new state. - updateInternalState((internalState = createInternalState(internalState))); + const newInternalState = createInternalState(internalState); + updateInternalState(newInternalState); + return [newInternalState, updateInternalState] as const; } - const { observable, resultData } = internalState; + return [internalState, updateInternalState] as const; +} + +export function useQueryInternals< + TData = any, + TVariables extends OperationVariables = OperationVariables, +>( + query: DocumentNode | TypedDocumentNode, + options: QueryHookOptions, NoInfer> +) { + const client = useApolloClient(options.client); + + // The renderPromises field gets initialized here in the useQuery method, at + // the beginning of everything (for a given component rendering, at least), + // so we can safely use this.renderPromises in other/later InternalState + // methods without worrying it might be uninitialized. Even after + // initialization, this.renderPromises is usually undefined (unless SSR is + // happening), but that's fine as long as it has been initialized that way, + // rather than left uninitialized. + const renderPromises = React.useContext(getApolloContext()).renderPromises; + + const makeWatchQueryOptions = ( + observable?: ObservableQuery + ) => + createWatchQueryOptions( + client, + query, + options, + !!renderPromises, + observable + ); + + const [{ observable, resultData }, updateInternalState] = useInternalState( + client, + query, + options, + renderPromises, + makeWatchQueryOptions + ); + const watchQueryOptions: Readonly> = makeWatchQueryOptions(observable); From 7717b4fe2ab90370ee3ddfd484e5c4a102668ed2 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Wed, 5 Jun 2024 18:05:24 +0200 Subject: [PATCH 21/32] extract `useResubscribeIfNecessary` hook --- src/react/hooks/useQuery.ts | 84 ++++++++++++++++++++++--------------- 1 file changed, 51 insertions(+), 33 deletions(-) diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index 6233b414c0e..95596a485d2 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -226,6 +226,8 @@ export function useQueryInternals< // happening), but that's fine as long as it has been initialized that way, // rather than left uninitialized. const renderPromises = React.useContext(getApolloContext()).renderPromises; + const disableNetworkFetches = client.disableNetworkFetches; + const ssrAllowed = !(options.ssr === false || options.skip); const makeWatchQueryOptions = ( observable?: ObservableQuery @@ -249,35 +251,13 @@ export function useQueryInternals< const watchQueryOptions: Readonly> = makeWatchQueryOptions(observable); - // TODO: this part is not compatible with any rules of React, and there's - // no good way to rewrite it. - // it should be moved out into a separate hook that will not be optimized by the compiler - { - if ( - observable[lastWatchOptions] && - !equal(observable[lastWatchOptions], watchQueryOptions) - ) { - // Though it might be tempting to postpone this reobserve call to the - // useEffect block, we need getCurrentResult to return an appropriate - // loading:true result synchronously (later within the same call to - // useQuery). Since we already have this.observable here (not true for - // the very first call to useQuery), we are not initiating any new - // 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. - observable.reobserve( - getObsQueryOptions(client, options, watchQueryOptions, observable) - ); - - // Make sure getCurrentResult returns a fresh ApolloQueryResult, - // but save the current data as this.previousData, just like setResult - // usually does. - resultData.previousData = - resultData.current?.data || resultData.previousData; - resultData.current = void 0; - } - observable[lastWatchOptions] = watchQueryOptions; - } + useResubscribeIfNecessary( + observable, // might get mutated during render + resultData, // might get mutated during render + watchQueryOptions, + client, + options + ); const _callbacks = { onCompleted: options.onCompleted || noop, @@ -310,7 +290,7 @@ export function useQueryInternals< ); if ( - (renderPromises || client.disableNetworkFetches) && + (renderPromises || disableNetworkFetches) && options.ssr === false && !options.skip ) { @@ -347,8 +327,6 @@ export function useQueryInternals< resultData.current = void 0; } - const ssrAllowed = !(options.ssr === false || options.skip); - if (renderPromises && ssrAllowed) { renderPromises.registerSSRObservable(observable); @@ -358,7 +336,6 @@ export function useQueryInternals< } } - const disableNetworkFetches = client.disableNetworkFetches; const partialRefetch = options.partialRefetch; const result = useSyncExternalStore( React.useCallback( @@ -477,6 +454,47 @@ export function useQueryInternals< updateInternalState, }; } +// this hook is not compatible with any rules of React, and there's no good way to rewrite it. +// it should stay a separate hook that will not be optimized by the compiler +function useResubscribeIfNecessary< + TData = any, + TVariables extends OperationVariables = OperationVariables, +>( + /** this hook will mutate properties on `observable` */ + observable: ObsQueryWithMeta, + /** this hook will mutate properties on `resultData` */ + resultData: InternalResult, + watchQueryOptions: Readonly>, + client: ApolloClient, + options: QueryHookOptions, NoInfer> +) { + { + if ( + observable[lastWatchOptions] && + !equal(observable[lastWatchOptions], watchQueryOptions) + ) { + // Though it might be tempting to postpone this reobserve call to the + // useEffect block, we need getCurrentResult to return an appropriate + // loading:true result synchronously (later within the same call to + // useQuery). Since we already have this.observable here (not true for + // the very first call to useQuery), we are not initiating any new + // 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. + observable.reobserve( + getObsQueryOptions(client, options, watchQueryOptions, observable) + ); + + // Make sure getCurrentResult returns a fresh ApolloQueryResult, + // but save the current data as this.previousData, just like setResult + // usually does. + resultData.previousData = + resultData.current?.data || resultData.previousData; + resultData.current = void 0; + } + observable[lastWatchOptions] = watchQueryOptions; + } +} // A function to massage options before passing them to ObservableQuery. export function createWatchQueryOptions< From 3889fffe1bdb60bf51bd3d46bb742295c0c48ab8 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Wed, 5 Jun 2024 18:07:13 +0200 Subject: [PATCH 22/32] add comment --- src/react/hooks/useQuery.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index 95596a485d2..b6d19597f57 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -320,6 +320,8 @@ export function useQueryInternals< client ); } else if ( + // reset result if the last render was skipping for some reason, + // but this render isn't skipping anymore resultData.current && (resultData.current[originalResult] === ssrDisabledResult || resultData.current[originalResult] === skipStandbyResult) From da4b8405764926393dfecf99cdca396f499a6205 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Fri, 7 Jun 2024 12:05:37 +0200 Subject: [PATCH 23/32] extract `bindObservableMethods` --- src/react/hooks/useQuery.ts | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index b6d19597f57..1b62d02c113 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -276,18 +276,7 @@ export function useQueryInternals< const obsQueryFields = React.useMemo< Omit, "variables"> - >( - () => ({ - refetch: observable.refetch.bind(observable), - reobserve: observable.reobserve.bind(observable), - fetchMore: observable.fetchMore.bind(observable), - updateQuery: observable.updateQuery.bind(observable), - startPolling: observable.startPolling.bind(observable), - stopPolling: observable.stopPolling.bind(observable), - subscribeToMore: observable.subscribeToMore.bind(observable), - }), - [observable] - ); + >(() => bindObservableMethods(observable), [observable]); if ( (renderPromises || disableNetworkFetches) && @@ -769,3 +758,17 @@ const skipStandbyResult = maybeDeepFreeze({ error: void 0, networkStatus: NetworkStatus.ready, }); + +function bindObservableMethods( + observable: ObservableQuery +) { + return { + refetch: observable.refetch.bind(observable), + reobserve: observable.reobserve.bind(observable), + fetchMore: observable.fetchMore.bind(observable), + updateQuery: observable.updateQuery.bind(observable), + startPolling: observable.startPolling.bind(observable), + stopPolling: observable.stopPolling.bind(observable), + subscribeToMore: observable.subscribeToMore.bind(observable), + }; +} From f18b753f374eed2b2be268e550683f3c8d7245cd Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Fri, 7 Jun 2024 12:16:30 +0200 Subject: [PATCH 24/32] extract `useHandleSkip` and `useRegisterSSRObservable` hooks --- src/react/hooks/useQuery.ts | 132 +++++++++++++++++++++++------------- 1 file changed, 85 insertions(+), 47 deletions(-) diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index 1b62d02c113..8d1cef30eb3 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -278,54 +278,21 @@ export function useQueryInternals< Omit, "variables"> >(() => bindObservableMethods(observable), [observable]); - if ( - (renderPromises || disableNetworkFetches) && - options.ssr === false && - !options.skip - ) { - // If SSR has been explicitly disabled, and this function has been called - // on the server side, return the default loading state. - resultData.current = toQueryResult( - ssrDisabledResult, - resultData, - observable, - client - ); - } else if (options.skip || watchQueryOptions.fetchPolicy === "standby") { - // When skipping a query (ie. we're not querying for data but still want to - // render children), make sure the `data` is cleared out and `loading` is - // set to `false` (since we aren't loading anything). - // - // NOTE: We no longer think this is the correct behavior. Skipping should - // not automatically set `data` to `undefined`, but instead leave the - // previous data in place. In other words, skipping should not mandate that - // previously received data is all of a sudden removed. Unfortunately, - // changing this is breaking, so we'll have to wait until Apollo Client 4.0 - // to address this. - resultData.current = toQueryResult( - skipStandbyResult, - resultData, - observable, - client - ); - } else if ( - // reset result if the last render was skipping for some reason, - // but this render isn't skipping anymore - resultData.current && - (resultData.current[originalResult] === ssrDisabledResult || - resultData.current[originalResult] === skipStandbyResult) - ) { - resultData.current = void 0; - } - - if (renderPromises && ssrAllowed) { - renderPromises.registerSSRObservable(observable); + useHandleSkip( + renderPromises, + disableNetworkFetches, + options, + resultData, + observable, + client, + watchQueryOptions + ); - if (observable.getCurrentResult().loading) { - // TODO: This is a legacy API which could probably be cleaned up - renderPromises.addObservableQueryPromise(observable); - } - } + useRegisterSSRObservable( + renderPromises, + ssrAllowed, + observable + ); const partialRefetch = options.partialRefetch; const result = useSyncExternalStore( @@ -445,6 +412,77 @@ export function useQueryInternals< updateInternalState, }; } +function useRegisterSSRObservable< + TData = any, + TVariables extends OperationVariables = OperationVariables, +>( + renderPromises: RenderPromises | undefined, + ssrAllowed: boolean, + observable: ObsQueryWithMeta +) { + if (renderPromises && ssrAllowed) { + renderPromises.registerSSRObservable(observable); + + if (observable.getCurrentResult().loading) { + // TODO: This is a legacy API which could probably be cleaned up + renderPromises.addObservableQueryPromise(observable); + } + } +} + +function useHandleSkip< + TData = any, + TVariables extends OperationVariables = OperationVariables, +>( + renderPromises: RenderPromises | undefined, + disableNetworkFetches: boolean, + options: QueryHookOptions, NoInfer>, + resultData: InternalResult, + observable: ObsQueryWithMeta, + client: ApolloClient, + watchQueryOptions: Readonly> +) { + if ( + (renderPromises || disableNetworkFetches) && + options.ssr === false && + !options.skip + ) { + // If SSR has been explicitly disabled, and this function has been called + // on the server side, return the default loading state. + resultData.current = toQueryResult( + ssrDisabledResult, + resultData, + observable, + client + ); + } else if (options.skip || watchQueryOptions.fetchPolicy === "standby") { + // When skipping a query (ie. we're not querying for data but still want to + // render children), make sure the `data` is cleared out and `loading` is + // set to `false` (since we aren't loading anything). + // + // NOTE: We no longer think this is the correct behavior. Skipping should + // not automatically set `data` to `undefined`, but instead leave the + // previous data in place. In other words, skipping should not mandate that + // previously received data is all of a sudden removed. Unfortunately, + // changing this is breaking, so we'll have to wait until Apollo Client 4.0 + // to address this. + resultData.current = toQueryResult( + skipStandbyResult, + resultData, + observable, + client + ); + } else if ( + // reset result if the last render was skipping for some reason, + // but this render isn't skipping anymore + resultData.current && + (resultData.current[originalResult] === ssrDisabledResult || + resultData.current[originalResult] === skipStandbyResult) + ) { + resultData.current = void 0; + } +} + // this hook is not compatible with any rules of React, and there's no good way to rewrite it. // it should stay a separate hook that will not be optimized by the compiler function useResubscribeIfNecessary< From 8480ce61477ebb613368a58187a5d55693e8fc49 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Fri, 7 Jun 2024 12:31:56 +0200 Subject: [PATCH 25/32] extract useObservableSubscriptionResult hook --- src/react/hooks/useQuery.ts | 78 ++++++++++++++++++++++++------------- 1 file changed, 52 insertions(+), 26 deletions(-) diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index 8d1cef30eb3..5de1e8ececb 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -228,6 +228,7 @@ export function useQueryInternals< const renderPromises = React.useContext(getApolloContext()).renderPromises; const disableNetworkFetches = client.disableNetworkFetches; const ssrAllowed = !(options.ssr === false || options.skip); + const partialRefetch = options.partialRefetch; const makeWatchQueryOptions = ( observable?: ObservableQuery @@ -259,21 +260,6 @@ export function useQueryInternals< options ); - const _callbacks = { - onCompleted: options.onCompleted || noop, - onError: options.onError || noop, - }; - const callbackRef = React.useRef>(_callbacks); - React.useEffect(() => { - // Make sure state.onCompleted and state.onError always reflect the latest - // options.onCompleted and options.onError callbacks provided to useQuery, - // since those functions are often recreated every time useQuery is called. - // Like the forceUpdate method, the versions of these methods inherited from - // InternalState.prototype are empty no-ops, but we can override them on the - // base state object (without modifying the prototype). - callbackRef.current = _callbacks; - }); - const obsQueryFields = React.useMemo< Omit, "variables"> >(() => bindObservableMethods(observable), [observable]); @@ -294,8 +280,56 @@ export function useQueryInternals< observable ); - const partialRefetch = options.partialRefetch; - const result = useSyncExternalStore( + const result = useObservableSubscriptionResult( + disableNetworkFetches, + renderPromises, + resultData, + observable, + { + onCompleted: options.onCompleted || noop, + onError: options.onError || noop, + }, + partialRefetch, + client + ); + + return { + result, + obsQueryFields, + observable, + resultData, + client, + updateInternalState, + }; +} + +function useObservableSubscriptionResult< + TData = any, + TVariables extends OperationVariables = OperationVariables, +>( + disableNetworkFetches: boolean, + renderPromises: RenderPromises | undefined, + resultData: InternalResult, + observable: ObservableQuery, + callbacks: { + onCompleted: (data: TData) => void; + onError: (error: ApolloError) => void; + }, + partialRefetch: boolean | undefined, + client: ApolloClient +) { + const callbackRef = React.useRef>(callbacks); + React.useEffect(() => { + // Make sure state.onCompleted and state.onError always reflect the latest + // options.onCompleted and options.onError callbacks provided to useQuery, + // since those functions are often recreated every time useQuery is called. + // Like the forceUpdate method, the versions of these methods inherited from + // InternalState.prototype are empty no-ops, but we can override them on the + // base state object (without modifying the prototype). + callbackRef.current = callbacks; + }); + + return useSyncExternalStore( React.useCallback( (handleStoreChange) => { // reference `disableNetworkFetches` here to ensure that the rules of hooks @@ -402,16 +436,8 @@ export function useQueryInternals< client ) ); - - return { - result, - obsQueryFields, - observable, - resultData, - client, - updateInternalState, - }; } + function useRegisterSSRObservable< TData = any, TVariables extends OperationVariables = OperationVariables, From 30b17694627ea8409b54983256bf71d06bde4e9f Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Fri, 7 Jun 2024 12:50:05 +0200 Subject: [PATCH 26/32] change some method arguments. remove obsolete comment --- src/react/hooks/useQuery.ts | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index 5de1e8ececb..112e5a33824 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -218,14 +218,8 @@ export function useQueryInternals< ) { const client = useApolloClient(options.client); - // The renderPromises field gets initialized here in the useQuery method, at - // the beginning of everything (for a given component rendering, at least), - // so we can safely use this.renderPromises in other/later InternalState - // methods without worrying it might be uninitialized. Even after - // initialization, this.renderPromises is usually undefined (unless SSR is - // happening), but that's fine as long as it has been initialized that way, - // rather than left uninitialized. const renderPromises = React.useContext(getApolloContext()).renderPromises; + const isSyncSSR = !!renderPromises; const disableNetworkFetches = client.disableNetworkFetches; const ssrAllowed = !(options.ssr === false || options.skip); const partialRefetch = options.partialRefetch; @@ -265,10 +259,10 @@ export function useQueryInternals< >(() => bindObservableMethods(observable), [observable]); useHandleSkip( - renderPromises, + resultData, // might get mutated during render + isSyncSSR, disableNetworkFetches, options, - resultData, observable, client, watchQueryOptions @@ -282,7 +276,7 @@ export function useQueryInternals< const result = useObservableSubscriptionResult( disableNetworkFetches, - renderPromises, + isSyncSSR, resultData, observable, { @@ -308,7 +302,7 @@ function useObservableSubscriptionResult< TVariables extends OperationVariables = OperationVariables, >( disableNetworkFetches: boolean, - renderPromises: RenderPromises | undefined, + skipSubscribing: boolean, resultData: InternalResult, observable: ObservableQuery, callbacks: { @@ -336,7 +330,7 @@ function useObservableSubscriptionResult< // keep it as a dependency of this effect, even though it's not used disableNetworkFetches; - if (renderPromises) { + if (skipSubscribing) { return () => {}; } @@ -412,7 +406,7 @@ function useObservableSubscriptionResult< [ disableNetworkFetches, - renderPromises, + skipSubscribing, observable, resultData, partialRefetch, @@ -460,16 +454,17 @@ function useHandleSkip< TData = any, TVariables extends OperationVariables = OperationVariables, >( - renderPromises: RenderPromises | undefined, + /** this hook will mutate properties on `resultData` */ + resultData: InternalResult, + isSyncSSR: boolean, disableNetworkFetches: boolean, options: QueryHookOptions, NoInfer>, - resultData: InternalResult, observable: ObsQueryWithMeta, client: ApolloClient, watchQueryOptions: Readonly> ) { if ( - (renderPromises || disableNetworkFetches) && + (isSyncSSR || disableNetworkFetches) && options.ssr === false && !options.skip ) { From c8968854886f5714752d1791bb3a5a7bf9b71c58 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Fri, 7 Jun 2024 12:56:16 +0200 Subject: [PATCH 27/32] curry `createMakeWatchQueryOptions` --- src/react/hooks/useLazyQuery.ts | 11 ++-- src/react/hooks/useQuery.ts | 97 +++++++++++++++++---------------- 2 files changed, 53 insertions(+), 55 deletions(-) diff --git a/src/react/hooks/useLazyQuery.ts b/src/react/hooks/useLazyQuery.ts index 35662ecdd63..c73dc460f05 100644 --- a/src/react/hooks/useLazyQuery.ts +++ b/src/react/hooks/useLazyQuery.ts @@ -22,7 +22,7 @@ import type { UpdateInternalState, } from "./useQuery.js"; import { - createWatchQueryOptions, + createMakeWatchQueryOptions, getDefaultFetchPolicy, getObsQueryOptions, lastWatchOptions, @@ -167,7 +167,6 @@ export function useLazyQuery< const promise = executeQuery( { ...options, skip: false }, - false, document, resultData, observable, @@ -208,7 +207,6 @@ function executeQuery( options: QueryHookOptions & { query?: DocumentNode; }, - hasRenderPromises: boolean, currentQuery: DocumentNode, resultData: InternalResult, observable: ObsQueryWithMeta, @@ -216,13 +214,12 @@ function executeQuery( updateInternalState: UpdateInternalState ) { const query = options.query || currentQuery; - const watchQueryOptions = createWatchQueryOptions( + const watchQueryOptions = createMakeWatchQueryOptions( client, query, options, - hasRenderPromises, - observable - ); + false + )(observable); const concast = observable.reobserveAsConcast( getObsQueryOptions(client, options, watchQueryOptions, observable) diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index 112e5a33824..b1bbbb061b4 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -224,16 +224,12 @@ export function useQueryInternals< const ssrAllowed = !(options.ssr === false || options.skip); const partialRefetch = options.partialRefetch; - const makeWatchQueryOptions = ( - observable?: ObservableQuery - ) => - createWatchQueryOptions( - client, - query, - options, - !!renderPromises, - observable - ); + const makeWatchQueryOptions = createMakeWatchQueryOptions( + client, + query, + options, + isSyncSSR + ); const [{ observable, resultData }, updateInternalState] = useInternalState( client, @@ -546,8 +542,12 @@ function useResubscribeIfNecessary< } } -// A function to massage options before passing them to ObservableQuery. -export function createWatchQueryOptions< +/* + * A function to massage options before passing them to ObservableQuery. + * This is two-step curried because we want to reuse the `make` function, + * but the `observable` might differ between calls to `make`. + */ +export function createMakeWatchQueryOptions< TData = any, TVariables extends OperationVariables = OperationVariables, >( @@ -564,46 +564,47 @@ export function createWatchQueryOptions< // query property that we add below. ...otherOptions }: QueryHookOptions = {}, - hasRenderPromises: boolean, - observable: ObservableQuery | undefined -): WatchQueryOptions { - // This Object.assign is safe because otherOptions is a fresh ...rest object - // that did not exist until just now, so modifications are still allowed. - const watchQueryOptions: WatchQueryOptions = Object.assign( - otherOptions, - { query } - ); + hasRenderPromises: boolean +) { + return ( + observable?: ObservableQuery + ): WatchQueryOptions => { + // This Object.assign is safe because otherOptions is a fresh ...rest object + // that did not exist until just now, so modifications are still allowed. + const watchQueryOptions: WatchQueryOptions = + Object.assign(otherOptions, { query }); - if ( - hasRenderPromises && - (watchQueryOptions.fetchPolicy === "network-only" || - watchQueryOptions.fetchPolicy === "cache-and-network") - ) { - // this behavior was added to react-apollo without explanation in this PR - // https://github.com/apollographql/react-apollo/pull/1579 - watchQueryOptions.fetchPolicy = "cache-first"; - } + if ( + hasRenderPromises && + (watchQueryOptions.fetchPolicy === "network-only" || + watchQueryOptions.fetchPolicy === "cache-and-network") + ) { + // this behavior was added to react-apollo without explanation in this PR + // https://github.com/apollographql/react-apollo/pull/1579 + watchQueryOptions.fetchPolicy = "cache-first"; + } - if (!watchQueryOptions.variables) { - watchQueryOptions.variables = {} as TVariables; - } + if (!watchQueryOptions.variables) { + watchQueryOptions.variables = {} as TVariables; + } - if (skip) { - // When skipping, we set watchQueryOptions.fetchPolicy initially to - // "standby", but we also need/want to preserve the initial non-standby - // fetchPolicy that would have been used if not skipping. - watchQueryOptions.initialFetchPolicy = - watchQueryOptions.initialFetchPolicy || - watchQueryOptions.fetchPolicy || - getDefaultFetchPolicy(defaultOptions, client.defaultOptions); - watchQueryOptions.fetchPolicy = "standby"; - } else if (!watchQueryOptions.fetchPolicy) { - watchQueryOptions.fetchPolicy = - observable?.options.initialFetchPolicy || - getDefaultFetchPolicy(defaultOptions, client.defaultOptions); - } + if (skip) { + // When skipping, we set watchQueryOptions.fetchPolicy initially to + // "standby", but we also need/want to preserve the initial non-standby + // fetchPolicy that would have been used if not skipping. + watchQueryOptions.initialFetchPolicy = + watchQueryOptions.initialFetchPolicy || + watchQueryOptions.fetchPolicy || + getDefaultFetchPolicy(defaultOptions, client.defaultOptions); + watchQueryOptions.fetchPolicy = "standby"; + } else if (!watchQueryOptions.fetchPolicy) { + watchQueryOptions.fetchPolicy = + observable?.options.initialFetchPolicy || + getDefaultFetchPolicy(defaultOptions, client.defaultOptions); + } - return watchQueryOptions; + return watchQueryOptions; + }; } export function getObsQueryOptions< From 1485651e446b90c8b162a406d849ffdebc8078a8 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Fri, 7 Jun 2024 14:17:48 +0200 Subject: [PATCH 28/32] bring function and hook argyuments into a common order --- src/react/hooks/useLazyQuery.ts | 19 +++-- src/react/hooks/useQuery.ts | 132 ++++++++++++++++++-------------- 2 files changed, 85 insertions(+), 66 deletions(-) diff --git a/src/react/hooks/useLazyQuery.ts b/src/react/hooks/useLazyQuery.ts index c73dc460f05..6adb04f5b04 100644 --- a/src/react/hooks/useLazyQuery.ts +++ b/src/react/hooks/useLazyQuery.ts @@ -166,11 +166,11 @@ export function useLazyQuery< }); const promise = executeQuery( - { ...options, skip: false }, - document, resultData, observable, client, + document, + { ...options, skip: false }, updateInternalState ).then((queryResult) => Object.assign(queryResult, eagerMethods)); @@ -204,13 +204,13 @@ export function useLazyQuery< } function executeQuery( - options: QueryHookOptions & { - query?: DocumentNode; - }, - currentQuery: DocumentNode, resultData: InternalResult, observable: ObsQueryWithMeta, client: ApolloClient, + currentQuery: DocumentNode, + options: QueryHookOptions & { + query?: DocumentNode; + }, updateInternalState: UpdateInternalState ) { const query = options.query || currentQuery; @@ -222,7 +222,7 @@ function executeQuery( )(observable); const concast = observable.reobserveAsConcast( - getObsQueryOptions(client, options, watchQueryOptions, observable) + getObsQueryOptions(observable, client, options, watchQueryOptions) ); // this needs to be set to prevent an immediate `resubscribe` in the // next rerender of the `useQuery` internals @@ -233,9 +233,8 @@ function executeQuery( // might be a different query query, resultData: Object.assign(resultData, { - // Make sure getCurrentResult returns a fresh ApolloQueryResult, - // but save the current data as this.previousData, just like setResult - // usually does. + // We need to modify the previous `resultData` object as we rely on the + // object reference in other places previousData: resultData.current?.data || resultData.previousData, current: undefined, }), diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index b1bbbb061b4..ac8302521bf 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -1,3 +1,22 @@ +/** + * Function parameters in this file try to follow a common order for the sake of + * readability and consistency. The order is as follows: + * + * resultData + * observable + * client + * query + * options + * watchQueryOptions + * makeWatchQueryOptions + * isSSRAllowed + * disableNetworkFetches + * partialRefetch + * renderPromises + * isSyncSSR + * callbacks + */ +/** */ import { invariant } from "../../utilities/globals/index.js"; import * as React from "rehackt"; @@ -175,10 +194,10 @@ function useInternalState< renderPromises.getSSRObservable(makeWatchQueryOptions())) || client.watchQuery( getObsQueryOptions( + undefined, client, options, - makeWatchQueryOptions(), - undefined + makeWatchQueryOptions() ) ), resultData: { @@ -243,11 +262,11 @@ export function useQueryInternals< makeWatchQueryOptions(observable); useResubscribeIfNecessary( - observable, // might get mutated during render resultData, // might get mutated during render - watchQueryOptions, + observable, // might get mutated during render client, - options + options, + watchQueryOptions ); const obsQueryFields = React.useMemo< @@ -256,31 +275,31 @@ export function useQueryInternals< useHandleSkip( resultData, // might get mutated during render - isSyncSSR, - disableNetworkFetches, - options, observable, client, - watchQueryOptions + options, + watchQueryOptions, + disableNetworkFetches, + isSyncSSR ); useRegisterSSRObservable( + observable, renderPromises, - ssrAllowed, - observable + ssrAllowed ); const result = useObservableSubscriptionResult( - disableNetworkFetches, - isSyncSSR, resultData, observable, + client, + disableNetworkFetches, + partialRefetch, + isSyncSSR, { onCompleted: options.onCompleted || noop, onError: options.onError || noop, - }, - partialRefetch, - client + } ); return { @@ -297,16 +316,17 @@ function useObservableSubscriptionResult< TData = any, TVariables extends OperationVariables = OperationVariables, >( - disableNetworkFetches: boolean, - skipSubscribing: boolean, resultData: InternalResult, observable: ObservableQuery, + client: ApolloClient, + disableNetworkFetches: boolean, + partialRefetch: boolean | undefined, + skipSubscribing: boolean, + callbacks: { onCompleted: (data: TData) => void; onError: (error: ApolloError) => void; - }, - partialRefetch: boolean | undefined, - client: ApolloClient + } ) { const callbackRef = React.useRef>(callbacks); React.useEffect(() => { @@ -347,13 +367,13 @@ function useObservableSubscriptionResult< } setResult( - resultData, result, - handleStoreChange, - callbackRef.current, - partialRefetch, + resultData, observable, - client + client, + partialRefetch, + handleStoreChange, + callbackRef.current ); }; @@ -373,18 +393,18 @@ function useObservableSubscriptionResult< !equal(error, previousResult.error) ) { setResult( - resultData, { data: (previousResult && previousResult.data) as TData, error: error as ApolloError, loading: false, networkStatus: NetworkStatus.error, }, - handleStoreChange, - callbackRef.current, - partialRefetch, + resultData, observable, - client + client, + partialRefetch, + handleStoreChange, + callbackRef.current ); } }; @@ -432,9 +452,9 @@ function useRegisterSSRObservable< TData = any, TVariables extends OperationVariables = OperationVariables, >( + observable: ObsQueryWithMeta, renderPromises: RenderPromises | undefined, - ssrAllowed: boolean, - observable: ObsQueryWithMeta + ssrAllowed: boolean ) { if (renderPromises && ssrAllowed) { renderPromises.registerSSRObservable(observable); @@ -452,12 +472,12 @@ function useHandleSkip< >( /** this hook will mutate properties on `resultData` */ resultData: InternalResult, - isSyncSSR: boolean, - disableNetworkFetches: boolean, - options: QueryHookOptions, NoInfer>, observable: ObsQueryWithMeta, client: ApolloClient, - watchQueryOptions: Readonly> + options: QueryHookOptions, NoInfer>, + watchQueryOptions: Readonly>, + disableNetworkFetches: boolean, + isSyncSSR: boolean ) { if ( (isSyncSSR || disableNetworkFetches) && @@ -506,13 +526,13 @@ function useResubscribeIfNecessary< TData = any, TVariables extends OperationVariables = OperationVariables, >( - /** this hook will mutate properties on `observable` */ - observable: ObsQueryWithMeta, /** this hook will mutate properties on `resultData` */ resultData: InternalResult, - watchQueryOptions: Readonly>, + /** this hook will mutate properties on `observable` */ + observable: ObsQueryWithMeta, client: ApolloClient, - options: QueryHookOptions, NoInfer> + options: QueryHookOptions, NoInfer>, + watchQueryOptions: Readonly> ) { { if ( @@ -528,7 +548,7 @@ function useResubscribeIfNecessary< // (potentially) kicks off a network request (for example, when the // variables have changed), which is technically a side-effect. observable.reobserve( - getObsQueryOptions(client, options, watchQueryOptions, observable) + getObsQueryOptions(observable, client, options, watchQueryOptions) ); // Make sure getCurrentResult returns a fresh ApolloQueryResult, @@ -564,7 +584,7 @@ export function createMakeWatchQueryOptions< // query property that we add below. ...otherOptions }: QueryHookOptions = {}, - hasRenderPromises: boolean + isSyncSSR: boolean ) { return ( observable?: ObservableQuery @@ -575,7 +595,7 @@ export function createMakeWatchQueryOptions< Object.assign(otherOptions, { query }); if ( - hasRenderPromises && + isSyncSSR && (watchQueryOptions.fetchPolicy === "network-only" || watchQueryOptions.fetchPolicy === "cache-and-network") ) { @@ -611,10 +631,10 @@ export function getObsQueryOptions< TData, TVariables extends OperationVariables, >( + observable: ObservableQuery | undefined, client: ApolloClient, queryHookOptions: QueryHookOptions, - watchQueryOptions: Partial>, - observable: ObservableQuery | undefined + watchQueryOptions: Partial> ): WatchQueryOptions { const toMerge: Array>> = []; @@ -641,13 +661,13 @@ export function getObsQueryOptions< } function setResult( - resultData: InternalResult, nextResult: ApolloQueryResult, - forceUpdate: () => void, - callbacks: Callbacks, - partialRefetch: boolean | undefined, + resultData: InternalResult, observable: ObservableQuery, - client: ApolloClient + client: ApolloClient, + partialRefetch: boolean | undefined, + forceUpdate: () => void, + callbacks: Callbacks ) { const previousResult = resultData.current; if (previousResult && previousResult.data) { @@ -710,13 +730,13 @@ function getCurrentResult( // WARNING: SIDE-EFFECTS IN THE RENDER FUNCTION // this could call unsafeHandlePartialRefetch setResult( - resultData, observable.getCurrentResult(), - () => {}, - callbacks, - partialRefetch, + resultData, observable, - client + client, + partialRefetch, + () => {}, + callbacks ); } return resultData.current!; From 70f5aaf84ea60863140552619890bdd9fab73dda Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Fri, 7 Jun 2024 15:58:34 +0200 Subject: [PATCH 29/32] Move `onQueryExecuted` into `useInternalState` --- src/react/hooks/useLazyQuery.ts | 32 +++++++----------------------- src/react/hooks/useQuery.ts | 35 +++++++++++++++++++++++++++++---- 2 files changed, 38 insertions(+), 29 deletions(-) diff --git a/src/react/hooks/useLazyQuery.ts b/src/react/hooks/useLazyQuery.ts index 6adb04f5b04..35b89ca6950 100644 --- a/src/react/hooks/useLazyQuery.ts +++ b/src/react/hooks/useLazyQuery.ts @@ -6,6 +6,7 @@ import type { ApolloClient, ApolloQueryResult, OperationVariables, + WatchQueryOptions, } from "../../core/index.js"; import { mergeOptions } from "../../utilities/index.js"; import type { @@ -16,16 +17,11 @@ import type { QueryHookOptions, QueryResult, } from "../types/types.js"; -import type { - InternalResult, - ObsQueryWithMeta, - UpdateInternalState, -} from "./useQuery.js"; +import type { InternalResult, ObsQueryWithMeta } from "./useQuery.js"; import { createMakeWatchQueryOptions, getDefaultFetchPolicy, getObsQueryOptions, - lastWatchOptions, toQueryResult, useQueryInternals, } from "./useQuery.js"; @@ -108,7 +104,7 @@ export function useLazyQuery< client, resultData, observable, - updateInternalState, + onQueryExecuted, } = useQueryInternals(document, queryHookOptions); const initialFetchPolicy = @@ -171,7 +167,7 @@ export function useLazyQuery< client, document, { ...options, skip: false }, - updateInternalState + onQueryExecuted ).then((queryResult) => Object.assign(queryResult, eagerMethods)); // Because the return value of `useLazyQuery` is usually floated, we need @@ -187,7 +183,7 @@ export function useLazyQuery< initialFetchPolicy, observable, resultData, - updateInternalState, + onQueryExecuted, ] ); @@ -211,7 +207,7 @@ function executeQuery( options: QueryHookOptions & { query?: DocumentNode; }, - updateInternalState: UpdateInternalState + onQueryExecuted: (options: WatchQueryOptions) => void ) { const query = options.query || currentQuery; const watchQueryOptions = createMakeWatchQueryOptions( @@ -224,21 +220,7 @@ function executeQuery( const concast = observable.reobserveAsConcast( getObsQueryOptions(observable, client, options, watchQueryOptions) ); - // this needs to be set to prevent an immediate `resubscribe` in the - // next rerender of the `useQuery` internals - observable[lastWatchOptions] = watchQueryOptions; - updateInternalState({ - client, - observable, - // might be a different query - query, - resultData: Object.assign(resultData, { - // We need to modify the previous `resultData` object as we rely on the - // object reference in other places - previousData: resultData.current?.data || resultData.previousData, - current: undefined, - }), - }); + onQueryExecuted(watchQueryOptions); return new Promise< Omit, (typeof EAGER_METHODS)[number]> diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index ac8302521bf..2191ea0fb62 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -222,10 +222,37 @@ function useInternalState< // triggered with the new state. const newInternalState = createInternalState(internalState); updateInternalState(newInternalState); - return [newInternalState, updateInternalState] as const; + return [newInternalState, onQueryExecuted] as const; } - return [internalState, updateInternalState] as const; + return [internalState, onQueryExecuted] as const; + + /** + * Used by `useLazyQuery` when a new query is executed. + * We keep this logic here since it needs to update things in unsafe + * ways and here we at least can keep track of that in a single place. + */ + function onQueryExecuted( + watchQueryOptions: WatchQueryOptions + ) { + // this needs to be set to prevent an immediate `resubscribe` in the + // next rerender of the `useQuery` internals + Object.assign(internalState.observable, { + [lastWatchOptions]: watchQueryOptions, + }); + const resultData = internalState.resultData; + updateInternalState({ + ...internalState, + // might be a different query + query: watchQueryOptions.query, + resultData: Object.assign(resultData, { + // We need to modify the previous `resultData` object as we rely on the + // object reference in other places + previousData: resultData.current?.data || resultData.previousData, + current: undefined, + }), + }); + } } export function useQueryInternals< @@ -250,7 +277,7 @@ export function useQueryInternals< isSyncSSR ); - const [{ observable, resultData }, updateInternalState] = useInternalState( + const [{ observable, resultData }, onQueryExecuted] = useInternalState( client, query, options, @@ -308,7 +335,7 @@ export function useQueryInternals< observable, resultData, client, - updateInternalState, + onQueryExecuted, }; } From fed117bbd63735c573d1889e0bd951a84b8f7e40 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Fri, 7 Jun 2024 18:10:18 +0200 Subject: [PATCH 30/32] some style adjustments to be more compiler-friendly --- src/react/hooks/useQuery.ts | 46 ++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index 2191ea0fb62..da4321d93f7 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -70,7 +70,7 @@ interface InternalQueryResult [originalResult]: ApolloQueryResult; } -const noop = () => {}; +function noop() {} export const lastWatchOptions = Symbol(); export interface ObsQueryWithMeta @@ -213,20 +213,6 @@ function useInternalState< let [internalState, updateInternalState] = React.useState(createInternalState); - if (client !== internalState.client || query !== internalState.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 - // to run the current render function to completion. - // Since we sometimes trigger some side-effects in the render function, we - // re-assign `state` to the new state to ensure that those side-effects are - // triggered with the new state. - const newInternalState = createInternalState(internalState); - updateInternalState(newInternalState); - return [newInternalState, onQueryExecuted] as const; - } - - return [internalState, onQueryExecuted] as const; - /** * Used by `useLazyQuery` when a new query is executed. * We keep this logic here since it needs to update things in unsafe @@ -253,6 +239,20 @@ function useInternalState< }), }); } + + if (client !== internalState.client || query !== internalState.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 + // to run the current render function to completion. + // Since we sometimes trigger some side-effects in the render function, we + // re-assign `state` to the new state to ensure that those side-effects are + // triggered with the new state. + const newInternalState = createInternalState(internalState); + updateInternalState(newInternalState); + return [newInternalState, onQueryExecuted] as const; + } + + return [internalState, onQueryExecuted] as const; } export function useQueryInternals< @@ -405,8 +405,11 @@ function useObservableSubscriptionResult< }; const onError = (error: Error) => { - subscription.unsubscribe(); - subscription = observable.resubscribeAfterError(onNext, onError); + subscription.current.unsubscribe(); + subscription.current = observable.resubscribeAfterError( + onNext, + onError + ); if (!hasOwnProperty.call(error, "graphQLErrors")) { // The error is not a GraphQL error @@ -436,14 +439,19 @@ function useObservableSubscriptionResult< } }; - let subscription = observable.subscribe(onNext, onError); + // TODO evaluate if we keep this in + // React Compiler cannot handle scoped `let` access, but a mutable object + // like this is fine. + // was: + // let subscription = observable.subscribe(onNext, onError); + const subscription = { current: observable.subscribe(onNext, onError) }; // Do the "unsubscribe" with a short delay. // This way, an existing subscription can be reused without an additional // request if "unsubscribe" and "resubscribe" to the same ObservableQuery // happen in very fast succession. return () => { - setTimeout(() => subscription.unsubscribe()); + setTimeout(() => subscription.current.unsubscribe()); }; }, From a69327cce1b36e8855258e9b19427511e0af8748 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Wed, 3 Jul 2024 11:24:17 +0200 Subject: [PATCH 31/32] remove R19 exception from test, chores --- .changeset/nasty-olives-act.md | 5 ++++ .size-limits.json | 4 +-- src/react/hooks/__tests__/useQuery.test.tsx | 29 +-------------------- 3 files changed, 8 insertions(+), 30 deletions(-) create mode 100644 .changeset/nasty-olives-act.md diff --git a/.changeset/nasty-olives-act.md b/.changeset/nasty-olives-act.md new file mode 100644 index 00000000000..02fadec8cae --- /dev/null +++ b/.changeset/nasty-olives-act.md @@ -0,0 +1,5 @@ +--- +"@apollo/client": minor +--- + +Rewrite big parts of `useQuery` and `useLazyQuery` to be more compliant with the Rules of React and React Compiler diff --git a/.size-limits.json b/.size-limits.json index 09bf55362fa..ee1af372b82 100644 --- a/.size-limits.json +++ b/.size-limits.json @@ -1,4 +1,4 @@ { - "dist/apollo-client.min.cjs": 39604, - "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 32852 + "dist/apollo-client.min.cjs": 39808, + "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 32851 } diff --git a/src/react/hooks/__tests__/useQuery.test.tsx b/src/react/hooks/__tests__/useQuery.test.tsx index bfcd534c7e3..7909f8673d8 100644 --- a/src/react/hooks/__tests__/useQuery.test.tsx +++ b/src/react/hooks/__tests__/useQuery.test.tsx @@ -38,7 +38,6 @@ import { useApolloClient } from "../useApolloClient"; import { useLazyQuery } from "../useLazyQuery"; const IS_REACT_17 = React.version.startsWith("17"); -const IS_REACT_19 = React.version.startsWith("19"); describe("useQuery Hook", () => { describe("General use", () => { @@ -1536,33 +1535,7 @@ describe("useQuery Hook", () => { function checkObservableQueries(expectedLinkCount: number) { const obsQueries = client.getObservableQueries("all"); - /* -This is due to a timing change in React 19 - -In React 18, you observe this pattern: - - 1. render - 2. useState initializer - 3. component continues to render with first state - 4. strictMode: render again - 5. strictMode: call useState initializer again - 6. component continues to render with second state - -now, in React 19 it looks like this: - - 1. render - 2. useState initializer - 3. strictMode: call useState initializer again - 4. component continues to render with one of these two states - 5. strictMode: render again - 6. component continues to render with the same state as during the first render - -Since useQuery breaks the rules of React and mutably creates an ObservableQuery on the state during render if none is present, React 18 did create two, while React 19 only creates one. - -This is pure coincidence though, and the useQuery rewrite that doesn't break the rules of hooks as much and creates the ObservableQuery as part of the state initializer will end up with behaviour closer to the old React 18 behaviour again. - -*/ - expect(obsQueries.size).toBe(IS_REACT_19 ? 1 : 2); + expect(obsQueries.size).toBe(2); const activeSet = new Set(); const inactiveSet = new Set(); From 987aaadca203b1dc62839b234e7cf173dc299d20 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Thu, 4 Jul 2024 13:23:58 +0200 Subject: [PATCH 32/32] Apply suggestions from code review Co-authored-by: Jerel Miller --- .size-limits.json | 2 +- src/react/hooks/useLazyQuery.ts | 9 ++-- src/react/hooks/useQuery.ts | 83 ++++++++++++++------------------- 3 files changed, 41 insertions(+), 53 deletions(-) diff --git a/.size-limits.json b/.size-limits.json index ee1af372b82..c3eda5f3708 100644 --- a/.size-limits.json +++ b/.size-limits.json @@ -1,4 +1,4 @@ { - "dist/apollo-client.min.cjs": 39808, + "dist/apollo-client.min.cjs": 39819, "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 32851 } diff --git a/src/react/hooks/useLazyQuery.ts b/src/react/hooks/useLazyQuery.ts index 35b89ca6950..911d2b9e69c 100644 --- a/src/react/hooks/useLazyQuery.ts +++ b/src/react/hooks/useLazyQuery.ts @@ -25,6 +25,7 @@ import { toQueryResult, useQueryInternals, } from "./useQuery.js"; +import { useIsomorphicLayoutEffect } from "./internal/useIsomorphicLayoutEffect.js"; // The following methods, when called will execute the query, regardless of // whether the useLazyQuery execute function was called before. @@ -188,7 +189,7 @@ export function useLazyQuery< ); const executeRef = React.useRef(execute); - React.useLayoutEffect(() => { + useIsomorphicLayoutEffect(() => { executeRef.current = execute; }); @@ -239,14 +240,16 @@ function executeQuery( resolve( toQueryResult( observable.getCurrentResult(), - resultData, + resultData.previousData, observable, client ) ); }, complete: () => { - resolve(toQueryResult(result, resultData, observable, client)); + resolve( + toQueryResult(result, resultData.previousData, observable, client) + ); }, }); }); diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index da4321d93f7..f3ef9aacb0e 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -193,12 +193,7 @@ function useInternalState< (renderPromises && renderPromises.getSSRObservable(makeWatchQueryOptions())) || client.watchQuery( - getObsQueryOptions( - undefined, - client, - options, - makeWatchQueryOptions() - ) + getObsQueryOptions(void 0, client, options, makeWatchQueryOptions()) ), resultData: { // Reuse previousData from previous InternalState (if any) to provide @@ -267,7 +262,7 @@ export function useQueryInternals< const renderPromises = React.useContext(getApolloContext()).renderPromises; const isSyncSSR = !!renderPromises; const disableNetworkFetches = client.disableNetworkFetches; - const ssrAllowed = !(options.ssr === false || options.skip); + const ssrAllowed = options.ssr !== false && !options.skip; const partialRefetch = options.partialRefetch; const makeWatchQueryOptions = createMakeWatchQueryOptions( @@ -310,11 +305,7 @@ export function useQueryInternals< isSyncSSR ); - useRegisterSSRObservable( - observable, - renderPromises, - ssrAllowed - ); + useRegisterSSRObservable(observable, renderPromises, ssrAllowed); const result = useObservableSubscriptionResult( resultData, @@ -349,7 +340,6 @@ function useObservableSubscriptionResult< disableNetworkFetches: boolean, partialRefetch: boolean | undefined, skipSubscribing: boolean, - callbacks: { onCompleted: (data: TData) => void; onError: (error: ApolloError) => void; @@ -483,11 +473,8 @@ function useObservableSubscriptionResult< ); } -function useRegisterSSRObservable< - TData = any, - TVariables extends OperationVariables = OperationVariables, ->( - observable: ObsQueryWithMeta, +function useRegisterSSRObservable( + observable: ObsQueryWithMeta, renderPromises: RenderPromises | undefined, ssrAllowed: boolean ) { @@ -523,7 +510,7 @@ function useHandleSkip< // on the server side, return the default loading state. resultData.current = toQueryResult( ssrDisabledResult, - resultData, + resultData.previousData, observable, client ); @@ -540,7 +527,7 @@ function useHandleSkip< // to address this. resultData.current = toQueryResult( skipStandbyResult, - resultData, + resultData.previousData, observable, client ); @@ -569,32 +556,30 @@ function useResubscribeIfNecessary< options: QueryHookOptions, NoInfer>, watchQueryOptions: Readonly> ) { - { - if ( - observable[lastWatchOptions] && - !equal(observable[lastWatchOptions], watchQueryOptions) - ) { - // Though it might be tempting to postpone this reobserve call to the - // useEffect block, we need getCurrentResult to return an appropriate - // loading:true result synchronously (later within the same call to - // useQuery). Since we already have this.observable here (not true for - // the very first call to useQuery), we are not initiating any new - // 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. - observable.reobserve( - getObsQueryOptions(observable, client, options, watchQueryOptions) - ); - - // Make sure getCurrentResult returns a fresh ApolloQueryResult, - // but save the current data as this.previousData, just like setResult - // usually does. - resultData.previousData = - resultData.current?.data || resultData.previousData; - resultData.current = void 0; - } - observable[lastWatchOptions] = watchQueryOptions; + if ( + observable[lastWatchOptions] && + !equal(observable[lastWatchOptions], watchQueryOptions) + ) { + // Though it might be tempting to postpone this reobserve call to the + // useEffect block, we need getCurrentResult to return an appropriate + // loading:true result synchronously (later within the same call to + // useQuery). Since we already have this.observable here (not true for + // the very first call to useQuery), we are not initiating any new + // 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. + observable.reobserve( + getObsQueryOptions(observable, client, options, watchQueryOptions) + ); + + // Make sure getCurrentResult returns a fresh ApolloQueryResult, + // but save the current data as this.previousData, just like setResult + // usually does. + resultData.previousData = + resultData.current?.data || resultData.previousData; + resultData.current = void 0; } + observable[lastWatchOptions] = watchQueryOptions; } /* @@ -710,7 +695,7 @@ function setResult( } resultData.current = toQueryResult( unsafeHandlePartialRefetch(nextResult, observable, partialRefetch), - resultData, + resultData.previousData, observable, client ); @@ -724,7 +709,7 @@ function setResult( ); } -function handleErrorOrCompleted( +function handleErrorOrCompleted( result: ApolloQueryResult, previousResult: ApolloQueryResult | undefined, callbacks: Callbacks @@ -801,7 +786,7 @@ function toApolloError( export function toQueryResult( result: ApolloQueryResult, - resultData: InternalResult, + previousData: TData | undefined, observable: ObservableQuery, client: ApolloClient ): InternalQueryResult { @@ -813,7 +798,7 @@ export function toQueryResult( observable: observable, variables: observable.variables, called: result !== ssrDisabledResult && result !== skipStandbyResult, - previousData: resultData.previousData, + previousData, } satisfies Omit< InternalQueryResult, typeof originalResult