From 6314d27df57b68deca54cd1c59cf90e0e71a0471 Mon Sep 17 00:00:00 2001
From: Brian Kim <briankimpossible@gmail.com>
Date: Fri, 23 Jul 2021 19:09:34 -0400
Subject: [PATCH] Stop delaying useQuery calls by a microtask

After https://github.com/apollographql/apollo-client/pull/8414, the
changes made in https://github.com/apollographql/apollo-client/pull/6107
are unnneccessary, because all ObservableQuery callbacks will only be
fired in useEffect calls (hopefully). This changes the timings of some
of tests.
---
 src/react/hooks/__tests__/useQuery.test.tsx | 33 +++++++--------------
 src/react/hooks/useQuery.ts                 | 12 ++------
 2 files changed, 14 insertions(+), 31 deletions(-)

diff --git a/src/react/hooks/__tests__/useQuery.test.tsx b/src/react/hooks/__tests__/useQuery.test.tsx
index eeb4fc11777..9ce26df42a5 100644
--- a/src/react/hooks/__tests__/useQuery.test.tsx
+++ b/src/react/hooks/__tests__/useQuery.test.tsx
@@ -314,7 +314,6 @@ describe('useQuery Hook', () => {
           variables: { something }
         },
         result: { data: CAR_RESULT_DATA },
-        delay: 1000
       }));
 
       let renderCount = 0;
@@ -1311,7 +1310,7 @@ describe('useQuery Hook', () => {
       expect(result.current.loading).toBe(false);
       expect(result.current.networkStatus).toBe(NetworkStatus.ready);
       expect(result.current.data).toEqual({ letters: ab });
-      result.current.fetchMore({
+      act(() => void result.current.fetchMore({
         variables: { limit: 2 },
         updateQuery: (prev, { fetchMoreResult }) => ({
           letters: prev.letters.concat(fetchMoreResult.letters),
@@ -1352,14 +1351,13 @@ describe('useQuery Hook', () => {
       expect(result.current.networkStatus).toBe(NetworkStatus.ready);
       expect(result.current.data).toEqual({ letters: ab });
 
-      result.current.fetchMore({
+      act(() => void result.current.fetchMore({
         variables: { limit: 2 },
         updateQuery: (prev, { fetchMoreResult }) => ({
           letters: prev.letters.concat(fetchMoreResult.letters),
         }),
-      });
+      }));
 
-      await waitForNextUpdate();
       expect(result.current.loading).toBe(true);
       expect(result.current.networkStatus).toBe(NetworkStatus.fetchMore);
       expect(result.current.data).toEqual({ letters: ab });
@@ -1440,9 +1438,7 @@ describe('useQuery Hook', () => {
       expect(result.current.networkStatus).toBe(NetworkStatus.ready);
       expect(result.current.data).toEqual({ letters: ab });
 
-      result.current.fetchMore({ variables: { limit: 2 } });
-
-      await waitForNextUpdate();
+      act(() => void result.current.fetchMore({ variables: { limit: 2 } }));
       expect(result.current.loading).toBe(true);
       expect(result.current.networkStatus).toBe(NetworkStatus.fetchMore);
       expect(result.current.data).toEqual({ letters: ab });
@@ -2146,7 +2142,7 @@ describe('useQuery Hook', () => {
         {
           request: { query: mutation },
           error: new Error('Oh no!'),
-          delay: 10,
+          delay: 500,
         }
       ];
 
@@ -2202,13 +2198,15 @@ describe('useQuery Hook', () => {
 
       act(() => void mutate());
       // The mutation ran and is loading the result. The query stays at not
-      // loading as nothing has changed for the query.
+      // loading as nothing has changed for the query, but optimistic data is
+      // rendered.
+
       expect(result.current.mutation[1].loading).toBe(true);
       expect(result.current.query.loading).toBe(false);
-
+      expect(result.current.query.data).toEqual(allCarsData);
       await waitForNextUpdate();
-      // There is a missing update here because mutation and query update in
-      // the same microtask loop.
+      // TODO: There is a missing update here because mutation and query update
+      // in the same microtask loop.
       const previous = result.all[result.all.length - 2];
       if (previous instanceof Error) {
         throw previous;
@@ -2219,15 +2217,6 @@ describe('useQuery Hook', () => {
       expect(previous.mutation[1].loading).toBe(true);
       expect(previous.query.loading).toBe(false);
 
-      // The first part of the mutation has completed using the defined
-      // optimisticResponse data. This means that while the mutation stays in a
-      // loading state, it has made its optimistic data available to the query.
-      // New optimistic data doesn't trigger a query loading state.
-      expect(result.current.mutation[1].loading).toBe(true);
-      expect(result.current.query.loading).toBe(false);
-      expect(result.current.query.data).toEqual(allCarsData);
-
-      await waitForNextUpdate();
       // The mutation has completely finished, leaving the query with access to
       // the original cache data.
       expect(result.current.mutation[1].loading).toBe(false);
diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts
index 72e0899b947..241379465a9 100644
--- a/src/react/hooks/useQuery.ts
+++ b/src/react/hooks/useQuery.ts
@@ -587,18 +587,12 @@ export function useQuery<TData = any, TVariables = OperationVariables>(
       options: updatedOptions,
       context,
       onNewData() {
-        if (!queryData.ssrInitiated()) {
-          // When new data is received from the `QueryData` object, we want to
-          // force a re-render to make sure the new data is displayed. We can't
-          // force that re-render if we're already rendering however so to be
-          // safe we'll trigger the re-render in a microtask. In case the
-          // component gets unmounted before this callback fires, we re-check
-          // queryDataRef.current.isMounted before calling forceUpdate().
-          Promise.resolve().then(() => queryDataRef.current && queryDataRef.current.isMounted && forceUpdate());
-        } else {
+        if (queryData.ssrInitiated()) {
           // If we're rendering on the server side we can force an update at
           // any point.
           forceUpdate();
+        } else if (queryDataRef.current && queryDataRef.current.isMounted) {
+          forceUpdate();
         }
       }
     })