Try alternate approach using an option to skip updateQuery when there…

… are partial results
Cellule · Feb 3, 2025 · 5fd221d · 5fd221d
1 parent 0c11195
commit 5fd221d
Show file tree

Hide file tree

Showing 16 changed files with 95 additions and 112 deletions.
diff --git a/.changeset/pretty-planets-cough.md b/.changeset/pretty-planets-cough.md
@@ -2,4 +2,8 @@
 "@apollo/client": patch
 ---
 
-Changed typings of updateQuery's previousQueryResult to be potentially undefined
+Added option to skip updateQuery when cache has partial or no data.
+Will become default behavior in v4.
+
+Fixed typescript type of `variables` in query.subscribeToMore's callback's options.
+Added `subscriptionVariables` to the options to access the subscription's variables.
diff --git a/src/__tests__/ApolloClient.ts b/src/__tests__/ApolloClient.ts
@@ -3219,32 +3219,24 @@ describe("ApolloClient", () => {
       >();
 
       observableQuery.updateQuery((previousData) => {
-        expectTypeOf(previousData).toMatchTypeOf<Partial<UnmaskedQuery>>();
-        if (previousData.complete) {
-          expectTypeOf(previousData).toMatchTypeOf<UnmaskedQuery>();
-          expectTypeOf(previousData).not.toMatchTypeOf<Query>();
-        } else {
-          expectTypeOf(previousData).toMatchTypeOf<Partial<UnmaskedQuery>>();
-          expectTypeOf(previousData).not.toMatchTypeOf<Query>();
-        }
+        expectTypeOf(previousData).toEqualTypeOf<UnmaskedQuery>();
+        expectTypeOf(previousData).not.toMatchTypeOf<Query>();
 
         return undefined;
       });
 
       observableQuery.subscribeToMore({
         document: subscription,
         updateQuery(queryData, { subscriptionData }) {
-          expectTypeOf(queryData).toMatchTypeOf<
-            UnmaskedQuery | Partial<UnmaskedQuery>
-          >();
+          expectTypeOf(queryData).toEqualTypeOf<UnmaskedQuery>();
           expectTypeOf(queryData).not.toMatchTypeOf<Query>();
 
           expectTypeOf(
             subscriptionData.data
           ).toMatchTypeOf<UnmaskedSubscription>();
           expectTypeOf(subscriptionData.data).not.toMatchTypeOf<Subscription>();
 
-          return {} as UnmaskedQuery;
+          return undefined;
         },
       });
     });

