From 65b3a88bea486e41b0a335f0afe65f110dae2498 Mon Sep 17 00:00:00 2001
From: Hugh Willson <hugh@octonary.com>
Date: Fri, 6 Sep 2019 07:16:17 -0400
Subject: [PATCH] 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                                  |  2 +
 .../hooks/src/__tests__/useQuery.test.tsx     | 82 +++++++++++++++++++
 packages/hooks/src/data/QueryData.ts          |  1 -
 packages/hooks/src/utils/useBaseQuery.ts      | 19 ++++-
 4 files changed, 100 insertions(+), 4 deletions(-)

diff --git a/Changelog.md b/Changelog.md
index 79abc4eba3..ee68644c43 100644
--- a/Changelog.md
+++ b/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)
 
diff --git a/packages/hooks/src/__tests__/useQuery.test.tsx b/packages/hooks/src/__tests__/useQuery.test.tsx
index ae25712d83..3be04ad41d 100644
--- a/packages/hooks/src/__tests__/useQuery.test.tsx
+++ b/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);
+      });
+    });
+  });
 });
diff --git a/packages/hooks/src/data/QueryData.ts b/packages/hooks/src/data/QueryData.ts
index 8b9e93588b..d000cc5d23 100644
--- a/packages/hooks/src/data/QueryData.ts
+++ b/packages/hooks/src/data/QueryData.ts
@@ -1,6 +1,5 @@
 import {
   ApolloQueryResult,
-  ObservableQuery,
   ApolloError,
   NetworkStatus,
   FetchMoreOptions,
diff --git a/packages/hooks/src/utils/useBaseQuery.ts b/packages/hooks/src/utils/useBaseQuery.ts
index 42dad7ab25..18922e90be 100644
--- a/packages/hooks/src/utils/useBaseQuery.ts
+++ b/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();