Merge pull request #11431 from apollographql/pr/fix-resubscribe

change re-subscription order

src/core/ObservableQuery.ts

@@ -1029,6 +1029,9 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`,
     this.subscriptions.clear();
     this.queryManager.stopQuery(this.queryId);
     this.observers.clear();
+    // this would actually be a good idea to prevent accidental errors,
+    // but would also be bound to make this class a lot more complicated
+    // this.queryInfo = undefined;
     this.isTornDown = true;
   }
 

src/core/QueryInfo.ts

@@ -320,6 +320,8 @@ export class QueryInfo {
   private lastWatch?: Cache.WatchOptions;
 
   private updateWatch(variables = this.variables) {
+    // this is not *required* to make the tests pass, but would be a good idea?
+    // this.stopped = false;
     const oq = this.observableQuery;
     if (oq && oq.options.fetchPolicy === "no-cache") {
       return;

src/react/cache/QueryReference.ts

@@ -181,10 +181,26 @@ export class InternalQueryReference<TData = unknown> {
 
     const originalFetchPolicy = this.watchQueryOptions.fetchPolicy;
 
+    const observer = {
+      next: this.handleNext,
+      error: this.handleError,
+    };
+
     if (originalFetchPolicy !== "no-cache") {
       observable.resetLastResults();
-      observable.forceDiff();
       observable.silentSetOptions({ fetchPolicy: "cache-first" });
+    } else {
+      observable.silentSetOptions({ fetchPolicy: "standby" });
+    }
+
+    this.subscription = observable
+      .filter(
+        (result) => !equal(result.data, {}) && !equal(result, this.result)
+      )
+      .subscribe(observer);
+
+    if (originalFetchPolicy !== "no-cache") {
+      observable.forceDiff();
 
       const result = this.observable.getCurrentResult();
 
@@ -206,20 +222,6 @@ export class InternalQueryReference<TData = unknown> {
       }
     }
 
-    const observer = {
-      next: this.handleNext,
-      error: this.handleError,
-    };
-
-    // Avoid triggering reobserve when resubscribing
-    observable["observers"].add(observer);
-
-    this.subscription = observable
-      .filter(
-        (result) => !equal(result.data, {}) && !equal(result, this.result)
-      )
-      .subscribe(observer);
-
     observable.silentSetOptions({ fetchPolicy: originalFetchPolicy });
   }
 

src/react/query-preloader/__tests__/createQueryPreloader.test.tsx

@@ -243,11 +243,12 @@ test("useReadQuery auto-resubscribes the query after its disposed", async () =>
 
   let count = 0;
   const link = new ApolloLink((operation) => {
+    let returnedCount = ++count;
     return new Observable((observer) => {
       setTimeout(() => {
-        observer.next({ data: { greeting: `Hello ${++count}` } });
+        observer.next({ data: { greeting: `Hello ${returnedCount}` } });
         observer.complete();
-      }, 10);
+      }, 100);
     });
   });
 
@@ -384,9 +385,17 @@ test("useReadQuery auto-resubscribes the query after its disposed", async () =>
     },
   });
 
+  // we wait a moment to ensure no network request is triggered
+  // by the `cache.modify` (even with a slight delay)
+  await new Promise((resolve) => setTimeout(resolve, 10));
+  expect(count).toBe(1);
+
   // mount ReadQueryHook
   await act(() => user.click(toggleButton));
 
+  // this should now trigger a network request
+  expect(count).toBe(2);
+
   expect(queryRef).not.toBeDisposed();
 
   {