Merge pull request #9762 from apollographql/fix-useLazyQuery-defaultO…

…ptions-merging

Allow `useLazyQuery(query, { defaultOptions })` to benefit from `defaultOptions.variables`

CHANGELOG.md

@@ -1,3 +1,10 @@
+## Apollo Client 3.6.6 (unreleased)
+
+### Bug Fixes
+
+- Allow `useLazyQuery(query, { defaultOptions })` to benefit from `defaultOptions.variables` and `client.defaultOptions.watchQuery.variables` merging. <br/>
+  [@benjamn](https://github.com/benjamn) in [#9762](https://github.com/apollographql/apollo-client/pull/9762)
+
 ## Apollo Client 3.6.5 (2022-05-23)
 
 ### Bug Fixes

package.json

@@ -56,7 +56,7 @@
     {
       "name": "apollo-client",
       "path": "./dist/apollo-client.min.cjs",
-      "maxSize": "29.55kB"
+      "maxSize": "29.6kB"
     }
   ],
   "engines": {

src/react/hooks/__tests__/useLazyQuery.test.tsx

@@ -229,12 +229,16 @@ describe('useLazyQuery Hook', () => {
     const counterQuery: TypedDocumentNode<{
       counter: number;
     }, {
-      hookVar?: any;
-      execVar?: any;
+      hookVar?: boolean;
+      execVar?: boolean;
+      localDefaultVar?: boolean;
+      globalDefaultVar?: boolean;
     }> = gql`
       query GetCounter (
         $hookVar: Boolean
         $execVar: Boolean
+        $localDefaultVar: Boolean
+        $globalDefaultVar: Boolean
       ) {
         counter
         vars
@@ -243,6 +247,13 @@ describe('useLazyQuery Hook', () => {
 
     let count = 0;
     const client = new ApolloClient({
+      defaultOptions: {
+        watchQuery: {
+          variables: {
+            globalDefaultVar: true,
+          },
+        },
+      },
       cache: new InMemoryCache(),
       link: new ApolloLink(request => new Observable(observer => {
         if (request.operationName === "GetCounter") {
@@ -270,6 +281,11 @@ describe('useLazyQuery Hook', () => {
           variables: {
             hookVar: true,
           },
+          defaultOptions: {
+            variables: {
+              localDefaultVar: true,
+            },
+          },
         });
         return {
           exec,
@@ -296,6 +312,8 @@ describe('useLazyQuery Hook', () => {
     const expectedFinalData = {
       counter: 1,
       vars: {
+        globalDefaultVar: true,
+        localDefaultVar: true,
         hookVar: true,
         execVar: true,
       },
@@ -304,7 +322,7 @@ describe('useLazyQuery Hook', () => {
     const execPromise = act(() => result.current.exec({
       variables: {
         execVar: true,
-      }
+      },
     }).then(finalResult => {
       expect(finalResult.loading).toBe(false);
       expect(finalResult.called).toBe(true);
@@ -324,6 +342,80 @@ describe('useLazyQuery Hook', () => {
     await expect(waitForNextUpdate({
       timeout: 20,
     })).rejects.toThrow('Timed out');
+
+    const refetchPromise = act(() => result.current.query.reobserve({
+      fetchPolicy: "network-only",
+      nextFetchPolicy: "cache-first",
+      variables: {
+        execVar: false,
+      },
+    }).then(finalResult => {
+      expect(finalResult.loading).toBe(false);
+      expect(finalResult.data).toEqual({
+        counter: 2,
+        vars: {
+          execVar: false,
+        },
+      });
+    }));
+    await waitForNextUpdate();
+    expect(result.current.query.loading).toBe(false);
+    expect(result.current.query.called).toBe(true);
+    expect(result.current.query.data).toEqual({
+      counter: 2,
+      vars: {
+        execVar: false,
+      },
+    });
+
+    await refetchPromise;
+
+    await expect(waitForNextUpdate({
+      timeout: 20,
+    })).rejects.toThrow('Timed out');
+
+    const execPromise2 = act(() => result.current.exec({
+      fetchPolicy: "cache-and-network",
+      nextFetchPolicy: "cache-first",
+      variables: {
+        execVar: true,
+      },
+    }).then(finalResult => {
+      expect(finalResult.loading).toBe(false);
+      expect(finalResult.called).toBe(true);
+      expect(finalResult.data).toEqual({
+        counter: 3,
+        vars: {
+          ...expectedFinalData.vars,
+          execVar: true,
+        },
+      });
+    }));
+
+    expect(result.current.query.loading).toBe(true);
+    expect(result.current.query.called).toBe(true);
+    expect(result.current.query.data).toEqual({
+      counter: 2,
+      vars: {
+        execVar: false,
+      },
+    });
+    await waitForNextUpdate();
+    expect(result.current.query.loading).toBe(false);
+    expect(result.current.query.called).toBe(true);
+    expect(result.current.query.data).toEqual({
+      counter: 3,
+      vars: {
+        ...expectedFinalData.vars,
+        execVar: true,
+      },
+    });
+
+    await execPromise2;
+
+    await expect(waitForNextUpdate({
+      timeout: 20,
+    })).rejects.toThrow('Timed out');
   });
 
   it('should fetch data each time the execution function is called, when using a "network-only" fetch policy', async () => {

src/react/hooks/useQuery.ts

@@ -30,7 +30,7 @@ import {
 
 import { DocumentType, verifyDocumentType } from '../parser';
 import { useApolloClient } from './useApolloClient';
-import { canUseWeakMap, canUseWeakSet, isNonEmptyArray, maybeDeepFreeze } from '../../utilities';
+import { canUseWeakMap, canUseWeakSet, compact, isNonEmptyArray, maybeDeepFreeze } from '../../utilities';
 
 const {
   prototype: {
@@ -276,7 +276,7 @@ class InternalState<TData, TVariables> {
         // 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(watchQueryOptions);
+        this.observable.reobserve(this.getObsQueryOptions());
 
         // Make sure getCurrentResult returns a fresh ApolloQueryResult<TData>,
         // but save the current data as this.previousData, just like setResult
@@ -326,6 +326,38 @@ class InternalState<TData, TVariables> {
     }
   }
 
+  private getObsQueryOptions(): WatchQueryOptions<TVariables, TData> {
+    const toMerge: Array<
+      Partial<WatchQueryOptions<TVariables, TData>>
+    > = [];
+
+    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<TVariables, TData>;
+  }
+
   private ssrDisabledResult = maybeDeepFreeze({
     loading: true,
     data: void 0 as unknown as TData,
@@ -424,13 +456,7 @@ class InternalState<TData, TVariables> {
       this.renderPromises
         && this.renderPromises.getSSRObservable(this.watchQueryOptions)
         || this.observable // Reuse this.observable if possible (and not SSR)
-        || this.client.watchQuery(mergeOptions(
-          // Any options.defaultOptions passed to useQuery serve as default
-          // options because we use them only here, when first creating the
-          // ObservableQuery by calling client.watchQuery.
-          this.queryHookOptions.defaultOptions,
-          this.watchQueryOptions,
-        ));
+        || this.client.watchQuery(this.getObsQueryOptions());
 
     this.obsQueryFields = useMemo(() => ({
       refetch: obsQuery.refetch.bind(obsQuery),