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);
   }
@@ -616,10 +615,13 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`);
     // options.fetchPolicy even if options !== this.options, though that happens
     // most often when the options are temporary, used for only one request and
     // then thrown away, so nextFetchPolicy may not end up mattering.
-    options: WatchQueryOptions<TVariables, TData> = this.options,
+    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;
       }
@@ -755,6 +758,7 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`);
 
     // Save the old variables, since Object.assign may modify them below.
     const oldVariables = this.options.variables;
+    const oldFetchPolicy = this.options.fetchPolicy;
 
     const mergedOptions = mergeOptions(this.options, newOptions || {});
     const options = useDisposableConcast
@@ -772,10 +776,10 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`);
       if (
         newOptions &&
         newOptions.variables &&
-        !newOptions.fetchPolicy &&
-        !equal(newOptions.variables, oldVariables)
+        !equal(newOptions.variables, oldVariables) &&
+        (!newOptions.fetchPolicy || newOptions.fetchPolicy === oldFetchPolicy)
       ) {
-        this.applyNextFetchPolicy("variables-changed");
+        this.applyNextFetchPolicy("variables-changed", options);
         if (newNetworkStatus === void 0) {
           newNetworkStatus = NetworkStatus.setVariables;
         }

src/core/QueryManager.ts

@@ -1060,11 +1060,12 @@ export class QueryManager<TStore> {
         const aqr: ApolloQueryResult<TData> = {
           data: result.data,
           loading: false,
-          networkStatus: queryInfo.networkStatus || NetworkStatus.ready,
+          networkStatus: NetworkStatus.ready,
         };
 
         if (hasErrors && options.errorPolicy !== "ignore") {
           aqr.errors = result.errors;
+          aqr.networkStatus = NetworkStatus.error;
         }
 
         return aqr;

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/components/__tests__/client/Query.test.tsx

@@ -59,6 +59,7 @@ describe('Query component', () => {
             observable,
             fetchMore,
             refetch,
+            reobserve,
             startPolling,
             stopPolling,
             subscribeToMore,

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

@@ -7,6 +7,7 @@ import { ApolloClient, InMemoryCache } from '../../../core';
 import { ApolloProvider } from '../../../react';
 import { MockedProvider, mockSingleLink } from '../../../testing';
 import { useLazyQuery } from '../useLazyQuery';
+import { QueryResult } from '../../types/types';
 
 describe('useLazyQuery Hook', () => {
   it('should hold query execution until manually triggered', async () => {
@@ -532,18 +533,22 @@ describe('useLazyQuery Hook', () => {
     expect(result.current[1].loading).toBe(false);
     expect(result.current[1].data).toBe(undefined);
     const execute = result.current[0];
-    let executeResult: any;
-    setTimeout(() => {
-      executeResult = execute();
+
+    const executeResult = new Promise<QueryResult<any, any>>(resolve => {
+      setTimeout(() => resolve(execute()));
     });
 
     await waitForNextUpdate();
     expect(result.current[1].loading).toBe(true);
 
     await waitForNextUpdate();
-    expect(result.current[1].loading).toBe(false);
-    expect(result.current[1].data).toEqual({ hello: 'world' });
-    await expect(executeResult).resolves.toEqual(result.current[1]);
+    const latestRenderResult = result.current[1];
+    expect(latestRenderResult.loading).toBe(false);
+    expect(latestRenderResult.data).toEqual({ hello: 'world' });
+
+    return executeResult.then(finalResult => {
+      expect(finalResult).toEqual(latestRenderResult);
+    });
   });
 
   it('should have matching results from execution function and hook', async () => {

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/index.ts

@@ -3,6 +3,6 @@ import '../../utilities/globals';
 export * from './useApolloClient';
 export * from './useLazyQuery';
 export * from './useMutation';
-export * from './useQuery';
+export { useQuery } from './useQuery';
 export * from './useSubscription';
 export * from './useReactiveVar';