Skip to content

Commit

Permalink
Fixes race condition when changing back to initial variables before r…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
jerelmiller authored Sep 1, 2023
1 parent 51045c3 commit f1d429f
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 6 deletions.
5 changes: 5 additions & 0 deletions .changeset/odd-beers-perform.md
Original file line number Diff line number Diff line change
@@ -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.
13 changes: 8 additions & 5 deletions src/core/QueryInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/core/QueryManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1516,7 +1516,7 @@ export class QueryManager<TStore> {
networkStatus,
});

const readCache = () => queryInfo.getDiff(variables);
const readCache = () => queryInfo.getDiff();

const resultsFromCache = (
diff: Cache.DiffResult<TData>,
Expand Down
62 changes: 62 additions & 0 deletions src/core/__tests__/ObservableQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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,
});
});

0 comments on commit f1d429f

Please sign in to comment.