Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix regression that causes partial data to be reported unexpectedly in some circumstances #11579

Merged
merged 33 commits into from
Feb 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
3b90234
Write failing test for issue 11400
jerelmiller Feb 8, 2024
3019c02
Remove properties from test that would pass with fix that arent there
jerelmiller Feb 8, 2024
3432591
Set data to undefined if its partial and we don't allow partial data
jerelmiller Feb 9, 2024
3c3e8fc
Add comment about partial data
jerelmiller Feb 9, 2024
5b1853d
Update test to check for react version
jerelmiller Feb 9, 2024
5d70cd8
Add test to ensure getCurrentResult doesn't have data loss
jerelmiller Feb 12, 2024
1ca66c6
Remove first proposed fix from ObservableQuery
jerelmiller Feb 12, 2024
1d7629d
Remove part of test that performs refetch
jerelmiller Feb 12, 2024
42fea55
Skip reporting result when cache diff is partial and returnPartailDat…
jerelmiller Feb 12, 2024
22355a6
Add changeset
jerelmiller Feb 12, 2024
ee92d43
Merge branch 'main' into jerel/issue-11400-unexpected-partial-data
jerelmiller Feb 12, 2024
7ffd17e
Minor tweak to comment
jerelmiller Feb 12, 2024
e7fb857
Update size limits
jerelmiller Feb 12, 2024
364ec36
Move check above updateLastDiff
jerelmiller Feb 12, 2024
0e78965
Schedule notification if cache eviction happened
jerelmiller Feb 13, 2024
5cee1db
Remove redundant check
jerelmiller Feb 13, 2024
3c24aa8
Add additional context to comment
jerelmiller Feb 13, 2024
2a624b2
Simplify conditional in test for React 17
jerelmiller Feb 13, 2024
d78f1ff
Add check against getCurrentResult to ensure partial data is not set
jerelmiller Feb 13, 2024
b497ebd
Remove unnecessary setTimeouts wrapped around simulateResult
jerelmiller Feb 13, 2024
ff3c8b1
Merge remote-tracking branch 'origin/main' into jerel/issue-11400-une…
jerelmiller Feb 13, 2024
fcf33f2
Update size limits
jerelmiller Feb 13, 2024
d4f5bf8
Add additional checks in test to show failed situation
jerelmiller Feb 13, 2024
c2afd2a
Account for extra React 17 render
jerelmiller Feb 13, 2024
1e8184b
Conditionally set last diff when we notify
jerelmiller Feb 13, 2024
359a43f
Update size limit
jerelmiller Feb 13, 2024
c9b9317
Only schedule notification if not dirty
jerelmiller Feb 14, 2024
73de9f5
Simplify inner conditional to avoid duplicate logic
jerelmiller Feb 14, 2024
e5033b5
Restore old if condition
jerelmiller Feb 14, 2024
449b6a2
Add delays to remove React 17 conditional
jerelmiller Feb 14, 2024
aa1dbd8
Update size limits
jerelmiller Feb 14, 2024
2896004
Add additional changeset
jerelmiller Feb 14, 2024
1e408bd
Merge branch 'main' into jerel/issue-11400-unexpected-partial-data
jerelmiller Feb 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/neat-toes-leave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@apollo/client": patch
---

Fix issue where partial data is reported to `useQuery` when using `notifyOnNetworkStatusChange` after it errors while another overlapping query succeeds.
5 changes: 5 additions & 0 deletions .changeset/seven-shoes-invite.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@apollo/client": patch
---

Fix an issue where a partial cache write for an errored query would result in automatically refetching that query.
4 changes: 2 additions & 2 deletions .size-limits.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{
"dist/apollo-client.min.cjs": 39042,
"import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 32550
"dist/apollo-client.min.cjs": 39075,
"import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 32584
}
21 changes: 21 additions & 0 deletions src/core/QueryInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,28 @@ export class QueryInfo {

setDiff(diff: Cache.DiffResult<any> | null) {
const oldDiff = this.lastDiff && this.lastDiff.diff;

// If we do not tolerate partial results, skip this update to prevent it
// from being reported. This prevents a situtuation where a query that
// errors and another succeeds with overlapping data does not report the
// partial data result to the errored query.
//
// See https://github.com/apollographql/apollo-client/issues/11400 for more
// information on this issue.
if (
diff &&
!diff.complete &&
!this.observableQuery?.options.returnPartialData &&
// In the case of a cache eviction, the diff will become partial so we
// schedule a notification to send a network request (this.oqListener) to
// go and fetch the missing data.
!(oldDiff && oldDiff.complete)
) {
return;
}

this.updateLastDiff(diff);

if (!this.dirty && !equal(oldDiff && oldDiff.result, diff && diff.result)) {
this.dirty = true;
if (!this.notifyTimeout) {
Expand Down
119 changes: 119 additions & 0 deletions src/core/__tests__/ObservableQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
itAsync,
MockLink,
mockSingleLink,
MockSubscriptionLink,
subscribeAndCount,
wait,
} from "../../testing";
Expand Down Expand Up @@ -2389,6 +2390,124 @@ describe("ObservableQuery", () => {
}
);

it("handles multiple calls to getCurrentResult without losing data", async () => {
const query = gql`
{
greeting {
message
... on Greeting @defer {
recipient {
name
}
}
}
}
`;

const link = new MockSubscriptionLink();

const client = new ApolloClient({
link,
cache: new InMemoryCache(),
});

const obs = client.watchQuery({ query });
const stream = new ObservableStream(obs);

link.simulateResult({
result: {
data: {
greeting: {
message: "Hello world",
__typename: "Greeting",
},
},
hasNext: true,
},
});

{
const result = await stream.takeNext();
expect(result.data).toEqual({
greeting: {
message: "Hello world",
__typename: "Greeting",
},
});
}

expect(obs.getCurrentResult().data).toEqual({
greeting: {
message: "Hello world",
__typename: "Greeting",
},
});

expect(obs.getCurrentResult().data).toEqual({
greeting: {
message: "Hello world",
__typename: "Greeting",
},
});

link.simulateResult(
{
result: {
incremental: [
{
data: {
recipient: {
name: "Alice",
__typename: "Person",
},
__typename: "Greeting",
},
path: ["greeting"],
},
],
hasNext: false,
},
},
true
);

{
const result = await stream.takeNext();
expect(result.data).toEqual({
greeting: {
message: "Hello world",
recipient: {
name: "Alice",
__typename: "Person",
},
__typename: "Greeting",
},
});
}

expect(obs.getCurrentResult().data).toEqual({
greeting: {
message: "Hello world",
recipient: {
name: "Alice",
__typename: "Person",
},
__typename: "Greeting",
},
});

expect(obs.getCurrentResult().data).toEqual({
greeting: {
message: "Hello world",
recipient: {
name: "Alice",
__typename: "Person",
},
__typename: "Greeting",
},
});
});

{
type Result = Partial<ApolloQueryResult<{ hello: string }>>;

Expand Down