diff --git a/src/__tests__/dataMasking.ts b/src/__tests__/dataMasking.ts
@@ -1892,12 +1892,6 @@ describe("client.watchQuery", () => {
 
     const updateQuery: Parameters<typeof observable.updateQuery>[0] = jest.fn(
       (previousResult) => {
-        expect(previousResult.complete).toBeTruthy();
-        expect(typeof previousResult.complete).toBe("symbol");
-        // Type guard
-        if (!previousResult.complete) {
-          return undefined;
-        }
         return { user: { ...previousResult.user, name: "User (updated)" } };
       }
     );

diff --git a/src/__tests__/subscribeToMore.ts b/src/__tests__/subscribeToMore.ts
@@ -241,12 +241,6 @@ describe("subscribeToMore", () => {
         }
       `,
       updateQuery: (prev, { subscriptionData }) => {
-        expect(prev.complete).toBeTruthy();
-        expect(typeof prev.complete).toBe("symbol");
-        // Type guard
-        if (!prev.complete) {
-          return undefined;
-        }
         expect(prev.entry).not.toContainEqual(nextMutation);
         return {
           entry: [...prev.entry, { value: subscriptionData.data.name }],

diff --git a/src/core/ObservableQuery.ts b/src/core/ObservableQuery.ts
@@ -32,6 +32,7 @@ import type {
   NextFetchPolicyContext,
   WatchQueryFetchPolicy,
   UpdateQueryMapFn,
+  UpdateQueryFnOptions,
 } from "./watchQueryOptions.js";
 import type { QueryInfo } from "./QueryInfo.js";
 import type { MissingFieldError } from "../cache/index.js";
@@ -55,10 +56,6 @@ export interface FetchMoreOptions<
   ) => TData;
 }
 
-export interface UpdateQueryOptions<TVariables> {
-  variables?: TVariables;
-}
-
 interface Last<TData, TVariables> {
   result: ApolloQueryResult<TData>;
   variables?: TVariables;
@@ -623,7 +620,7 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`,
       TSubscriptionData,
       TVariables
     >
-  ) {
+  ): () => void {
     const subscription = this.queryManager
       .startGraphQLSubscription({
         query: options.document,
@@ -634,12 +631,15 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`,
         next: (subscriptionData: { data: Unmasked<TSubscriptionData> }) => {
           const { updateQuery } = options;
           if (updateQuery) {
-            this.updateQuery((previous, { variables }) =>
-              updateQuery(previous, {
-                subscriptionData,
-                subscriptionVariables: options.variables,
-                variables,
-              })
+            this.updateQuery(
+              (previous, { variables, complete }) =>
+                updateQuery(previous, {
+                  subscriptionData,
+                  subscriptionVariables: options.variables,
+                  variables,
+                  complete,
+                }),
+              options.updateQueryOptions
             );
           }
         },
@@ -724,7 +724,12 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`,
    *
    * See [using updateQuery and updateFragment](https://www.apollographql.com/docs/react/caching/cache-interaction/#using-updatequery-and-updatefragment) for additional information.
    */
-  public updateQuery(mapFn: UpdateQueryMapFn<TData, TVariables>): void {
+  public updateQuery(
+    mapFn: UpdateQueryMapFn<TData, TVariables>,
+    options?: UpdateQueryFnOptions | undefined
+  ): void {
+    const updateQueryOnPartialPreviousResult =
+      options?.updateQueryOnPartialPreviousResult ?? true;
     const { queryManager } = this;
     const { result, complete } = queryManager.cache.diff<Unmasked<TData>>({
       query: this.options.query,
@@ -733,23 +738,16 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`,
       optimistic: false,
     });
 
-    let maybeCompletedResult = result;
-    const completeSymbol = Symbol("complete");
-    if (complete && result) {
-      maybeCompletedResult = { ...result };
-      Object.defineProperty(maybeCompletedResult, "complete", {
-        value: completeSymbol,
-        writable: false,
-        enumerable: false,
-        configurable: false,
-      });
+    if ((!complete || !result) && !updateQueryOnPartialPreviousResult) {
+      return;
     }
 
-    const newResult = mapFn(maybeCompletedResult as any, {
+    const newResult = mapFn(result!, {
       variables: (this as any).variables,
+      complete: !!complete,
     });
 
-    if (newResult && newResult !== maybeCompletedResult) {
+    if (newResult) {
       queryManager.cache.writeQuery({
         query: this.options.query,
         data: newResult,

diff --git a/src/core/index.ts b/src/core/index.ts
@@ -2,10 +2,7 @@
 
 export type { ApolloClientOptions, DefaultOptions } from "./ApolloClient.js";
 export { ApolloClient, mergeOptions } from "./ApolloClient.js";
-export type {
-  FetchMoreOptions,
-  UpdateQueryOptions,
-} from "./ObservableQuery.js";
+export type { FetchMoreOptions } from "./ObservableQuery.js";
 export { ObservableQuery } from "./ObservableQuery.js";
 export type {
   QueryOptions,
@@ -21,7 +18,9 @@ export type {
   SubscribeToMoreOptions,
   SubscribeToMoreFunction,
   UpdateQueryFn,
+  UpdateQueryFnOptions,
   UpdateQueryMapFn,
+  UpdateQueryOptions,
 } from "./watchQueryOptions.js";
 export { NetworkStatus, isNetworkRequestSettled } from "./networkStatus.js";
 export type * from "./types.js";

diff --git a/src/core/watchQueryOptions.ts b/src/core/watchQueryOptions.ts
@@ -164,37 +164,53 @@ export interface FetchMoreQueryOptions<TVariables, TData = any> {
   context?: DefaultContext;
 }
 
+export interface UpdateQueryFnOptions {
+  /**
+   * If true, the mapFn will be called even if there is no previous query result.
+   *
+   * @default `true` Will change to `false` in v4
+   */
+  updateQueryOnPartialPreviousResult?: boolean;
+}
+
 export interface UpdateQueryFn<
   TData,
-  TVars extends OperationVariables,
+  TVariables extends OperationVariables,
   TOptions = {},
 > {
-  (mapFn: UpdateQueryMapFn<TData, TVars, TOptions>): void;
+  (
+    mapFn: UpdateQueryMapFn<TData, TVariables, TOptions>,
+    options?: UpdateQueryFnOptions
+  ): void;
+}
+
+export interface UpdateQueryOptions<TVariables> {
+  variables?: TVariables;
+  /**
+   * Indicate if the previous query result has been found fully in the cache.
+   */
+  complete: boolean;
 }
 
 export interface UpdateQueryMapFn<
   TData = any,
-  TVars extends OperationVariables = OperationVariables,
+  TVariables extends OperationVariables = OperationVariables,
   TOptions = {},
 > {
   (
-    previousQueryResult: // DeepPartial is more accurate, but causes typescript to stop on
-    // "Type instantiation is excessively deep and possibly infinite."
-    // | (DeepPartial<Unmasked<TData>> & { complete?: undefined })
-    | (Partial<Unmasked<TData>> & { complete?: undefined })
-      | (Unmasked<TData> & { complete: Symbol }),
-    options: TOptions & { variables?: TVars }
+    previousQueryResult: Unmasked<TData>,
+    options: TOptions & UpdateQueryOptions<TVariables>
   ): Unmasked<TData> | undefined;
 }
 
 export type SubscribeToMoreUpdateQueryFn<
   TData = any,
-  TVars extends OperationVariables = OperationVariables,
-  TSubscriptionVariables extends OperationVariables = TVars,
+  TVariables extends OperationVariables = OperationVariables,
+  TSubscriptionVariables extends OperationVariables = TVariables,
   TSubscriptionData = TData,
 > = UpdateQueryMapFn<
   TData,
-  TVars,
+  TVariables,
   {
     subscriptionData: { data: Unmasked<TSubscriptionData> };
     subscriptionVariables: TSubscriptionVariables | undefined;
@@ -205,35 +221,36 @@ export interface SubscribeToMoreOptions<
   TData = any,
   TSubscriptionVariables extends OperationVariables = OperationVariables,
   TSubscriptionData = TData,
-  TVars extends OperationVariables = TSubscriptionVariables,
+  TVariables extends OperationVariables = TSubscriptionVariables,
 > {
   document:
     | DocumentNode
     | TypedDocumentNode<TSubscriptionData, TSubscriptionVariables>;
   variables?: TSubscriptionVariables;
   updateQuery?: SubscribeToMoreUpdateQueryFn<
     TData,
-    TVars,
+    TVariables,
     TSubscriptionVariables,
     TSubscriptionData
   >;
+  updateQueryOptions?: UpdateQueryFnOptions;
   onError?: (error: Error) => void;
   context?: DefaultContext;
 }
 
 export interface SubscribeToMoreFunction<
   TData,
-  TVars extends OperationVariables = OperationVariables,
+  TVariables extends OperationVariables = OperationVariables,
 > {
   <
     TSubscriptionData = TData,
-    TSubscriptionVariables extends OperationVariables = TVars,
+    TSubscriptionVariables extends OperationVariables = TVariables,
   >(
     options: SubscribeToMoreOptions<
       TData,
       TSubscriptionVariables,
       TSubscriptionData,
-      TVars
+      TVariables
     >
   ): () => void;
 }

diff --git a/src/react/hoc/types.ts b/src/react/hoc/types.ts
@@ -1,13 +1,14 @@
-import type { ApolloCache, ApolloClient } from "../../core/index.js";
 import type { ApolloError } from "../../errors/index.js";
 import type {
+  ApolloCache,
+  ApolloClient,
   ApolloQueryResult,
-  OperationVariables,
+  DefaultContext,
   FetchMoreOptions,
-  UpdateQueryOptions,
   FetchMoreQueryOptions,
+  OperationVariables,
   SubscribeToMoreOptions,
-  DefaultContext,
+  UpdateQueryFn,
 } from "../../core/index.js";
 import type {
   MutationFunction,
@@ -32,9 +33,7 @@ export interface QueryControls<
   startPolling: (pollInterval: number) => void;
   stopPolling: () => void;
   subscribeToMore: (options: SubscribeToMoreOptions) => () => void;
-  updateQuery: (
-    mapFn: (previousQueryResult: any, options: UpdateQueryOptions<any>) => any
-  ) => void;
+  updateQuery: UpdateQueryFn<TData, any>;
 }
 
 export type DataValue<

diff --git a/src/react/hooks/__tests__/useBackgroundQuery.test.tsx b/src/react/hooks/__tests__/useBackgroundQuery.test.tsx
@@ -8386,10 +8386,7 @@ describe.skip("type tests", () => {
           queryData,
           { subscriptionData, subscriptionVariables, variables }
         ) => {
-          expectTypeOf(queryData).toEqualTypeOf<
-            | (Partial<UnmaskedVariablesCaseData> & { complete?: undefined })
-            | (UnmaskedVariablesCaseData & { complete: Symbol })
-          >();
+          expectTypeOf(queryData).toEqualTypeOf<UnmaskedVariablesCaseData>();
           expectTypeOf(queryData).not.toEqualTypeOf<MaskedVariablesCaseData>();
 
           expectTypeOf(
@@ -8432,10 +8429,7 @@ describe.skip("type tests", () => {
           queryData,
           { subscriptionData, subscriptionVariables, variables }
         ): UnmaskedVariablesCaseData => {
-          expectTypeOf(queryData).toEqualTypeOf<
-            | (Partial<UnmaskedVariablesCaseData> & { complete?: undefined })
-            | (UnmaskedVariablesCaseData & { complete: Symbol })
-          >();
+          expectTypeOf(queryData).toEqualTypeOf<UnmaskedVariablesCaseData>();
           expectTypeOf(queryData).not.toEqualTypeOf<MaskedVariablesCaseData>();
 
           expectTypeOf(
@@ -8449,13 +8443,6 @@ describe.skip("type tests", () => {
             VariablesCaseVariables | undefined
           >();
 
-          if (queryData.complete) {
-            expectTypeOf(queryData).toEqualTypeOf<
-              UnmaskedVariablesCaseData & { complete: Symbol }
-            >();
-            return queryData;
-          }
-          // @ts-expect-error -- The incomplete data cannot be returned
           return queryData;
         },
       });

diff --git a/src/react/hooks/__tests__/useLazyQuery.test.tsx b/src/react/hooks/__tests__/useLazyQuery.test.tsx
@@ -3352,7 +3352,7 @@ describe.skip("Type Tests", () => {
     subscribeToMore({
       document: gql`` as TypedDocumentNode<Subscription, never>,
       updateQuery(queryData, { subscriptionData }) {
-        expectTypeOf(queryData).toEqualTypeOf<UnmaskedQuery | undefined>();
+        expectTypeOf(queryData).toEqualTypeOf<UnmaskedQuery>();
         expectTypeOf(
           subscriptionData.data
         ).toEqualTypeOf<UnmaskedSubscription>();
@@ -3362,7 +3362,7 @@ describe.skip("Type Tests", () => {
     });
 
     updateQuery((previousData) => {
-      expectTypeOf(previousData).toEqualTypeOf<UnmaskedQuery | undefined>();
+      expectTypeOf(previousData).toEqualTypeOf<UnmaskedQuery>();
 
       return {} as UnmaskedQuery;
     });
@@ -3451,7 +3451,7 @@ describe.skip("Type Tests", () => {
     subscribeToMore({
       document: gql`` as TypedDocumentNode<Subscription, never>,
       updateQuery(queryData, { subscriptionData }) {
-        expectTypeOf(queryData).toEqualTypeOf<UnmaskedQuery | undefined>();
+        expectTypeOf(queryData).toEqualTypeOf<UnmaskedQuery>();
         expectTypeOf(
           subscriptionData.data
         ).toEqualTypeOf<UnmaskedSubscription>();
@@ -3461,7 +3461,7 @@ describe.skip("Type Tests", () => {
     });
 
     updateQuery((previousData) => {
-      expectTypeOf(previousData).toEqualTypeOf<UnmaskedQuery | undefined>();
+      expectTypeOf(previousData).toEqualTypeOf<UnmaskedQuery>();
 
       return {} as UnmaskedQuery;
     });