Prevent duplicate onCompleted calls during query execution (#3461)

This commit tweaks the dependencies that are watched to decide
if `afterExecute` should be called (which triggers `onCompleted`),
in `useQuery`. This helps prevent extra and un-necessary repeated
calls to `onCompleted`, during a single query request-response
cycle.

Changelog.md

@@ -22,6 +22,8 @@
   [@mikebm](https://github.com/mikebm) in [#3435](https://github.com/apollographql/react-apollo/pull/3435)
 - Remove `void` from the `MutationFunction`'s returned Promise types. <br/>
   [@hwillson](https://github.com/hwillson) in [#3458](https://github.com/apollographql/react-apollo/pull/3458)
+- Prevent duplicate `onCompleted` calls during the same query execution cycle. <br/>
+  [@hwillson](https://github.com/hwillson) in [#3461](https://github.com/apollographql/react-apollo/pull/3461)
 - Documentation fixes. <br/>
   [@SeanRoberts](https://github.com/SeanRoberts) in [#3380](https://github.com/apollographql/react-apollo/pull/3380)
 

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

@@ -804,4 +804,86 @@ describe('useQuery Hook', () => {
       });
     });
   });
+
+  describe('Callbacks', () => {
+    it(
+      'should pass loaded data to onCompleted when using the cache-only ' +
+        'fetch policy',
+      async () => {
+        const cache = new InMemoryCache();
+        const client = new ApolloClient({
+          cache,
+          resolvers: {}
+        });
+
+        cache.writeQuery({
+          query: CAR_QUERY,
+          data: CAR_RESULT_DATA
+        });
+
+        let onCompletedCalled = false;
+        const Component = () => {
+          const { loading, data } = useQuery(CAR_QUERY, {
+            fetchPolicy: 'cache-only',
+            onCompleted(data) {
+              onCompletedCalled = true;
+              expect(data).toBeDefined();
+              console.log(data);
+            }
+          });
+          if (!loading) {
+            expect(data).toEqual(CAR_RESULT_DATA);
+          }
+          return null;
+        };
+
+        render(
+          <ApolloProvider client={client}>
+            <Component />
+          </ApolloProvider>
+        );
+
+        await wait(() => {
+          expect(onCompletedCalled).toBeTruthy();
+        });
+      }
+    );
+
+    it('should only call onCompleted once per query run', async () => {
+      const cache = new InMemoryCache();
+      const client = new ApolloClient({
+        cache,
+        resolvers: {}
+      });
+
+      cache.writeQuery({
+        query: CAR_QUERY,
+        data: CAR_RESULT_DATA
+      });
+
+      let onCompletedCount = 0;
+      const Component = () => {
+        const { loading, data } = useQuery(CAR_QUERY, {
+          fetchPolicy: 'cache-only',
+          onCompleted() {
+            onCompletedCount += 1;
+          }
+        });
+        if (!loading) {
+          expect(data).toEqual(CAR_RESULT_DATA);
+        }
+        return null;
+      };
+
+      render(
+        <ApolloProvider client={client}>
+          <Component />
+        </ApolloProvider>
+      );
+
+      await wait(() => {
+        expect(onCompletedCount).toBe(1);
+      });
+    });
+  });
 });

packages/hooks/src/data/QueryData.ts

@@ -1,6 +1,5 @@
 import {
   ApolloQueryResult,
-  ObservableQuery,
   ApolloError,
   NetworkStatus,
   FetchMoreOptions,

packages/hooks/src/utils/useBaseQuery.ts

@@ -1,8 +1,12 @@
 import { useContext, useEffect, useReducer, useRef } from 'react';
-import { getApolloContext, OperationVariables } from '@apollo/react-common';
+import {
+  getApolloContext,
+  OperationVariables,
+  QueryResult
+} from '@apollo/react-common';
 import { DocumentNode } from 'graphql';
 
-import { QueryHookOptions, QueryOptions } from '../types';
+import { QueryHookOptions, QueryOptions, QueryTuple } from '../types';
 import { QueryData } from '../data/QueryData';
 import { useDeepMemo } from './useDeepMemo';
 
@@ -43,7 +47,16 @@ export function useBaseQuery<TData = any, TVariables = OperationVariables>(
     memo
   );
 
-  useEffect(() => queryData.afterExecute({ lazy }), [result]);
+  const queryResult = lazy
+    ? (result as QueryTuple<TData, TVariables>)[1]
+    : (result as QueryResult<TData, TVariables>);
+
+  useEffect(() => queryData.afterExecute({ lazy }), [
+    queryResult.loading,
+    queryResult.networkStatus,
+    queryResult.error,
+    queryResult.data
+  ]);
 
   useEffect(() => {
     return () => queryData.cleanup();