diff --git a/.changeset/odd-beers-perform.md b/.changeset/odd-beers-perform.md new file mode 100644 index 00000000000..84266675835 --- /dev/null +++ b/.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. diff --git a/src/core/QueryInfo.ts b/src/core/QueryInfo.ts index 0323c42cefb..4ddb60fce34 100644 --- a/src/core/QueryInfo.ts +++ b/src/core/QueryInfo.ts @@ -157,14 +157,14 @@ export class QueryInfo { this.dirty = false; } - getDiff(variables = this.variables): Cache.DiffResult { - const options = this.getDiffOptions(variables); + getDiff(): Cache.DiffResult { + 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); diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index a2493076e6a..efdd3c05a92 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -1516,7 +1516,7 @@ export class QueryManager { networkStatus, }); - const readCache = () => queryInfo.getDiff(variables); + const readCache = () => queryInfo.getDiff(); const resultsFromCache = ( diff: Cache.DiffResult, diff --git a/src/core/__tests__/ObservableQuery.ts b/src/core/__tests__/ObservableQuery.ts index 969057a6a33..3104be8ea96 100644 --- a/src/core/__tests__/ObservableQuery.ts +++ b/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 = 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({ + 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, + }); +});