Merge pull request #9636 from apollographql/issue-9632-remove-fetchBl…

…ockingPromise Remove `WatchQueryOptions["fetchBlockingPromise"]` option due to regressions
apollographql · Apr 28, 2022 · c7f268d · c7f268d
2 parents 7530f1b + b238937
commit c7f268d
Show file tree

Hide file tree

Showing 7 changed files with 24 additions and 282 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,10 @@
+## Apollo Client 3.6.1 (unreleased)
+
+### Bug Fixes
+
+- Remove recently-added, internal `fetchBlockingPromise` option from the `WatchQueryOptions` interface, due to regressions. <br/>
+  [@benjamn](https://github.com/benjamn) in [#9504](https://github.com/apollographql/apollo-client/pull/9504)
+
 ## Apollo Client 3.6.0 (2022-04-26)
 
 ### Potentially disruptive changes

diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts
@@ -1333,7 +1333,6 @@ export class QueryManager<TStore> {
       returnPartialData,
       context,
       notifyOnNetworkStatusChange,
-      fetchBlockingPromise,
     }: WatchQueryOptions<TVars, TData>,
     // The initial networkStatus for this fetch, most often
     // NetworkStatus.loading, but also possibly fetchMore, poll, refetch,
@@ -1391,41 +1390,16 @@ export class QueryManager<TStore> {
       ) ? CacheWriteBehavior.OVERWRITE
         : CacheWriteBehavior.MERGE;
 
-    const resultsFromLink = () => {
-      const get = () => this.getResultsFromLink<TData, TVars>(
-        queryInfo,
-        cacheWriteBehavior,
-        {
-          variables,
-          context,
-          fetchPolicy,
-          errorPolicy,
-        },
-      );
-
-      // If we have a fetchBlockingPromise, wait for it to be resolved before
-      // allowing any network requests, and only proceed if fetchBlockingPromise
-      // resolves to true. If it resolves to false, the request is discarded.
-      return fetchBlockingPromise ? fetchBlockingPromise.then(
-        ok => ok ? get() : Observable.of<ApolloQueryResult<TData>>(),
-        error => {
-          const apolloError = isApolloError(error)
-            ? error
-            : new ApolloError({ clientErrors: [error] });
-
-          if (errorPolicy !== "ignore") {
-            queryInfo.markError(apolloError);
-          }
-
-          return Observable.of<ApolloQueryResult<TData>>({
-            loading: false,
-            networkStatus: NetworkStatus.error,
-            error: apolloError,
-            data: readCache().result,
-          });
-        },
-      ) : get();
-    }
+    const resultsFromLink = () => this.getResultsFromLink<TData, TVars>(
+      queryInfo,
+      cacheWriteBehavior,
+      {
+        variables,
+        context,
+        fetchPolicy,
+        errorPolicy,
+      },
+    );
 
     const shouldNotify =
       notifyOnNetworkStatusChange &&

diff --git a/src/core/__tests__/ObservableQuery.ts b/src/core/__tests__/ObservableQuery.ts
@@ -2421,188 +2421,4 @@ describe('ObservableQuery', () => {
       error: reject,
     });
   });
