Fix an issue where incorrect results are returned when changing variables and skipping (#12461)

Author: jerelmiller

.changeset/little-trainers-compare.md

@@ -0,0 +1,5 @@
+---
+"@apollo/client": patch
+---
+
+Fix an issue where a `cache-first` query would return the result for previous variables when a cache update is issued after simultaneously changing variables and skipping the query.

.size-limits.json

@@ -1,4 +1,4 @@
 {
-  "dist/apollo-client.min.cjs": 42244,
-  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production)": 34450
+  "dist/apollo-client.min.cjs": 42263,
+  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production)": 34469
 }

src/core/QueryInfo.ts

@@ -86,6 +86,7 @@ export class QueryInfo {
   graphQLErrors?: ReadonlyArray<GraphQLFormattedError>;
   stopped = false;
 
+  private cancelWatch?: () => void;
   private cache: ApolloCache<any>;
 
   constructor(
@@ -128,6 +129,8 @@ export class QueryInfo {
 
     if (!equal(query.variables, this.variables)) {
       this.lastDiff = void 0;
+      // Ensure we don't continue to receive cache updates for old variables
+      this.cancel();
     }
 
     Object.assign(this, {
@@ -308,20 +311,17 @@ export class QueryInfo {
 
       // Cancel the pending notify timeout
       this.reset();
-
       this.cancel();
-      // Revert back to the no-op version of cancel inherited from
-      // QueryInfo.prototype.
-      this.cancel = QueryInfo.prototype.cancel;
 
       const oq = this.observableQuery;
       if (oq) oq.stopPolling();
     }
   }
 
-  // This method is a no-op by default, until/unless overridden by the
-  // updateWatch method.
-  private cancel() {}
+  private cancel() {
+    this.cancelWatch?.();
+    this.cancelWatch = void 0;
+  }
 
   private lastWatch?: Cache.WatchOptions;
 
@@ -342,7 +342,7 @@ export class QueryInfo {
 
     if (!this.lastWatch || !equal(watchOptions, this.lastWatch)) {
       this.cancel();
-      this.cancel = this.cache.watch((this.lastWatch = watchOptions));
+      this.cancelWatch = this.cache.watch((this.lastWatch = watchOptions));
     }
   }
 

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

@@ -1620,6 +1620,99 @@ describe("useQuery Hook", () => {
 
       await expect(takeSnapshot).not.toRerender();
     });
+
+    // https://github.com/apollographql/apollo-client/issues/12458
+    it("returns correct result when cache updates after changing variables and skipping query", async () => {
+      interface Data {
+        user: {
+          __typename: "User";
+          id: number;
+          name: string;
+        };
+      }
+
+      const query: TypedDocumentNode<Data, { id: number }> = gql`
+        query ($id: ID!) {
+          user(id: $id) {
+            id
+            name
+          }
+        }
+      `;
+
+      const client = new ApolloClient({
+        link: ApolloLink.empty(),
+        cache: new InMemoryCache(),
+      });
+
+      using _disabledAct = disableActEnvironment();
+      const { takeSnapshot, rerender } = await renderHookToSnapshotStream(
+        ({ id, skip }) => useQuery(query, { skip, variables: { id } }),
+        {
+          initialProps: { id: 1, skip: true },
+          wrapper: ({ children }) => (
+            <ApolloProvider client={client}>{children}</ApolloProvider>
+          ),
+        }
+      );
+
+      await expect(takeSnapshot()).resolves.toEqualQueryResult({
+        data: undefined,
+        error: undefined,
+        called: false,
+        loading: false,
+        networkStatus: NetworkStatus.ready,
+        previousData: undefined,
+        variables: { id: 1 },
+      });
+
+      client.writeQuery({
+        query,
+        variables: { id: 1 },
+        data: { user: { __typename: "User", id: 1, name: "User 1" } },
+      });
+      await rerender({ id: 1, skip: false });
+
+      await expect(takeSnapshot()).resolves.toEqualQueryResult({
+        data: { user: { __typename: "User", id: 1, name: "User 1" } },
+        called: true,
+        loading: false,
+        networkStatus: NetworkStatus.ready,
+        previousData: undefined,
+        variables: { id: 1 },
+      });
+
+      await rerender({ id: 2, skip: true });
+
+      await expect(takeSnapshot()).resolves.toEqualQueryResult({
+        data: undefined,
+        error: undefined,
+        called: false,
+        loading: false,
+        networkStatus: NetworkStatus.ready,
+        previousData: { user: { __typename: "User", id: 1, name: "User 1" } },
+        variables: { id: 2 },
+      });
+
+      client.writeQuery({
+        query,
+        variables: { id: 2 },
+        data: { user: { __typename: "User", id: 2, name: "User 2" } },
+      });
+
+      await rerender({ id: 2, skip: false });
+
+      await expect(takeSnapshot()).resolves.toEqualQueryResult({
+        data: { user: { __typename: "User", id: 2, name: "User 2" } },
+        called: true,
+        loading: false,
+        networkStatus: NetworkStatus.ready,
+        previousData: { user: { __typename: "User", id: 1, name: "User 1" } },
+        variables: { id: 2 },
+      });
+
+      await expect(takeSnapshot).not.toRerender();
+    });
   });
 
   describe("options.defaultOptions", () => {