Refactor useLazyQuery to reuse useInternalState and make execution function call reobserve instead of refetch

src/__tests__/client.ts

@@ -3321,7 +3321,7 @@ describe('@connection', () => {
               expect(context.reason).toBe("after-fetch");
               expect(context.observable).toBe(obs);
               expect(context.options).toBe(obs.options);
-              expect(context.initialPolicy).toBe("cache-first");
+              expect(context.initialFetchPolicy).toBe("cache-first");
 
               // Usually options.nextFetchPolicy applies only once, but a
               // nextFetchPolicy function can set this.nextFetchPolicy

src/core/ObservableQuery.ts

@@ -22,7 +22,6 @@ import {
   WatchQueryOptions,
   FetchMoreQueryOptions,
   SubscribeToMoreOptions,
-  WatchQueryFetchPolicy,
   NextFetchPolicyContext,
 } from './watchQueryOptions';
 import { QueryInfo } from './QueryInfo';
@@ -72,10 +71,6 @@ export class ObservableQuery<
     return this.options.variables;
   }
 
-  // Original value of this.options.fetchPolicy (defaulting to "cache-first"),
-  // from whenever the ObservableQuery was first created.
-  private initialFetchPolicy: WatchQueryFetchPolicy;
-
   private isTornDown: boolean;
   private queryManager: QueryManager<any>;
   private observers = new Set<Observer<ApolloQueryResult<TData>>>();
@@ -145,15 +140,19 @@ export class ObservableQuery<
     // active state
     this.isTornDown = false;
 
-    // query information
-    this.options = options;
+    this.options = {
+      // Remember the initial options.fetchPolicy so we can revert back to this
+      // policy when variables change. This information can also be specified
+      // (or overridden) by providing options.initialFetchPolicy explicitly.
+      initialFetchPolicy: options.fetchPolicy || "cache-first",
+      ...options,
+    };
+
     this.queryId = queryInfo.queryId || queryManager.generateQueryId();
 
     const opDef = getOperationDefinition(options.query);
     this.queryName = opDef && opDef.name && opDef.name.value;
 
-    this.initialFetchPolicy = options.fetchPolicy || "cache-first";
-
     // related classes
     this.queryManager = queryManager;
     this.queryInfo = queryInfo;
@@ -565,7 +564,7 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`);
 
     return this.reobserve({
       // Reset options.fetchPolicy to its original value.
-      fetchPolicy: this.initialFetchPolicy,
+      fetchPolicy: this.options.initialFetchPolicy,
       variables,
     }, NetworkStatus.setVariables);
   }
@@ -619,7 +618,10 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`);
     options: WatchQueryOptions<TVariables, TData>,
   ) {
     if (options.nextFetchPolicy) {
-      const { fetchPolicy = "cache-first" } = options;
+      const {
+        fetchPolicy = "cache-first",
+        initialFetchPolicy = fetchPolicy,
+      } = options;
 
       // When someone chooses "cache-and-network" or "network-only" as their
       // initial FetchPolicy, they often do not want future cache updates to
@@ -631,15 +633,16 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`);
       // options.nextFetchPolicy option provides an easy way to update
       // options.fetchPolicy after the initial network request, without having to
       // call observableQuery.setOptions.
+
       if (typeof options.nextFetchPolicy === "function") {
         options.fetchPolicy = options.nextFetchPolicy(fetchPolicy, {
           reason,
           options,
           observable: this,
-          initialPolicy: this.initialFetchPolicy,
+          initialFetchPolicy,
         });
       } else if (reason === "variables-changed") {
-        options.fetchPolicy = this.initialFetchPolicy;
+        options.fetchPolicy = initialFetchPolicy;
       } else {
         options.fetchPolicy = options.nextFetchPolicy;
       }

src/core/__tests__/ObservableQuery.ts

@@ -1199,7 +1199,7 @@ describe('ObservableQuery', () => {
       });
 
       expect(observable.options.fetchPolicy).toBe('cache-and-network');
-      expect(observable["initialFetchPolicy"]).toBe('cache-and-network');
+      expect(observable.options.initialFetchPolicy).toBe('cache-and-network');
 
       subscribeAndCount(reject, observable, (handleCount, result) => {
         expect(result.error).toBeUndefined();

src/core/__tests__/fetchPolicies.ts

@@ -938,7 +938,8 @@ describe("nextFetchPolicy", () => {
     expect(currentFetchPolicy).toBe(context.options.fetchPolicy);
     switch (context.reason) {
       case "variables-changed":
-        return context.initialPolicy;
+        expect(context.initialFetchPolicy).toBe(context.options.initialFetchPolicy);
+        return context.initialFetchPolicy;
       default:
       case "after-fetch":
         return "cache-first";
@@ -1070,7 +1071,8 @@ describe("nextFetchPolicy", () => {
     expect(currentFetchPolicy).toBe(context.options.fetchPolicy);
     switch (context.reason) {
       case "variables-changed":
-        return context.initialPolicy;
+        expect(context.initialFetchPolicy).toBe(context.options.initialFetchPolicy);
+        return context.initialFetchPolicy;
       default:
       case "after-fetch":
         return "cache-first";

src/core/watchQueryOptions.ts

@@ -121,6 +121,7 @@ export interface WatchQueryOptions<TVariables = OperationVariables, TData = any>
    * Specifies the {@link FetchPolicy} to be used for this query.
    */
   fetchPolicy?: WatchQueryFetchPolicy;
+
   /**
    * Specifies the {@link FetchPolicy} to be used after this query has completed.
    */
@@ -129,6 +130,14 @@ export interface WatchQueryOptions<TVariables = OperationVariables, TData = any>
     currentFetchPolicy: WatchQueryFetchPolicy,
     context: NextFetchPolicyContext<TData, TVariables>,
   ) => WatchQueryFetchPolicy);
+
+  /**
+   * Defaults to the initial value of options.fetchPolicy, but can be explicitly
+   * configured to specify the WatchQueryFetchPolicy to revert back to whenever
+   * variables change (unless nextFetchPolicy intervenes).
+   */
+  initialFetchPolicy?: WatchQueryFetchPolicy;
+
   /**
    * Specifies whether a {@link NetworkStatus.refetch} operation should merge
    * incoming field data with existing data, or overwrite the existing data.
@@ -144,7 +153,7 @@ export interface NextFetchPolicyContext<TData, TVariables> {
     | "variables-changed";
   observable: ObservableQuery<TData, TVariables>;
   options: WatchQueryOptions<TVariables, TData>;
-  initialPolicy: WatchQueryFetchPolicy;
+  initialFetchPolicy: WatchQueryFetchPolicy;
 }
 
 export interface FetchMoreQueryOptions<TVariables, TData = any> {

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

@@ -19,6 +19,7 @@ import { ApolloLink } from '../../../link/core';
 import { itAsync, MockLink, MockedProvider, mockSingleLink } from '../../../testing';
 import { useQuery } from '../useQuery';
 import { useMutation } from '../useMutation';
+import { QueryResult } from '../../types/types';
 
 describe('useQuery Hook', () => {
   describe('General use', () => {
@@ -3374,6 +3375,113 @@ describe('useQuery Hook', () => {
         networkStatus: 7,
       });
     });
+
+    it('should set correct initialFetchPolicy even if skip:true', async () => {
+      const query = gql`{ hello }`;
+      let linkCount = 0;
+      const link = new ApolloLink(() => Observable.of({
+        data: { hello: ++linkCount },
+      }));
+
+      const client = new ApolloClient({
+        cache: new InMemoryCache(),
+        link,
+      });
+
+      const correctInitialFetchPolicy: WatchQueryFetchPolicy =
+        "cache-and-network";
+
+      const { result, waitForNextUpdate, rerender } = renderHook<{
+        skip: boolean;
+      }, QueryResult>(
+        ({ skip = true }) => useQuery(query, {
+          // Skipping equates to using a fetchPolicy of "standby", but that
+          // should not mean we revert to standby whenever we want to go back to
+          // the initial fetchPolicy (e.g. when variables change).
+          skip,
+          fetchPolicy: correctInitialFetchPolicy,
+        }),
+        {
+          initialProps: {
+            skip: true,
+          },
+          wrapper: ({ children }) => (
+            <ApolloProvider client={client}>
+              {children}
+            </ApolloProvider>
+          ),
+        },
+      );
+
+      expect(result.current.loading).toBe(false);
+      expect(result.current.data).toBeUndefined();
+
+      function check(
+        expectedFetchPolicy: WatchQueryFetchPolicy,
+        expectedInitialFetchPolicy: WatchQueryFetchPolicy,
+      ) {
+        const { observable } = result.current;
+        const {
+          fetchPolicy,
+          initialFetchPolicy,
+        } = observable.options;
+
+        expect(fetchPolicy).toBe(expectedFetchPolicy);
+        expect(initialFetchPolicy).toBe(expectedInitialFetchPolicy);
+      }
+
+      check(
+        "standby",
+        correctInitialFetchPolicy,
+      );
+
+      rerender({
+        skip: false,
+      });
+
+      await waitForNextUpdate();
+
+      expect(result.current.loading).toBe(false);
+      expect(result.current.data).toEqual({
+        hello: 1,
+      });
+
+      check(
+        correctInitialFetchPolicy,
+        correctInitialFetchPolicy,
+      );
+
+      const reasons: string[] = [];
+
+      const reobservePromise = result.current.observable.reobserve({
+        variables: {
+          newVar: true,
+        },
+        nextFetchPolicy(currentFetchPolicy, context) {
+          expect(currentFetchPolicy).toBe("cache-and-network");
+          expect(context.initialFetchPolicy).toBe("cache-and-network");
+          reasons.push(context.reason);
+          return currentFetchPolicy;
+        },
+      }).then(result => {
+        expect(result.loading).toBe(false);
+        expect(result.data).toEqual({ hello: 2 });
+      });
+
+      await waitForNextUpdate();
+
+      expect(result.current.loading).toBe(false);
+      expect(result.current.data).toEqual({
+        hello: 2,
+      });
+
+      await reobservePromise;
+
+      expect(reasons).toEqual([
+        "variables-changed",
+        "after-fetch",
+      ]);
+    });
   });
 
   describe('Missing Fields', () => {

src/react/hooks/useLazyQuery.ts

@@ -44,8 +44,6 @@ export function useLazyQuery<TData = any, TVariables = OperationVariables>(
   const useQueryResult = internalState.useQuery({
     ...options,
     ...execOptionsRef.current,
-    // TODO This probably means this.observable.initialFetchPolicy is never set
-    // appropriately.
     skip: !execOptionsRef.current,
   });
 

src/react/hooks/useQuery.ts

@@ -220,9 +220,7 @@ class InternalState<TData, TVariables> {
     const watchQueryOptions: WatchQueryOptions<TVariables, TData> =
       Object.assign(merged, { query: this.query });
 
-    if (skip) {
-      watchQueryOptions.fetchPolicy = 'standby';
-    } else if (
+    if (
       this.renderPromises &&
       (
         watchQueryOptions.fetchPolicy === 'network-only' ||
@@ -239,6 +237,24 @@ class InternalState<TData, TVariables> {
       watchQueryOptions.fetchPolicy = 'cache-first';
     }
 
+    if (skip) {
+      const {
+        // The watchQueryOptions.initialFetchPolicy field usually defaults to
+        // watchQueryOptions.fetchPolicy, which has now been properly
+        // defaulted/initialized. However, watchQueryOptions.initialFetchPolicy
+        // can be provided explicitly instead, if more control is desired.
+        initialFetchPolicy = watchQueryOptions.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',
+      });
+    }
+
     if (!watchQueryOptions.variables) {
       watchQueryOptions.variables = {} as TVariables;
     }