-
-  describe("options.fetchBlockingPromise", () => {
-    itAsync("should not block future refetches", (resolve, reject) => {
-      const query: TypedDocumentNode<{
-        linkCount: number;
-      }> = gql`query Counter { linkCount }`;
-
-      let linkCount = 0;
-      const client = new ApolloClient({
-        cache: new InMemoryCache(),
-        link: new ApolloLink(request => new Observable(observer => {
-          if (request.operationName === "Counter") {
-            observer.next({
-              data: {
-                linkCount: ++linkCount,
-              },
-            });
-            observer.complete();
-          }
-        })),
-      });
-
-      let resolved = false;
-      const observable = client.watchQuery({
-        query,
-        notifyOnNetworkStatusChange: true,
-        fetchBlockingPromise: new Promise(resolve => {
-          setTimeout(() => {
-            resolved = true;
-            resolve(true);
-          }, 100);
-        }),
-      });
-
-      subscribeAndCount(reject, observable, (count, result) => {
-        if (count === 1) {
-          expect(resolved).toBe(true);
-          expect(result.loading).toBe(false);
-          expect(result.data).toEqual({
-            linkCount: 1,
-          });
-
-          client.cache.writeQuery({
-            query,
-            data: {
-              linkCount: 1234,
-            },
-          });
-
-        } else if (count === 2) {
-          expect(resolved).toBe(true);
-          expect(result.loading).toBe(false);
-          expect(result.data).toEqual({
-            linkCount: 1234,
-          });
-
-          // Make sure refetching is not blocked by the fetchBlockingPromise.
-          return observable.refetch();
-
-        } else if (count === 3) {
-          expect(resolved).toBe(true);
-          expect(result.loading).toBe(true);
-          expect(result.networkStatus).toBe(NetworkStatus.refetch);
-          expect(result.data).toEqual({
-            linkCount: 1234,
-          });
-
-        } else if (count === 4) {
-          expect(resolved).toBe(true);
-          expect(result.loading).toBe(false);
-          expect(result.networkStatus).toBe(NetworkStatus.ready);
-          expect(result.data).toEqual({
-            linkCount: 2,
-          });
-
-          const { fetchBlockingPromise } = observable.options;
-          expect(fetchBlockingPromise).toBeInstanceOf(Promise);
-          return fetchBlockingPromise!.then(ok => {
-            expect(ok).toBe(true);
-            setTimeout(resolve, 20);
-          });
-
-        } else {
-          reject(`Too many results (${count}, ${JSON.stringify(result)})`);
-        }
-      });
-    });
-
-    itAsync("should behave reasonably when rejected", (resolve, reject) => {
-      const query: TypedDocumentNode<{
-        linkCount: number;
-      }> = gql`query Counter { linkCount }`;
-
-      let linkCount = 0;
-      const client = new ApolloClient({
-        cache: new InMemoryCache(),
-        link: new ApolloLink(request => new Observable(observer => {
-          if (request.operationName === "Counter") {
-            observer.next({
-              data: {
-                linkCount: ++linkCount,
-              },
-            });
-            observer.complete();
-          }
-        })),
-      });
-
-      let rejected = false;
-      const observable = client.watchQuery({
-        query,
-        notifyOnNetworkStatusChange: true,
-        fetchBlockingPromise: new Promise((_, reject) => {
-          setTimeout(() => {
-            rejected = true;
-            reject(new Error("expected"));
-          }, 100);
-        }),
-      });
-
-      subscribeAndCount(reject, observable, (count, result) => {
-        if (count === 1) {
-          expect(rejected).toBe(true);
-          expect(result.loading).toBe(false);
-          expect(result.error!.message).toBe("expected");
-
-          client.cache.writeQuery({
-            query,
-            data: {
-              linkCount: 1234,
-            },
-          });
-
-        } else if (count === 2) {
-          expect(rejected).toBe(true);
-          expect(result.loading).toBe(false);
-          expect(result.networkStatus).toBe(NetworkStatus.ready);
-          expect(result.error).toBeUndefined();
-          expect(result.data).toEqual({
-            linkCount: 1234,
-          });
-
-          // Make sure refetching is not blocked by the fetchBlockingPromise.
-          return observable.refetch().then(result => {
-            expect(rejected).toBe(true);
-            expect(result.loading).toBe(false);
-            expect(result.networkStatus).toBe(NetworkStatus.error);
-            expect(result.error).toBeInstanceOf(ApolloError);
-            expect(result.error!.message).toBe("expected");
-            expect(result.data).toEqual({
-              linkCount: 1234,
-            });
-          });
-
-        } else if (count === 3) {
-          expect(rejected).toBe(true);
-          expect(result.loading).toBe(true);
-          expect(result.networkStatus).toBe(NetworkStatus.refetch);
-          expect(result.error).toBeUndefined();
-          expect(result.data).toEqual({
-            linkCount: 1234,
-          });
-
-        } else if (count === 4) {
-          expect(rejected).toBe(true);
-          expect(result.loading).toBe(false);
-          expect(result.networkStatus).toBe(NetworkStatus.error);
-          expect(result.error!.message).toBe("expected");
-
-          const { fetchBlockingPromise } = observable.options;
-          expect(fetchBlockingPromise).toBeInstanceOf(Promise);
-          return fetchBlockingPromise!.then(() => {
-            throw new Error("fetchBlockingPromise should be rejected");
-          }).catch(error => {
-            expect(error.message).toBe("expected");
-            setTimeout(resolve, 20);
-          });
-
-        } else {
-          reject(`Too many results (${count}, ${JSON.stringify(result)})`);
-        }
-      });
-    });
-  });
 });
diff --git a/src/core/watchQueryOptions.ts b/src/core/watchQueryOptions.ts
@@ -145,14 +145,6 @@ export interface WatchQueryOptions<TVariables = OperationVariables, TData = any>
    * behavior, for backwards compatibility with Apollo Client 3.x.
    */
   refetchWritePolicy?: RefetchWritePolicy;
-
-  /**
-   * If provided, stalls any network activity for this request until the Promise
-   * has resolved. If the Promise resolves to true, the network request will
-   * proceed. If the Promise resolves to false, the network request will be
-   * silently discarded.
-   */
-  fetchBlockingPromise?: Promise<boolean>;
 }
 
 export interface NextFetchPolicyContext<TData, TVariables> {

diff --git a/src/react/hoc/__tests__/queries/skip.test.tsx b/src/react/hoc/__tests__/queries/skip.test.tsx
@@ -672,7 +672,7 @@ describe('[queries] skip', () => {
                 expect(this.props.skip).toBe(false);
                 expect(this.props.data!.loading).toBe(true);
                 expect(this.props.data.allPeople).toEqual(data.allPeople);
-                expect(ranQuery).toBe(1);
+                expect(ranQuery).toBe(2);
                 break;
               case 5:
                 expect(this.props.skip).toBe(false);

diff --git a/src/react/hooks/__tests__/useQuery.test.tsx b/src/react/hooks/__tests__/useQuery.test.tsx
@@ -1078,8 +1078,8 @@ describe('useQuery Hook', () => {
     expect(result.current.data).toEqual({ hello: 'from link' });
   });
 
-  describe('options.fetchBlockingPromise', () => {
-    it("should prevent duplicate network requests in <React.StrictMode>", async () => {
+  describe('<React.StrictMode>', () => {
+    it("double-rendering should not trigger duplicate network requests", async () => {
       const query: TypedDocumentNode<{
         linkCount: number;
       }> = gql`query Counter { linkCount }`;
@@ -1143,8 +1143,6 @@ describe('useQuery Hook', () => {
           } else {
             expect(activeSet.has(obsQuery)).toBe(false);
             inactiveSet.add(obsQuery);
-            const { fetchBlockingPromise } = obsQuery.options;
-            expect(fetchBlockingPromise).toBeInstanceOf(Promise);
           }
         });
         expect(activeSet.size).toBe(1);