Fixes race condition when changing back to initial variables before r…

…equest finishes (#11186)

Prior to [v3.7.11](https://github.com/apollographql/apollo-client/releases/tag/v3.7.11), changing variables would abort a previous request, which meant there was no cache update for the previous variables. This behavior had some edge cases and resulted in some race condition bugs. This was fixed by #10597, but because #10597 no longer aborted requests, the previous request would complete and the cache would be written, but the broadcasted result would be incorrectly associated with the current variables.

.changeset/odd-beers-perform.md

@@ -0,0 +1,5 @@
+---
+"@apollo/client": patch
+---
+
+Fix an issue where race conditions when rapidly switching between variables would sometimes result in the wrong `data` returned from the query. Specifically this occurs when a query is triggered with an initial set of variables (`VariablesA`), then triggers the same query with another set of variables (`VariablesB`) but switches back to the `VariablesA` before the response for `VariablesB` is returned. Previously this would result in the data for `VariablesB` to be displayed while `VariablesA` was active. The data is for `VariablesA` is now properly returned.

src/core/QueryInfo.ts

@@ -157,14 +157,14 @@ export class QueryInfo {
     this.dirty = false;
   }
 
-  getDiff(variables = this.variables): Cache.DiffResult<any> {
-    const options = this.getDiffOptions(variables);
+  getDiff(): Cache.DiffResult<any> {
+    const options = this.getDiffOptions();
 
     if (this.lastDiff && equal(options, this.lastDiff.options)) {
       return this.lastDiff.diff;
     }
 
-    this.updateWatch((this.variables = variables));
+    this.updateWatch(this.variables);
 
     const oq = this.observableQuery;
     if (oq && oq.options.fetchPolicy === "no-cache") {
@@ -460,8 +460,11 @@ export class QueryInfo {
 
           // In case the QueryManager stops this QueryInfo before its
           // results are delivered, it's important to avoid restarting the
-          // cache watch when markResult is called.
-          if (!this.stopped) {
+          // cache watch when markResult is called. We also avoid updating
+          // the watch if we are writing a result that doesn't match the current
+          // variables to avoid race conditions from broadcasting the wrong
+          // result.
+          if (!this.stopped && equal(this.variables, options.variables)) {
             // Any time we're about to update this.diff, we need to make
             // sure we've started watching the cache.
             this.updateWatch(options.variables);

src/core/QueryManager.ts

@@ -1516,7 +1516,7 @@ export class QueryManager<TStore> {
       networkStatus,
     });
 
-    const readCache = () => queryInfo.getDiff(variables);
+    const readCache = () => queryInfo.getDiff();
 
     const resultsFromCache = (
       diff: Cache.DiffResult<TData>,

src/core/__tests__/ObservableQuery.ts

@@ -25,6 +25,7 @@ import {
   MockLink,
   mockSingleLink,
   subscribeAndCount,
+  wait,
 } from "../../testing";
 import mockQueryManager from "../../testing/core/mocking/mockQueryManager";
 import mockWatchQuery from "../../testing/core/mocking/mockWatchQuery";
@@ -3199,3 +3200,64 @@ test("regression test for #10587", async () => {
   );
   expect(query1Spy.mock.calls).toEqual(finalExpectedCalls.query1);
 });
+
+// https://github.com/apollographql/apollo-client/issues/11184
+test("handles changing variables in rapid succession before other request is completed", async () => {
+  interface UserCountQuery {
+    userCount: number;
+  }
+  interface UserCountVariables {
+    department: "HR" | null;
+  }
+
+  const query: TypedDocumentNode<UserCountQuery, UserCountVariables> = gql`
+    query UserCountQuery($department: Department) {
+      userCount(department: $department)
+    }
+  `;
+  const mocks = [
+    {
+      request: { query, variables: { department: null } },
+      result: { data: { userCount: 10 } },
+    },
+    {
+      request: { query, variables: { department: "HR" } },
+      result: { data: { userCount: 5 } },
+      delay: 50,
+    },
+  ];
+
+  const client = new ApolloClient({
+    link: new MockLink(mocks),
+    cache: new InMemoryCache(),
+  });
+
+  const observable = client.watchQuery<UserCountQuery, UserCountVariables>({
+    query,
+    variables: { department: null },
+  });
+
+  observable.subscribe(jest.fn());
+
+  await waitFor(() => {
+    expect(observable.getCurrentResult(false)).toEqual({
+      data: { userCount: 10 },
+      loading: false,
+      networkStatus: NetworkStatus.ready,
+    });
+  });
+
+  observable.reobserve({ variables: { department: "HR" } });
+  await wait(10);
+  observable.reobserve({ variables: { department: null } });
+
+  // Wait for request to finish
+  await wait(50);
+
+  expect(observable.options.variables).toEqual({ department: null });
+  expect(observable.getCurrentResult(false)).toEqual({
+    data: { userCount: 10 },
+    loading: false,
+    networkStatus: NetworkStatus.ready,
+  });
+});