Fix value of ObservableQuery.variables to be undefined when variables are empty (#12543)

Author: jerelmiller

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

@@ -1234,7 +1234,7 @@ interface MutationStoreValue {
     // (undocumented)
     mutation: DocumentNode_2;
     // (undocumented)
-    variables: Record<string, any>;
+    variables: Record<string, any> | undefined;
 }
 
 // @public @deprecated (undocumented)
@@ -1982,7 +1982,7 @@ export interface WatchQueryOptions<TVariables extends OperationVariables = Opera
     refetchWritePolicy?: RefetchWritePolicy;
     returnPartialData?: boolean;
     skipPollAttempt?: () => boolean;
-    variables?: TVariables;
+    variables?: NoInfer_2<TVariables>;
 }
 
 // @public (undocumented)
@@ -2025,10 +2025,10 @@ interface WriteContext extends ReadMergeModifyContext {
 // src/cache/inmemory/policies.ts:167:3 - (ae-forgotten-export) The symbol "FieldReadFunction_2" needs to be exported by the entry point index.d.ts
 // src/cache/inmemory/policies.ts:168:3 - (ae-forgotten-export) The symbol "FieldMergeFunction_2" needs to be exported by the entry point index.d.ts
 // src/cache/inmemory/types.ts:133:3 - (ae-forgotten-export) The symbol "KeyFieldsFunction" needs to be exported by the entry point index.d.ts
-// src/core/ObservableQuery.ts:140:5 - (ae-forgotten-export) The symbol "QueryManager" needs to be exported by the entry point index.d.ts
-// src/core/ObservableQuery.ts:141:5 - (ae-forgotten-export) The symbol "QueryInfo" needs to be exported by the entry point index.d.ts
-// src/core/QueryManager.ts:184:5 - (ae-forgotten-export) The symbol "MutationStoreValue" needs to be exported by the entry point index.d.ts
-// src/core/QueryManager.ts:454:7 - (ae-forgotten-export) The symbol "UpdateQueries" needs to be exported by the entry point index.d.ts
+// src/core/ObservableQuery.ts:143:5 - (ae-forgotten-export) The symbol "QueryManager" needs to be exported by the entry point index.d.ts
+// src/core/ObservableQuery.ts:144:5 - (ae-forgotten-export) The symbol "QueryInfo" needs to be exported by the entry point index.d.ts
+// src/core/QueryManager.ts:188:5 - (ae-forgotten-export) The symbol "MutationStoreValue" needs to be exported by the entry point index.d.ts
+// src/core/QueryManager.ts:460:7 - (ae-forgotten-export) The symbol "UpdateQueries" needs to be exported by the entry point index.d.ts
 
 // (No @packageDocumentation comment for this package)
 

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

@@ -510,7 +510,7 @@ interface MutationStoreValue {
     // (undocumented)
     mutation: DocumentNode;
     // (undocumented)
-    variables: Record<string, any>;
+    variables: Record<string, any> | undefined;
 }
 
 // @public @deprecated (undocumented)
@@ -1598,16 +1598,16 @@ interface WatchQueryOptions_2<TVariables extends OperationVariables_2 = Operatio
     refetchWritePolicy?: RefetchWritePolicy;
     returnPartialData?: boolean;
     skipPollAttempt?: () => boolean;
-    variables?: TVariables;
+    variables?: NoInfer_2<TVariables>;
 }
 
 // Warnings were encountered during analysis:
 //
 // src/core/LocalState.ts:71:3 - (ae-forgotten-export) The symbol "ApolloClient_2" needs to be exported by the entry point index.d.ts
