optimize useQuery result handling

.api-reports/api-report-react.api.md

@@ -1694,6 +1694,8 @@ export interface QueryResult<TData = any, TVariables extends OperationVariables
     client: ApolloClient<any>;
     data: TData | undefined;
     error?: ApolloError;
+    // @deprecated (undocumented)
+    errors?: ReadonlyArray<GraphQLFormattedError>;
     loading: boolean;
     networkStatus: NetworkStatus;
     observable: ObservableQuery<TData, TVariables>;

.api-reports/api-report-react_components.api.md

@@ -1498,6 +1498,8 @@ interface QueryResult<TData = any, TVariables extends OperationVariables = Opera
     client: ApolloClient<any>;
     data: TData | undefined;
     error?: ApolloError;
+    // @deprecated (undocumented)
+    errors?: ReadonlyArray<GraphQLFormattedError>;
     loading: boolean;
     networkStatus: NetworkStatus;
     observable: ObservableQuery<TData, TVariables>;

.api-reports/api-report-react_context.api.md

@@ -1426,6 +1426,8 @@ interface QueryResult<TData = any, TVariables extends OperationVariables = Opera
     client: ApolloClient<any>;
     data: TData | undefined;
     error?: ApolloError;
+    // @deprecated (undocumented)
+    errors?: ReadonlyArray<GraphQLFormattedError>;
     loading: boolean;
     networkStatus: NetworkStatus;
     observable: ObservableQuery<TData, TVariables>;

.api-reports/api-report-react_hooks.api.md

@@ -1559,6 +1559,8 @@ interface QueryResult<TData = any, TVariables extends OperationVariables = Opera
     client: ApolloClient<any>;
     data: TData | undefined;
     error?: ApolloError;
+    // @deprecated (undocumented)
+    errors?: ReadonlyArray<GraphQLFormattedError>;
     loading: boolean;
     networkStatus: NetworkStatus;
     observable: ObservableQuery<TData, TVariables>;

.api-reports/api-report-react_internal.api.md

@@ -1621,6 +1621,8 @@ interface QueryResult<TData = any, TVariables extends OperationVariables = Opera
     client: ApolloClient<any>;
     data: TData | undefined;
     error?: ApolloError;
+    // @deprecated (undocumented)
+    errors?: ReadonlyArray<GraphQLFormattedError>;
     loading: boolean;
     networkStatus: NetworkStatus;
     observable: ObservableQuery<TData, TVariables>;

.api-reports/api-report-react_ssr.api.md

@@ -1411,6 +1411,8 @@ interface QueryResult<TData = any, TVariables extends OperationVariables = Opera
     client: ApolloClient<any>;
     data: TData | undefined;
     error?: ApolloError;
+    // @deprecated (undocumented)
+    errors?: ReadonlyArray<GraphQLFormattedError>;
     loading: boolean;
     networkStatus: NetworkStatus;
     observable: ObservableQuery<TData, TVariables>;

.api-reports/api-report.api.md

@@ -2265,6 +2265,8 @@ export interface QueryResult<TData = any, TVariables extends OperationVariables
     client: ApolloClient<any>;
     data: TData | undefined;
     error?: ApolloError;
+    // @deprecated (undocumented)
+    errors?: ReadonlyArray<GraphQLFormattedError>;
     loading: boolean;
     networkStatus: NetworkStatus;
     observable: ObservableQuery<TData, TVariables>;

.size-limits.json

@@ -1,4 +1,4 @@
 {
-  "dist/apollo-client.min.cjs": 40206,
+  "dist/apollo-client.min.cjs": 40162,
   "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 32999
 }

src/react/hooks/useQuery.ts

@@ -61,14 +61,11 @@ const {
   prototype: { hasOwnProperty },
 } = Object;
 
-const originalResult = Symbol();
 interface InternalQueryResult<TData, TVariables extends OperationVariables>
   extends Omit<
     QueryResult<TData, TVariables>,
     Exclude<keyof ObservableQueryFields<TData, TVariables>, "variables">
-  > {
-  [originalResult]: ApolloQueryResult<TData>;
-}
+  > {}
 
 function noop() {}
 export const lastWatchOptions = Symbol();
@@ -295,22 +292,14 @@ export function useQueryInternals<
     Omit<ObservableQueryFields<TData, TVariables>, "variables">
   >(() => bindObservableMethods(observable), [observable]);
 
-  useHandleSkip<TData, TVariables>(
-    resultData, // might get mutated during render
-    observable,
-    client,
-    options,
-    watchQueryOptions,
-    disableNetworkFetches,
-    isSyncSSR
-  );
-
   useRegisterSSRObservable(observable, renderPromises, ssrAllowed);
 
   const result = useObservableSubscriptionResult<TData, TVariables>(
     resultData,
     observable,
     client,
+    options,
+    watchQueryOptions,
     disableNetworkFetches,
     partialRefetch,
     isSyncSSR,
@@ -337,9 +326,11 @@ function useObservableSubscriptionResult<
   resultData: InternalResult<TData, TVariables>,
   observable: ObservableQuery<TData, TVariables>,
   client: ApolloClient<object>,
+  options: QueryHookOptions<NoInfer<TData>, NoInfer<TVariables>>,
+  watchQueryOptions: Readonly<WatchQueryOptions<TVariables, TData>>,
   disableNetworkFetches: boolean,
   partialRefetch: boolean | undefined,
-  skipSubscribing: boolean,
+  isSyncSSR: boolean,
   callbacks: {
     onCompleted: (data: TData) => void;
     onError: (error: ApolloError) => void;
@@ -356,14 +347,45 @@ function useObservableSubscriptionResult<
     callbackRef.current = callbacks;
   });
 
