Call onCompleted each time useLazyQuery executes

Make sure `onCompleted` is called each time a `useLazyQuery` based query completes, after the execution function is called. Fixes #3488
apollographql · Sep 15, 2019 · 65fb106 · 65fb106
1 parent 5598ddd
commit 65fb106
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 7 deletions.
diff --git a/Changelog.md b/Changelog.md
@@ -19,6 +19,8 @@
   [@jet2jet](https://github.com/jet2jet) in [#3477](https://github.com/apollographql/react-apollo/pull/3477)
 - Add back in the removed `ChildDataProps` and `ChildMutateProps` types. <br/>
   [@hwillson](https://github.com/hwillson) in [#3495](https://github.com/apollographql/react-apollo/pull/3495)
+- Make sure `onCompleted` is called each time a `useLazyQuery` based query completes, after the execution function is called. <br/>
+  [@hwillson](https://github.com/hwillson) in [#TODO](https://github.com/apollographql/react-apollo/pull/TODO)
 
 ## 3.1.0 (2019-09-06)
 

diff --git a/packages/hooks/src/__tests__/useLazyQuery.test.tsx b/packages/hooks/src/__tests__/useLazyQuery.test.tsx
@@ -1,4 +1,4 @@
-import React from 'react';
+import React, { useState } from 'react';
 import { DocumentNode } from 'graphql';
 import gql from 'graphql-tag';
 import { MockedProvider } from '@apollo/react-testing';
@@ -389,4 +389,62 @@ describe('useLazyQuery Hook', () => {
       });
     }
   );
+
+  it('should only call onCompleted once per query run', async () => {
+    let renderCount = 0;
+    let onCompletedCount = 0;
+    const Component = () => {
+      const [_, setCounter] = useState(0);
+      const [execute, { loading, data }] = useLazyQuery(CAR_QUERY, {
+        onCompleted() {
+          onCompletedCount += 1;
+        }
+      });
+
+      switch (renderCount) {
+        case 0:
+          expect(loading).toEqual(false);
+          setTimeout(() => {
+            execute();
+          });
+          break;
+        case 1:
+          expect(loading).toEqual(true);
+          break;
+        case 2:
+          expect(loading).toEqual(false);
+          expect(data).toEqual(CAR_RESULT_DATA);
+          setTimeout(() => {
+            execute();
+          });
+          break;
+        case 3:
+          expect(loading).toEqual(false);
+          expect(data).toEqual(CAR_RESULT_DATA);
+          // Force a render to help make sure onCompleted isn't called again
+          // since the query isn't re-run.
+          setCounter(1);
+          break;
+        case 4:
+          expect(loading).toEqual(false);
+          expect(data).toEqual(CAR_RESULT_DATA);
+          break;
+        default: // Do nothing
+      }
+
+      renderCount += 1;
+      return null;
+    };
+
+    render(
+      <MockedProvider mocks={CAR_MOCKS}>
+        <Component />
+      </MockedProvider>
+    );
+
+    await wait(() => {
+      expect(onCompletedCount).toBe(2);
+      expect(renderCount).toBe(5);
+    });
+  });
 });
diff --git a/packages/hooks/src/utils/useBaseQuery.ts b/packages/hooks/src/utils/useBaseQuery.ts
@@ -51,12 +51,17 @@ export function useBaseQuery<TData = any, TVariables = OperationVariables>(
     ? (result as QueryTuple<TData, TVariables>)[1]
     : (result as QueryResult<TData, TVariables>);
 
-  useEffect(() => queryData.afterExecute({ lazy }), [
-    queryResult.loading,
-    queryResult.networkStatus,
-    queryResult.error,
-    queryResult.data
-  ]);
+  useEffect(
+    () => queryData.afterExecute({ lazy }),
+    lazy
+      ? undefined
+      : [
+          queryResult.loading,
+          queryResult.networkStatus,
+          queryResult.error,
+          queryResult.data
+        ]
+  );
 
   useEffect(() => {
     return () => queryData.cleanup();