-// src/core/ObservableQuery.ts:140:5 - (ae-forgotten-export) The symbol "QueryManager" needs to be exported by the entry point index.d.ts
-// src/core/ObservableQuery.ts:141:5 - (ae-forgotten-export) The symbol "QueryInfo" needs to be exported by the entry point index.d.ts
-// src/core/QueryManager.ts:184:5 - (ae-forgotten-export) The symbol "MutationStoreValue" needs to be exported by the entry point index.d.ts
-// src/core/QueryManager.ts:454:7 - (ae-forgotten-export) The symbol "UpdateQueries" needs to be exported by the entry point index.d.ts
+// src/core/ObservableQuery.ts:143:5 - (ae-forgotten-export) The symbol "QueryManager" needs to be exported by the entry point index.d.ts
+// src/core/ObservableQuery.ts:144:5 - (ae-forgotten-export) The symbol "QueryInfo" needs to be exported by the entry point index.d.ts
+// src/core/QueryManager.ts:188:5 - (ae-forgotten-export) The symbol "MutationStoreValue" needs to be exported by the entry point index.d.ts
+// src/core/QueryManager.ts:460:7 - (ae-forgotten-export) The symbol "UpdateQueries" needs to be exported by the entry point index.d.ts
 // src/core/types.ts:204:3 - (ae-forgotten-export) The symbol "MutationQueryReducer" needs to be exported by the entry point index.d.ts
 // src/core/types.ts:233:5 - (ae-forgotten-export) The symbol "Resolver" needs to be exported by the entry point index.d.ts
 // src/core/watchQueryOptions.ts:195:3 - (ae-forgotten-export) The symbol "UpdateQueryOptions" needs to be exported by the entry point index.d.ts

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

@@ -76,6 +76,9 @@ const globalCaches: {
     canonicalStringify?: () => number;
 };
 
+// @public
+export function normalizeVariables<TVariables>(variables: TVariables): NonNullable<TVariables> | undefined;
+
 // @public (undocumented)
 type ObservableEvent<T> = {
     type: "complete";

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

@@ -1520,7 +1520,7 @@ interface MutationStoreValue {
     // (undocumented)
     mutation: DocumentNode;
     // (undocumented)
-    variables: Record<string, any>;
+    variables: Record<string, any> | undefined;
 }
 
 // @public @deprecated (undocumented)
@@ -2437,7 +2437,7 @@ export interface WatchQueryOptions<TVariables extends OperationVariables = Opera
     refetchWritePolicy?: RefetchWritePolicy;
     returnPartialData?: boolean;
     skipPollAttempt?: () => boolean;
-    variables?: TVariables;
+    variables?: NoInfer_2<TVariables>;
 }
 
 // @public (undocumented)
@@ -2479,10 +2479,10 @@ interface WriteContext extends ReadMergeModifyContext {
 // src/cache/inmemory/policies.ts:166:3 - (ae-forgotten-export) The symbol "KeySpecifier" needs to be exported by the entry point index.d.ts
 // src/cache/inmemory/policies.ts:166:3 - (ae-forgotten-export) The symbol "KeyArgsFunction" needs to be exported by the entry point index.d.ts
 // src/cache/inmemory/types.ts:133:3 - (ae-forgotten-export) The symbol "KeyFieldsFunction" needs to be exported by the entry point index.d.ts
-// src/core/ObservableQuery.ts:140:5 - (ae-forgotten-export) The symbol "QueryManager" needs to be exported by the entry point index.d.ts
-// src/core/ObservableQuery.ts:141:5 - (ae-forgotten-export) The symbol "QueryInfo" needs to be exported by the entry point index.d.ts
-// src/core/QueryManager.ts:184:5 - (ae-forgotten-export) The symbol "MutationStoreValue" needs to be exported by the entry point index.d.ts
-// src/core/QueryManager.ts:454:7 - (ae-forgotten-export) The symbol "UpdateQueries" needs to be exported by the entry point index.d.ts
+// src/core/ObservableQuery.ts:143:5 - (ae-forgotten-export) The symbol "QueryManager" needs to be exported by the entry point index.d.ts
+// src/core/ObservableQuery.ts:144:5 - (ae-forgotten-export) The symbol "QueryInfo" needs to be exported by the entry point index.d.ts
+// src/core/QueryManager.ts:188:5 - (ae-forgotten-export) The symbol "MutationStoreValue" needs to be exported by the entry point index.d.ts
+// src/core/QueryManager.ts:460:7 - (ae-forgotten-export) The symbol "UpdateQueries" needs to be exported by the entry point index.d.ts
 // src/link/http/selectHttpOptionsAndBody.ts:128:1 - (ae-forgotten-export) The symbol "HttpQueryOptions" needs to be exported by the entry point index.d.ts
 
 // (No @packageDocumentation comment for this package)

.changeset/fair-ways-enjoy.md

@@ -0,0 +1,7 @@
+---
+"@apollo/client": patch
+---
+
+`ObservableQuery.variables` now returns `undefined` if no variables were set from `watchQuery` or if `variables` was set to an empty object. This ensures the value of `variables` lines up with the type.
+
+This change also propogates to `useQuery` and `useLazyQuery`. The `variables` property returned from these hooks will be `undefined` instead of an empty object if no `variables` were set.

.changeset/sixty-meals-travel.md

@@ -0,0 +1,13 @@
+---
+"@apollo/client": minor
+---
+
+`ObservableQuery` `variables` can now be reset back to `undefined` by calling `reobserve` with a `variables` key set to `undefined`:
+
+```ts
+observable.reobserve({ variables: undefined });
+```
+
+Previously this would leave `variables` unchanged and would require setting `variables` to an empty object.
+
+Calling `reobserve({ variables: {} })` has the same effect as `undefined` and will reset `ObservableQuery.variables` back to `undefined`.

.changeset/sour-starfishes-pump.md

@@ -0,0 +1,5 @@
+---
+"@apollo/client": patch
+---
+
+Ensure `variables` passed to the `update` callback is `undefined` instead of an empty object when `variables` are not defined.

.size-limits.json

@@ -1,6 +1,6 @@
 {
-  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (CJS)": 42984,
-  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production) (CJS)": 38432,
-  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\"": 32968,
-  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production)": 27861
+  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (CJS)": 43112,
+  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production) (CJS)": 38549,
+  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\"": 32997,
+  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production)": 27868
 }