+  const resultOverride =
+    (
+      (isSyncSSR || 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.
+      ssrDisabledResult
+    : 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.
+      skipStandbyResult
+    : void 0;
+
+  const previousData = resultData.previousData;
+  const currentResultOverride = React.useMemo(
+    () =>
+      resultOverride &&
+      toQueryResult(resultOverride, previousData, observable, client),
+    [client, observable, resultOverride, previousData]
+  );
+
   return 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 (skipSubscribing) {
+        if (isSyncSSR) {
           return () => {};
         }
 
@@ -447,14 +469,15 @@ function useObservableSubscriptionResult<
 
       [
         disableNetworkFetches,
-        skipSubscribing,
+        isSyncSSR,
         observable,
         resultData,
         partialRefetch,
         client,
       ]
     ),
     () =>
+      currentResultOverride ||
       getCurrentResult(
         resultData,
         observable,
@@ -463,6 +486,7 @@ function useObservableSubscriptionResult<
         client
       ),
     () =>
+      currentResultOverride ||
       getCurrentResult(
         resultData,
         observable,
@@ -488,60 +512,6 @@ function useRegisterSSRObservable(
   }
 }
 
-function useHandleSkip<
-  TData = any,
-  TVariables extends OperationVariables = OperationVariables,
->(
-  /** this hook will mutate properties on `resultData` */
-  resultData: InternalResult<TData, TVariables>,
-  observable: ObsQueryWithMeta<TData, TVariables>,
-  client: ApolloClient<object>,
-  options: QueryHookOptions<NoInfer<TData>, NoInfer<TVariables>>,
-  watchQueryOptions: Readonly<WatchQueryOptions<TVariables, TData>>,
-  disableNetworkFetches: boolean,
-  isSyncSSR: boolean
-) {
-  if (
-    (isSyncSSR || 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.previousData,
-      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.previousData,
-      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<
@@ -693,6 +663,15 @@ function setResult<TData, TVariables extends OperationVariables>(
   if (previousResult && previousResult.data) {
     resultData.previousData = previousResult.data;
   }
+
+  if (!nextResult.error && isNonEmptyArray(nextResult.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?
+    nextResult.error = new ApolloError({ graphQLErrors: nextResult.errors });
+  }
+
   resultData.current = toQueryResult(
     unsafeHandlePartialRefetch(nextResult, observable, partialRefetch),
     resultData.previousData,
@@ -702,16 +681,12 @@ function setResult<TData, TVariables extends OperationVariables>(
   // 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],
-    callbacks
-  );
+  handleErrorOrCompleted(nextResult, previousResult?.networkStatus, callbacks);
 }
 
 function handleErrorOrCompleted<TData>(
   result: ApolloQueryResult<TData>,
-  previousResult: ApolloQueryResult<TData> | undefined,
+  previousNetworkStatus: NetworkStatus | undefined,
   callbacks: Callbacks<TData>
 ) {
   if (!result.loading) {
@@ -724,7 +699,7 @@ function handleErrorOrCompleted<TData>(
           callbacks.onError(error);
         } else if (
           result.data &&
-          previousResult?.networkStatus !== result.networkStatus &&
+          previousNetworkStatus !== result.networkStatus &&
           result.networkStatus === NetworkStatus.ready
         ) {
           callbacks.onCompleted(result.data);
@@ -799,21 +774,7 @@ export function toQueryResult<TData, TVariables extends OperationVariables>(
     variables: observable.variables,
     called: result !== ssrDisabledResult && result !== skipStandbyResult,
     previousData,
-  } satisfies Omit<
-    InternalQueryResult<TData, TVariables>,
-    typeof originalResult
-  > 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
-    // 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 });
-  }
-
+  };
   return queryResult;
 }
 

src/react/types/types.ts

@@ -1,5 +1,5 @@
 import type * as ReactTypes from "react";
-import type { DocumentNode } from "graphql";
+import type { DocumentNode, GraphQLFormattedError } from "graphql";
 import type { TypedDocumentNode } from "@graphql-typed-document-node/core";
 
 import type {
@@ -148,6 +148,11 @@ export interface QueryResult<
   previousData?: TData;
   /** {@inheritDoc @apollo/client!QueryResultDocumentation#error:member} */
   error?: ApolloError;
+  /**
+   * @deprecated This property will be removed in a future version of Apollo Client.
+   * Please use `error.graphQLErrors` instead.
+   */
+  errors?: ReadonlyArray<GraphQLFormattedError>;
   /** {@inheritDoc @apollo/client!QueryResultDocumentation#loading:member} */
   loading: boolean;
   /** {@inheritDoc @apollo/client!QueryResultDocumentation#networkStatus:member} */