Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The partial data type uses the unmasked DataValue.Partial<TData> instead of wrapping with MaybeMasked, which is inconsistent with the resource's complete-data typing and Apollo Client's own useQuery.Result.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
This PR adds type-only support for returnPartialData: true to useQuery. When that option is set, Apollo Client's cache can return an incomplete result until the network responds, so the returned data should be typed as partial. Previously useQuery always typed data as complete, so reading a missing field would compile. The PR introduces a PartialQueryResource type and adds overloads to both the Classic and Modern signature styles that return it when returnPartialData: true is present. There is no runtime change.
Changes:
- Added
PartialQueryResource<TData, TVariables>inquery.ts, overridingdatatoDataValue.Partial<TData> | undefined. - Added
returnPartialData: trueoverloads touseQuery.Signatures.Classicand.ModernreturningPartialQueryResource. - Exported
PartialQueryResourcefrom the package entrypoint and added type tests for both signature styles.
| File | Description |
|---|---|
| glimmer-apollo/src/-private/query.ts | Imports DataValue and defines the new PartialQueryResource type. |
| glimmer-apollo/src/-private/usables.ts | Adds returnPartialData: true overloads to the Classic and Modern signatures. |
| glimmer-apollo/src/index.ts | Re-exports PartialQueryResource from the public API. |
| test-app/tests/unit/types/query-types-test.ts | Adds type assertions covering partial data for both signature styles. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
With returnPartialData, data can miss fields, so Apollo Client types it as DataValue.Partial<TData>; useQuery typed it as complete. QueryResource gets a third type parameter for the shape of data, and the returnPartialData overloads (literal true or a boolean flag, as in Apollo's own overloads) return it as DataValue.Partial<MaybeMasked<TData>>. onComplete receives the same shape. A complete resource stays assignable to a partial one. Also drop the NoInfer import from @apollo/client/utilities/internal: Apollo 4.3 removed it and TypeScript 5.4+ ships it. This is what failed the floating dependencies CI job.
45010a2 to
d6a677f
Compare
With
returnPartialData: true,dataholds what the cache had until the network answers, so fields can be missing. Apollo Client 4 types this asDataValue.Partial<TData>(dataState: "partial").useQuerytyped it as complete, so code that reads a missing field compiles.This adds an overload to both signature styles: with
returnPartialData: truein the options,useQueryreturns aPartialQueryResourcewhosedataisDataValue.Partial<TData> | undefined. No runtime change.The same approach in Apollo Client 4.2.10:
useQueryoverload forreturnPartialData: true: src/react/hooks/useQuery.ts#L334-L352"partial"data state: src/core/types.ts#L318-L323DataValue.Partial: src/core/types.ts#L139Type tests cover both signature styles; the existing tests and the test-app suite pass.
Cowritten by Claude