src/__tests__/__snapshots__/exports.ts.snap

@@ -487,6 +487,7 @@ Array [
   "getApolloCacheMemoryInternals",
   "getApolloClientMemoryInternals",
   "getInMemoryCacheMemoryInternals",
+  "normalizeVariables",
   "onAnyEvent",
   "registerGlobalCache",
   "toQueryResult",

src/__tests__/dataMasking.ts

@@ -5086,7 +5086,7 @@ describe("observableQuery.subscribeToMore", () => {
       },
       {
         complete: true,
-        variables: {},
+        variables: undefined,
         previousData: {
           recentComment: {
             __typename: "Comment",
@@ -5229,7 +5229,7 @@ describe("observableQuery.subscribeToMore", () => {
       },
       {
         complete: true,
-        variables: {},
+        variables: undefined,
         previousData: {
           recentComment: {
             __typename: "Comment",
@@ -5378,7 +5378,7 @@ describe("observableQuery.subscribeToMore", () => {
       },
       {
         complete: true,
-        variables: {},
+        variables: undefined,
         previousData: {
           recentComment: {
             __typename: "Comment",
@@ -5671,7 +5671,7 @@ describe("client.mutate", () => {
           },
         },
       },
-      { context: undefined, variables: {} }
+      { context: undefined, variables: undefined }
     );
   });
 

src/__tests__/mutationResults.ts

@@ -11,7 +11,7 @@ import { InMemoryCache } from "@apollo/client/cache";
 import { CombinedGraphQLErrors } from "@apollo/client/errors";
 import { ApolloLink } from "@apollo/client/link/core";
 import type { MockedResponse } from "@apollo/client/testing";
-import { mockSingleLink } from "@apollo/client/testing";
+import { MockLink, mockSingleLink } from "@apollo/client/testing";
 import {
   ObservableStream,
   spyOnConsole,
@@ -646,16 +646,40 @@ describe("mutation results", () => {
       },
     };
 
-    const { client, obsQuery } = setupObsQuery(
-      {
-        request: { query: queryTodos },
-        result: queryTodosResult,
-      },
-      {
-        request: { query: mutationTodo },
-        result: mutationTodoResult,
-      }
-    );
+    const client = new ApolloClient({
+      link: new MockLink(
+        [
+          {
+            request: { query: queryWithTypename } as any,
+            result,
+          },
+          {
+            request: { query: queryTodos },
+            result: queryTodosResult,
+          },
+          {
+            request: { query: mutationTodo },
+            result: mutationTodoResult,
+          },
+        ],
+        { defaultOptions: { delay: 0 } }
+      ),
+      cache: new InMemoryCache({
+        dataIdFromObject: (obj: any) => {
+          if (obj.id && obj.__typename) {
+            return obj.__typename + obj.id;
+          }
+          return null;
+        },
+        // Passing an empty map enables warnings about missing fields:
+        possibleTypes: {},
+      }),
+    });
+
+    const obsQuery = client.watchQuery({
+      query,
+      notifyOnNetworkStatusChange: false,
+    });
 
     await firstValueFrom(from(obsQuery));
 

src/core/ObservableQuery.ts

@@ -21,7 +21,10 @@ import {
   preventUnhandledRejection,
 } from "@apollo/client/utilities";
 import { __DEV__ } from "@apollo/client/utilities/environment";
-import { toQueryResult } from "@apollo/client/utilities/internal";
+import {
+  normalizeVariables,
+  toQueryResult,
+} from "@apollo/client/utilities/internal";
 import { invariant } from "@apollo/client/utilities/invariant";
 
 import { equalByQuery } from "./equalByQuery.js";
@@ -227,6 +230,7 @@ export class ObservableQuery<
       initialFetchPolicy = fetchPolicy === "standby" ? defaultFetchPolicy : (
         fetchPolicy
       ),
+      variables,
     } = options;
 
     this.options = {
@@ -240,6 +244,10 @@ export class ObservableQuery<
       // This ensures this.options.fetchPolicy always has a string value, in
       // case options.fetchPolicy was not provided.
       fetchPolicy,
+
+      // `variables` might be an empty object due to QueryManager.getVariables,
+      // so we reset to `undefined` if there are no values
+      variables: normalizeVariables(variables),
     };
 
     this.queryId = queryInfo.queryId || queryManager.generateQueryId();
@@ -787,6 +795,8 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`,
   public async setVariables(
     variables: TVariables
   ): Promise<QueryResult<TData>> {
+    variables = normalizeVariables(variables) as TVariables;
+
     if (equal(this.variables, variables)) {
       // If we have no observers, then we don't actually want to make a network
       // request. As soon as someone observes the query, the request will kick
@@ -1043,6 +1053,15 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`,
     const oldFetchPolicy = this.options.fetchPolicy;
 
     const mergedOptions = compact(this.options, newOptions || {});
+
+    // Allow variables to be reset back to `undefined` if explicitly passed as
+    // variables: undefined to reobserve, otherwise `compact` will ignore the
+    // `variables` key from from `newOptions`.
+    mergedOptions.variables =
+      newOptions && "variables" in newOptions ?
+        newOptions.variables
+      : this.options.variables;
+
     const options =
       useDisposableObservable ?
         // Disposable Observable fetches receive a shallow copy of this.options
@@ -1057,6 +1076,10 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`,
     const query = this.transformDocument(options.query);
     const { fetchPolicy } = options;
 
+    // note: This may mutate this.options.variables when not using a disposable
+    // observable. This is intentional
+    options.variables = normalizeVariables(options.variables);
+
     this.lastQuery = query;
 
     if (!useDisposableObservable) {
@@ -1066,9 +1089,7 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`,
       // Reset options.fetchPolicy to its original value when variables change,
       // unless a new fetchPolicy was provided by newOptions.
       if (
-        newOptions &&
-        newOptions.variables &&
-        !equal(newOptions.variables, oldVariables) &&
+        !equal(options.variables, oldVariables) &&
         // Don't mess with the fetchPolicy if it's currently "standby".
         fetchPolicy !== "standby" &&
         // If we're changing the fetchPolicy anyway, don't try to change it here
@@ -1092,8 +1113,7 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`,
 
       if (
         oldNetworkStatus !== NetworkStatus.loading &&
-        newOptions?.variables &&
-        !equal(newOptions.variables, oldVariables)
+        !equal(options.variables, oldVariables)
       ) {
         newNetworkStatus = NetworkStatus.setVariables;
       }