Remove onError and onCompleted from memoized options

Changelog.md

@@ -14,6 +14,8 @@
   [@hwillson](https://github.com/hwillson) in [#3433](https://github.com/apollographql/react-apollo/pull/3433)
 - Add `client` to the `useMutation` result. <br/>
   [@joshalling](https://github.com/joshalling) in [#3417](https://github.com/apollographql/react-apollo/pull/3417)
+- Prevent inline `onError` and `onCompleted` callbacks from being part of the internal memoization that's used to decide when certain after render units of functionality are run, when using `useQuery`. This fixes issues related to un-necessary component cleanup, like `error` disappearing from results when it should be present. <br/>
+  [@dylanwulf](https://github.com/dylanwulf) in [#3419](https://github.com/apollographql/react-apollo/pull/3419)
 - Documentation fixes. <br/>
   [@SeanRoberts](https://github.com/SeanRoberts) in [#3380](https://github.com/apollographql/react-apollo/pull/3380)
 

package.json

@@ -92,7 +92,7 @@
     {
       "name": "@apollo/react-hooks",
       "path": "./packages/hooks/lib/react-hooks.cjs.min.js",
-      "maxSize": "4 kB"
+      "maxSize": "4.02 kB"
     },
     {
       "name": "@apollo/react-ssr",

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

@@ -434,6 +434,70 @@ describe('useQuery Hook', () => {
         </MockedProvider>
       );
     });
+
+    it(
+      'should persist errors on re-render when inlining onError and/or ' +
+        'onCompleted callbacks',
+      async () => {
+        const query = gql`
+          query SomeQuery {
+            stuff {
+              thing
+            }
+          }
+        `;
+
+        const mocks = [
+          {
+            request: { query },
+            result: {
+              errors: [new GraphQLError('forced error')]
+            }
+          }
+        ];
+
+        let renderCount = 0;
+        function App() {
+          const [_, forceUpdate] = useReducer(x => x + 1, 0);
+          const { loading, error } = useQuery(query, {
+            onError: () => {},
+            onCompleted: () => {}
+          });
+
+          switch (renderCount) {
+            case 0:
+              expect(loading).toBeTruthy();
+              expect(error).toBeUndefined();
+              break;
+            case 1:
+              expect(error).toBeDefined();
+              expect(error!.message).toEqual('GraphQL error: forced error');
+              setTimeout(() => {
+                forceUpdate(0);
+              });
+              break;
+            case 2:
+              expect(error).toBeDefined();
+              expect(error!.message).toEqual('GraphQL error: forced error');
+              break;
+            default: // Do nothing
+          }
+
+          renderCount += 1;
+          return null;
+        }
+
+        render(
+          <MockedProvider mocks={mocks}>
+            <App />
+          </MockedProvider>
+        );
+
+        await wait(() => {
+          expect(renderCount).toBe(3);
+        });
+      }
+    );
   });
 
   describe('Pagination', () => {

packages/hooks/src/utils/useBaseQuery.ts

@@ -29,8 +29,11 @@ export function useBaseQuery<TData = any, TVariables = OperationVariables>(
   queryData.setOptions(updatedOptions);
   queryData.context = context;
 
+  // `onError` and `onCompleted` callback functions will not always have a
+  // stable identity, so we'll exclude them from the memoization key to
+  // prevent `afterExecute` from being triggered un-necessarily.
   const memo = {
-    options: updatedOptions,
+    options: { ...updatedOptions, onError: undefined, onCompleted: undefined },
     context,
     tick
   };