Make ObservableQuery#getCurrentResult always call queryInfo.getDiff()

CHANGELOG.md

@@ -27,10 +27,14 @@
 - `InMemoryCache` now coalesces `EntityStore` updates to guarantee only one `store.merge(id, fields)` call per `id` per cache write. <br/>
   [@benjamn](https://github.com/benjamn) in [#8372](https://github.com/apollographql/apollo-client/pull/8372)
 
-- Fix polling when used with `React.StrictMode`, <br/>
+- Fix polling when used with `<React.StrictMode>`. <br/>
   [@brainkim](https://github.com/brainkim) in [#8414](https://github.com/apollographql/apollo-client/pull/8414)
 
-- Fix the React integration logging `Warning: Can't perform a React state update on an unmounted component`. <br/> [@wuarmin](https://github.com/wuarmin) in [#7745](https://github.com/apollographql/apollo-client/pull/7745)
+- Fix the React integration logging `Warning: Can't perform a React state update on an unmounted component`. <br/>
+  [@wuarmin](https://github.com/wuarmin) in [#7745](https://github.com/apollographql/apollo-client/pull/7745)
+
+- Make `ObservableQuery#getCurrentResult` always call `queryInfo.getDiff()`. <br/>
+  [@benjamn](https://github.com/benjamn) in [#8422](https://github.com/apollographql/apollo-client/pull/8422)
 
 ### Potentially disruptive changes
 

src/core/ObservableQuery.ts

@@ -169,7 +169,12 @@ export class ObservableQuery<
   }
 
   public getCurrentResult(saveAsLastResult = true): ApolloQueryResult<TData> {
-    const { lastResult } = this;
+    const {
+      lastResult,
+      options: {
+        fetchPolicy = "cache-first",
+      },
+    } = this;
 
     const networkStatus =
       this.queryInfo.networkStatus ||
@@ -182,33 +187,18 @@ export class ObservableQuery<
       networkStatus,
     } as ApolloQueryResult<TData>;
 
-    if (this.isTornDown) {
-      return result;
-    }
-
-    const { fetchPolicy = 'cache-first' } = this.options;
-    if (fetchPolicy === 'no-cache' ||
-        fetchPolicy === 'network-only') {
-      // Similar to setting result.partial to false, but taking advantage
-      // of the falsiness of missing fields.
-      delete result.partial;
-    } else if (
-      !result.data ||
-      // If this.options.query has @client(always: true) fields, we cannot
-      // trust result.data, since it was read from the cache without
-      // running local resolvers (and it's too late to run resolvers now,
-      // since we must return a result synchronously). TODO In the future
-      // (after Apollo Client 3.0), we should find a way to trust
-      // this.lastResult in more cases, and read from the cache only in
-      // cases when no result has been received yet.
-      !this.queryManager.transform(this.options.query).hasForcedResolvers
-    ) {
+    // If this.options.query has @client(always: true) fields, we cannot trust
+    // diff.result, since it was read from the cache without running local
+    // resolvers (and it's too late to run resolvers now, since we must return a
+    // result synchronously).
+    if (!this.queryManager.transform(this.options.query).hasForcedResolvers) {
       const diff = this.queryInfo.getDiff();
-      // XXX the only reason this typechecks is that diff.result is inferred as any
+
       result.data = (
         diff.complete ||
         this.options.returnPartialData
       ) ? diff.result : void 0;
+
       if (diff.complete) {
         // If the diff is complete, and we're using a FetchPolicy that
         // terminates after a complete cache read, we can assume the next
@@ -220,7 +210,10 @@ export class ObservableQuery<
           result.loading = false;
         }
         delete result.partial;
-      } else {
+      } else if (fetchPolicy !== "no-cache") {
+        // Since result.partial comes from diff.complete, and we shouldn't be
+        // using cache data to provide a DiffResult when the fetchPolicy is
+        // "no-cache", avoid annotating result.partial for "no-cache" results.
         result.partial = true;
       }
 

src/core/__tests__/ObservableQuery.ts

@@ -1648,15 +1648,16 @@ describe('ObservableQuery', () => {
         });
 
         expect(observable.getCurrentResult()).toEqual({
-          data: void 0,
+          data: dataOne,
           loading: true,
-          networkStatus: 1,
+          networkStatus: NetworkStatus.loading,
         });
 
         subscribeAndCount(reject, observable, (handleCount, subResult) => {
           if (handleCount === 1) {
             expect(subResult).toEqual({
               loading: true,
+              data: dataOne,
               networkStatus: NetworkStatus.loading,
             });
           } else if (handleCount === 2) {

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

@@ -3107,6 +3107,232 @@ describe('useQuery Hook', () => {
         expect(renderCount).toBe(5);
       }).then(resolve, reject);
     });
+
+    itAsync("should be cleared when variables change causes cache miss", (resolve, reject) => {
+      const peopleData = [
+        { id: 1, name: 'John Smith', gender: 'male' },
+        { id: 2, name: 'Sara Smith', gender: 'female' },
+        { id: 3, name: 'Budd Deey', gender: 'nonbinary' },
+        { id: 4, name: 'Johnny Appleseed', gender: 'male' },
+        { id: 5, name: 'Ada Lovelace', gender: 'female' },
+      ];
+
+      const link = new ApolloLink(operation => {
+        return new Observable(observer => {
+          const { gender } = operation.variables;
+          new Promise(resolve => setTimeout(resolve, 300)).then(() => {
+            observer.next({
+              data: {
+                people: gender === "all" ? peopleData :
+                  gender ? peopleData.filter(
+                    person => person.gender === gender
+                  ) : peopleData,
+              }
+            });
+            observer.complete();
+          });
+        });
+      });
+
+      type Person = {
+        __typename: string;
+        id: string;
+        name: string;
+      };
+
+      const ALL_PEOPLE: TypedDocumentNode<{
+        people: Person[];
+      }> = gql`
+        query AllPeople($gender: String!) {
+          people(gender: $gender) {
+            id
+            name
+          }
+        }
+      `;
+
+      let renderCount = 0;
+      function App() {
+        const [gender, setGender] = useState("all");
+        const {
+          loading,
+          networkStatus,
+          data,
+        } = useQuery(ALL_PEOPLE, {
+          variables: { gender },
+          fetchPolicy: "network-only",
+        });
+
+        const currentPeopleNames = data?.people?.map(person => person.name);
+
+        switch (++renderCount) {
+          case 1:
+            expect(gender).toBe("all");
+            expect(loading).toBe(true);
+            expect(networkStatus).toBe(NetworkStatus.loading);
+            expect(data).toBeUndefined();
+            expect(currentPeopleNames).toBeUndefined();
+            break;
+
+          case 2:
+            expect(gender).toBe("all");
+            expect(loading).toBe(false);
+            expect(networkStatus).toBe(NetworkStatus.ready);
+            expect(data).toEqual({
+              people: peopleData.map(({ gender, ...person }) => person),
+            });
+            expect(currentPeopleNames).toEqual([
+              "John Smith",
+              "Sara Smith",
+              "Budd Deey",
+              "Johnny Appleseed",
+              "Ada Lovelace",
+            ]);
+            act(() => {
+              setGender("female");
+            });
+            break;
+
+          case 3:
+            expect(gender).toBe("female");
+            expect(loading).toBe(true);
+            expect(networkStatus).toBe(NetworkStatus.setVariables);
+            expect(data).toBeUndefined();
+            expect(currentPeopleNames).toBeUndefined();
+            break;
+
+          case 4:
+            expect(gender).toBe("female");
+            expect(loading).toBe(false);
+            expect(networkStatus).toBe(NetworkStatus.ready);
+            expect(data!.people.length).toBe(2);
+            expect(currentPeopleNames).toEqual([
+              "Sara Smith",
+              "Ada Lovelace",
+            ]);
+            act(() => {
+              setGender("nonbinary");
+            });
+            break;
+
+          case 5:
+            expect(gender).toBe("nonbinary");
+            expect(loading).toBe(true);
+            expect(networkStatus).toBe(NetworkStatus.setVariables);
+            expect(data).toBeUndefined();
+            expect(currentPeopleNames).toBeUndefined();
+            break;
+
+          case 6:
+            expect(gender).toBe("nonbinary");
+            expect(loading).toBe(false);
+            expect(networkStatus).toBe(NetworkStatus.ready);
+            expect(data!.people.length).toBe(1);
+            expect(currentPeopleNames).toEqual([
+              "Budd Deey",
+            ]);
+            act(() => {
+              setGender("male");
+            });
+            break;
+
+          case 7:
+            expect(gender).toBe("male");
+            expect(loading).toBe(true);
+            expect(networkStatus).toBe(NetworkStatus.setVariables);
+            expect(data).toBeUndefined();
+            expect(currentPeopleNames).toBeUndefined();
+            break;
+
+          case 8:
+            expect(gender).toBe("male");
+            expect(loading).toBe(false);
+            expect(networkStatus).toBe(NetworkStatus.ready);
+            expect(data!.people.length).toBe(2);
+            expect(currentPeopleNames).toEqual([
+              "John Smith",
+              "Johnny Appleseed",
+            ]);
+            act(() => {
+              setGender("female");
+            });
+            break;
+
+          case 9:
+            expect(gender).toBe("female");
+            expect(loading).toBe(true);
+            expect(networkStatus).toBe(NetworkStatus.setVariables);
+            expect(data!.people.length).toBe(2);
+            expect(currentPeopleNames).toEqual([
+              "Sara Smith",
+              "Ada Lovelace",
+            ]);
+            break;
+
+          case 10:
+            expect(gender).toBe("female");
+            expect(loading).toBe(false);
+            expect(networkStatus).toBe(NetworkStatus.ready);
+            expect(data!.people.length).toBe(2);
+            expect(currentPeopleNames).toEqual([
+              "Sara Smith",
+              "Ada Lovelace",
+            ]);
+            act(() => {
+              setGender("all");
+            });
+            break;
+
+          case 11:
+            expect(gender).toBe("all");
+            expect(loading).toBe(true);
+            expect(networkStatus).toBe(NetworkStatus.setVariables);
+            expect(data!.people.length).toBe(5);
+            expect(currentPeopleNames).toEqual([
+              "John Smith",
+              "Sara Smith",
+              "Budd Deey",
+              "Johnny Appleseed",
+              "Ada Lovelace",
+            ]);
+            break;
+
+          case 12:
+            expect(gender).toBe("all");
+            expect(loading).toBe(false);
+            expect(networkStatus).toBe(NetworkStatus.ready);
+            expect(data!.people.length).toBe(5);
+            expect(currentPeopleNames).toEqual([
+              "John Smith",
+              "Sara Smith",
+              "Budd Deey",
+              "Johnny Appleseed",
+              "Ada Lovelace",
+            ]);
+            break;
+
+          default:
+            reject(`too many (${renderCount}) renders`);
+        }
+
+        return null;
+      }
+
+      const client = new ApolloClient({
+        cache: new InMemoryCache(),
+        link
+      });
+
+      render(
+        <ApolloProvider client={client}>
+          <App />
+        </ApolloProvider>,
+      );
+
+      return wait(() => {
+        expect(renderCount).toBe(12);
+      }).then(resolve, reject);
+    });
   });
 
   describe("canonical cache results", () => {