Skip to content

Commit 0abd8de

Browse files
authored
Fix partial/complete data values for incremental responses, especially when combined with returnPartialData (#13324)
Apollo Client 4.0-4.2.x is fairly naive in the way `dataState` is reported for incremental responses. Those versions set `dataState` as `streaming` if the request is still in-flight, regardless of whether the data actually satisfies the definition of `streaming`. `streaming` is meant to represent a state where the only holes in `data` are at `@defer` boundaries that aren't streamed in. Prior to this change, this could lead to runtime crashes due to the inaccuracy (see the changeset for an example). The other issue is that field `read` functions that modified values on individual fields were not applied correctly in intermediate responses returned by the cache. The `read` functions were run correctly, but due to the completeness checks in `QueryInfo`, the values were never applied. `data` returned in incremental chunks are now properly reported as `partial`, or in some cases `complete` depending on whether it fulfills the requirements of the query and/or `@defer` boundaries. This change also fixes the issue where field `read` function return values were not applied to intermediate chunks. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved `dataState` reporting for incremental `@defer` and `@stream` responses. * Results now correctly distinguish between `partial`, `streaming`, and `complete` based on available cached data. * Network status continues to indicate when incremental delivery is still in progress. * Improved handling of partial cached results, refetches, errors, and cache merges. * **New Features** * Added internal utilities for analyzing GraphQL fields and deferred fragments. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: jerelmiller <565661+jerelmiller@users.noreply.github.com>
1 parent a9e4462 commit 0abd8de

36 files changed

Lines changed: 7826 additions & 2029 deletions

.api-reports/api-report-utilities_internal.api.md

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { ErrorLike } from '@apollo/client';
1414
import type { FieldNode } from 'graphql';
1515
import type { FormattedExecutionResult } from 'graphql';
1616
import type { FragmentDefinitionNode } from 'graphql';
17+
import type { FragmentSpreadNode } from 'graphql';
1718
import type { GraphQLFormattedError } from 'graphql';
1819
import type { HKT } from '@apollo/client/utilities';
1920
import type { Incremental } from '@apollo/client/incremental';
@@ -85,6 +86,21 @@ export type ClassicSignature = SignatureStyle extends "classic" ? unknown : neve
8586
// @internal @deprecated
8687
export function cloneDeep<T>(value: T): T;
8788

89+
// @public (undocumented)
90+
interface CollectionContext {
91+
// (undocumented)
92+
exclude: SelectionNode;
93+
// Warning: (ae-incompatible-release-tags) The symbol "fragmentMap" is marked as @public, but its signature references "FragmentMap" which is marked as @internal
94+
//
95+
// (undocumented)
96+
fragmentMap: FragmentMap;
97+
}
98+
99+
// Warning: (ae-forgotten-export) The symbol "CollectionContext" needs to be exported by the entry point index.d.ts
100+
//
101+
// @internal @deprecated
102+
export function collectSiblingFields(selectionSet: SelectionSetNode, context: CollectionContext, visitedFragments?: Map<string, FieldMap>): FieldMap;
103+
88104
// @public
89105
export function combineLatestBatched<T>(observables: Array<Observable<T> & {
90106
dirty?: boolean;
@@ -221,6 +237,11 @@ export interface ExtensionsWithStreamInfo extends Record<string, unknown> {
221237
};
222238
}
223239

240+
// @public (undocumented)
241+
export type FieldMap = {
242+
[fieldName: string]: FieldMap | true;
243+
};
244+
224245
// @public (undocumented)
225246
export function filterMap<T, R>(fn: (value: T, context: undefined) => R | undefined): OperatorFunction<T, R>;
226247

@@ -368,6 +389,9 @@ export type IsAny<T> = 0 extends 1 & T ? true : false;
368389
// @internal @deprecated
369390
export const isArray: (a: any) => a is any[] | readonly any[];
370391

392+
// @public (undocumented)
393+
export function isDeferredFragment(fragmentSelection: InlineFragmentNode | FragmentSpreadNode, variables: OperationVariables): boolean;
394+
371395
// @internal @deprecated (undocumented)
372396
export function isDocumentNode(value: unknown): value is DocumentNode;
373397

@@ -390,6 +414,9 @@ export function isNonNullObject(obj: unknown): obj is Record<string | number, an
390414
// @internal @deprecated (undocumented)
391415
export function isPlainObject(obj: unknown): obj is Record<string | number, any>;
392416

417+
// @public (undocumented)
418+
export function isTypenameField(field: FieldNode): boolean;
419+
393420
// @public
394421
export type LazyType<T> = T & {
395422
[K in "" as never]: LazyType<never>;

.changeset/angry-baboons-decide.md

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
---
2+
"@apollo/client": minor
3+
---
4+
5+
Fix the accuracy of `dataState` in complex incremental streaming scenarios, especially when combined with `returnPartialData: true`.
6+
7+
Prior to this change, all intermediate chunks used for both `@defer` and `@stream` directives returned a `dataState` of `streaming`, regardless of whether the actual data shape fit the definition of the `streaming` data state. The `streaming` data state represents an incomplete incremental response where the only holes in the data occur at `@defer` boundaries.
8+
9+
Let's use the following example of where the previous `dataState` fell down when combined with `returnPartialData`.
10+
11+
```gql
12+
query GreetingQuery {
13+
greeting {
14+
message
15+
... @defer {
16+
recipient {
17+
name
18+
email
19+
}
20+
}
21+
}
22+
}
23+
```
24+
25+
1. Scenario 1: partial data inside a `@defer` boundary written to the cache
26+
27+
Let's say the cache contained the following partial data:
28+
29+
```ts
30+
{
31+
greeting: {
32+
__typename: "Greeting",
33+
recipient: {
34+
__typename: "Person",
35+
name: "John Doe",
36+
},
37+
},
38+
};
39+
```
40+
41+
After the first chunk arrives from the server, the data looks like the following:
42+
43+
```ts
44+
{
45+
greeting: {
46+
__typename: "Greeting",
47+
message: "Hello, John",
48+
recipient: {
49+
__typename: "Person",
50+
name: "John Doe",
51+
},
52+
},
53+
};
54+
```
55+
56+
This data is not `complete` because `recipient.email` is missing. This data is also not `streaming` because the data requirements in the `@defer` boundary are partially fulfilled due to the existence of `recipient`. This could lead to runtime crashes on `recipient.email` if you use the existence of `recipient` to detect whether data in the `@defer` boundary has streamed in or not. This change now accurately reports this as `partial` to ensure the field is marked as a partial field in `recipient`.
57+
58+
2. Scenario 2: partial data written to the cache that fulfills the data requirements of the `@defer` boundary
59+
60+
Let's say the cache contained the following partial data:
61+
62+
```ts
63+
{
64+
greeting: {
65+
__typename: "Greeting",
66+
recipient: {
67+
__typename: "Person",
68+
name: "John Doe",
69+
email: "john@example.com",
70+
},
71+
},
72+
};
73+
```
74+
75+
After the first chunk arrives from the server, the data looks like the following:
76+
77+
```ts
78+
{
79+
greeting: {
80+
__typename: "Greeting",
81+
message: "Hello, John",
82+
recipient: {
83+
__typename: "Person",
84+
name: "John Doe",
85+
email: "john@example.com",
86+
},
87+
},
88+
};
89+
```
90+
91+
In this case, the combination of the first chunk and the partial data in the cache now fulfills the data requirements of the query. Even though the server is still streaming data (`NetworkStatus.streaming`), we can report this as `dataState: "complete"` since it is safe to access data on all fields.
92+
93+
This change also means `@stream` queries by definition fulfill the data requirements of the query after the first chunk arrives since `@stream` operates on lists and contains no data holes. `@stream` queries now accurately report `dataState` as `complete` or `partial`, depending on whether the list mixes partial data with streamed list items.
94+
95+
As a result of this change, some cases where you'd previously see `dataState` reported as `"streaming"` are now reported as `partial` or `complete`.
96+
97+
If you use `dataState` to determine whether an incremental request is still in-flight, please use `networkStatus` instead to check for `NetworkStatus.streaming`. `dataState` is type narrowing feature and not intended to report the network status.

.changeset/fast-geckos-help.md

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
---
2+
"@apollo/client": patch
3+
---
4+
5+
Fix an issue where field `read` functions were not applied to intermediate results while streaming `@defer` responses. `cache.diff` ran the `read` functions, but the transformed values were only applied to the emitted result when the updated cache result was considered complete. Intermediate chunks whose only holes were at `@defer` boundaries now correctly return the result of field `read` functions.
6+
7+
```ts
8+
new InMemoryCache({
9+
typePolicies: {
10+
Greeting: {
11+
fields: {
12+
message: {
13+
read: (message) => message.toUpperCase(),
14+
},
15+
},
16+
},
17+
},
18+
});
19+
20+
// query GreetingQuery {
21+
// greeting {
22+
// message
23+
// ... @defer {
24+
// recipient { name }
25+
// }
26+
// }
27+
// }
28+
29+
// First chunk previously returned:
30+
// { greeting: { message: "Hello world" } }
31+
//
32+
// Now correctly returns while still streaming:
33+
// { greeting: { message: "HELLO WORLD" } }
34+
```

.changeset/serious-bugs-move.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@apollo/client": patch
3+
---
4+
5+
Fix an issue with `@stream` queries when using `returnPartialData: true` where the streamed list was truncated after the first incremental chunk when the list contained partial cache data. The list is no longer truncated and partial list items are now retained as incremental chunks arrive. The `dataState` is now reported as `partial` until the server has streamed enough of the list so that each list item fully satisfies the query.
6+
7+
This change also updates `@stream` queries so that they reported with `dataState: "complete` instead of `"streaming"` since it is safe to access all fields in the response.

.size-limits.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
2-
"import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (CJS)": 48863,
3-
"import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production) (CJS)": 43151,
4-
"import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\"": 36730,
5-
"import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production)": 30145
2+
"import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (CJS)": 49697,
3+
"import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production) (CJS)": 43842,
4+
"import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\"": 37288,
5+
"import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production)": 30735
66
}

src/__tests__/__snapshots__/exports.ts.snap

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,7 @@ Array [
445445
"canonicalStringify",
446446
"checkDocument",
447447
"cloneDeep",
448+
"collectSiblingFields",
448449
"combineLatestBatched",
449450
"compact",
450451
"createFragmentMap",
@@ -474,11 +475,13 @@ Array [
474475
"hasDirectives",
475476
"hasForcedResolvers",
476477
"isArray",
478+
"isDeferredFragment",
477479
"isDocumentNode",
478480
"isField",
479481
"isNonEmptyArray",
480482
"isNonNullObject",
481483
"isPlainObject",
484+
"isTypenameField",
482485
"makeReference",
483486
"makeUniqueId",
484487
"mapObservableFragmentMemoized",

src/core/ObservableQuery.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1009,10 +1009,7 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`,
10091009
// will be overwritten anyways, just here for types sake
10101010
loading: false,
10111011
data: diff.result,
1012-
dataState:
1013-
fetchMoreResult.dataState === "streaming" ?
1014-
"streaming"
1015-
: "complete",
1012+
dataState: diff.complete ? "complete" : "streaming",
10161013
},
10171014
});
10181015
}

0 commit comments

Comments
 (0)