Make sure onCompleted does not fire when skip is true (#6589)

Fixes #6122

CHANGELOG.md

@@ -1,3 +1,10 @@
+## Apollo Client 3.0.1 (TBD - not yet released)
+
+## Bug Fixes
+
+- Make sure `useQuery` `onCompleted` is not fired when `skip` is `true`.  <br/>
+  [@hwillson](https://github.com/hwillson) in [#6589](https://github.com/apollographql/apollo-client/pull/6589)
+
 # Apollo Client 3.0.0
 
 ## Improvements

src/react/data/QueryData.ts

@@ -412,7 +412,13 @@ export class QueryData<TData, TVariables> extends OperationData {
     const { data, loading, error } = this.previousData.result;
 
     if (!loading) {
-      const { query, variables, onCompleted, onError } = this.getOptions();
+      const {
+        query,
+        variables,
+        onCompleted,
+        onError,
+        skip
+      } = this.getOptions();
 
       // No changes, so we won't call onError/onCompleted.
       if (
@@ -424,7 +430,7 @@ export class QueryData<TData, TVariables> extends OperationData {
         return;
       }
 
-      if (onCompleted && !error) {
+      if (onCompleted && !error && !skip) {
         onCompleted(data);
       } else if (onError && error) {
         onError(error);

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

@@ -1798,6 +1798,27 @@ describe('useQuery Hook', () => {
         expect(renderCount).toBe(3);
       }).then(resolve, reject);
     });
+
+    itAsync('should not call onCompleted if skip is true', (resolve, reject) => {
+      function Component() {
+        const { loading } = useQuery(CAR_QUERY, {
+          skip: true,
+          onCompleted() {
+            fail('should not call onCompleted!');
+          }
+        });
+        expect(loading).toBeFalsy();
+        return null;
+      }
+
+      render(
+        <MockedProvider mocks={CAR_MOCKS}>
+          <Component />
+        </MockedProvider>
+      );
+
+      return wait().then(resolve, reject);
+    });
   });
 
   describe('Optimistic data', () => {