Skip to content

Commit 1d581d2

Browse files
AmariahAKatlarix-agentrenovate[bot]jerelmiller
authored
perf: defer MissingFieldError construction and avoid JSON.stringify in cache diff (#13329)
Fixes #13305. ## Problem Two expensive operations run eagerly on the cache-diff hot path even when the caller only checks `diff.complete` (a cheap boolean): 1. **`JSON.stringify(objectOrReference, null, 2)`** — pretty-prints the full parent object for every missing-field message, scaling with object size. 2. **`new MissingFieldError(...)`** — constructs an `Error` subclass (paying V8 stack capture) per incomplete diff, even though `diff.missing` is only consumed in `__DEV__`-guarded logging on most call paths. ## Solution - **Cheap messages:** Replaced the `JSON.stringify` with a `__typename` lookup. Embedded parents now produce `"object Profile"` instead of the full pretty-printed object dump. - **Lazy errors:** `diffQueryAgainstStore` now derives `diff.complete` from the raw `execResult.missing` tree (a `MissingTree`). The `MissingFieldError` is built lazily via a getter, only when `diff.missing` is actually accessed. The getter caches the result so repeated access is cheap. All three consumers of `diff.missing` are safe — two are `__DEV__`-guarded, one is a legitimate `watchFragment` consumer. The hot `broadcastWatch` path only compares `diff.result` with `equal()`, so the getter is never triggered by property enumeration. ## Changes | File | Change | |------|--------| | `src/cache/inmemory/readFromStore.ts` | Cheap missing-field message + lazy `MissingFieldError` getter | | `src/cache/inmemory/__tests__/readFromStore.ts` | 2 new tests + 3 existing assertions updated | | `src/cache/inmemory/__tests__/diffAgainstStore.ts` | 1 assertion updated | | `src/cache/inmemory/__tests__/policies.ts` | 1 assertion updated | | `.changeset/lazy-diff-diagnostics.md` | Patch-level changeset | ### Checklist: - [x] Includes a changeset - [x] Significant new logic is covered by tests - [ ] ~~New feature~~ (not applicable — performance fix for existing behavior) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Performance** - Improved cache diff performance by avoiding unnecessary serialization of stored objects. - Missing-field diagnostics are now generated only when requested. - **Bug Fixes** - Improved missing-field error messages with shorter, more relevant object descriptions. - Cache completeness is now reported accurately without requiring full error construction. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: DeepSeek V4 Pro agent <agent@atlarix.dev> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Jerel Miller <jerelmiller@gmail.com>
1 parent 880eae8 commit 1d581d2

3 files changed

Lines changed: 154 additions & 18 deletions

File tree

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+
Cache diffs for incomplete queries no longer pay the cost of building a full `MissingFieldError` when the `missing` property is not accessed. The error object is now only constructed when the `missing` property is accessed the first time. This improves performance by avoiding a V8 stack capture when `missing` is ignored entirely.
6+
7+
As an additional small performance improvement, `JSON.stringify` is no longer used in the error message on objects whose cache ID is known. `JSON.stringify` is only used for non-normalized objects.

src/cache/inmemory/__tests__/readFromStore.ts

Lines changed: 125 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1612,12 +1612,12 @@ describe("reading from the store", () => {
16121612
],
16131613
},
16141614
missing: new MissingFieldError(
1615-
"Can't find field 'id' on object undefined",
1615+
"Can't find field 'id' on object {}",
16161616
{
16171617
ducks: {
16181618
2: {
1619-
id: "Can't find field 'id' on object undefined",
1620-
quacking: "Can't find field 'quacking' on object undefined",
1619+
id: "Can't find field 'id' on object {}",
1620+
quacking: "Can't find field 'quacking' on object {}",
16211621
},
16221622
},
16231623
},
@@ -2178,3 +2178,125 @@ describe("reading from the store", () => {
21782178
expect(result2.abc).toBe(abc);
21792179
});
21802180
});
2181+
2182+
describe("lazy MissingFieldError diagnostics", () => {
2183+
it("only constructs MissingFieldError when diff.missing is accessed", () => {
2184+
const cache = new InMemoryCache();
2185+
2186+
const fullQuery = gql`
2187+
query {
2188+
customer {
2189+
id
2190+
name
2191+
address {
2192+
street
2193+
city
2194+
}
2195+
}
2196+
}
2197+
`;
2198+
2199+
const partialQuery = gql`
2200+
query {
2201+
customer {
2202+
id
2203+
}
2204+
}
2205+
`;
2206+
2207+
cache.writeQuery({
2208+
query: partialQuery,
2209+
data: {
2210+
customer: {
2211+
__typename: "Customer",
2212+
id: "c1",
2213+
},
2214+
},
2215+
});
2216+
2217+
const diff = cache.diff({
2218+
query: fullQuery,
2219+
returnPartialData: true,
2220+
optimistic: true,
2221+
});
2222+
2223+
// @ts-ignore
2224+
const missingSpy = jest.spyOn(diff, "missing", "get");
2225+
expect(missingSpy).not.toHaveBeenCalled();
2226+
2227+
// diff.missing should be lazily constructed only when accessed
2228+
expect(diff.missing).toEqual(
2229+
new MissingFieldError(
2230+
"Can't find field 'name' on Customer:c1 object",
2231+
{
2232+
customer: {
2233+
name: "Can't find field 'name' on Customer:c1 object",
2234+
address: "Can't find field 'address' on Customer:c1 object",
2235+
},
2236+
},
2237+
fullQuery,
2238+
{}
2239+
)
2240+
);
2241+
2242+
expect(missingSpy).toHaveBeenCalledTimes(1);
2243+
});
2244+
2245+
it("missing message uses JSON.stringify for non-normalized embedded parents", () => {
2246+
const cache = new InMemoryCache();
2247+
2248+
const query = gql`
2249+
query {
2250+
profile {
2251+
bio
2252+
largeField
2253+
}
2254+
}
2255+
`;
2256+
2257+
cache.writeQuery({
2258+
query: gql`
2259+
query {
2260+
profile {
2261+
bio
2262+
}
2263+
}
2264+
`,
2265+
data: {
2266+
profile: {
2267+
__typename: "Profile",
2268+
bio: "a".repeat(10),
2269+
},
2270+
},
2271+
});
2272+
2273+
const diff = cache.diff({
2274+
query,
2275+
returnPartialData: true,
2276+
optimistic: true,
2277+
});
2278+
2279+
const message = `Can't find field 'largeField' on object ${JSON.stringify(
2280+
{ __typename: "Profile", bio: "a".repeat(10) },
2281+
null,
2282+
2
2283+
)}`;
2284+
2285+
expect(diff).toStrictEqualTyped({
2286+
result: {
2287+
profile: { __typename: "Profile", bio: "a".repeat(10) },
2288+
},
2289+
complete: false,
2290+
missing: new MissingFieldError(
2291+
message,
2292+
{
2293+
profile: {
2294+
largeField: message,
2295+
},
2296+
},
2297+
query,
2298+
{}
2299+
),
2300+
});
2301+
});
2302+
});

src/cache/inmemory/readFromStore.ts

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -224,18 +224,10 @@ export class StoreReader {
224224
},
225225
});
226226

227-
let missing: MissingFieldError | undefined;
228-
if (execResult.missing) {
229-
missing = new MissingFieldError(
230-
firstMissing(execResult.missing)!,
231-
execResult.missing,
232-
query,
233-
variables
234-
);
235-
}
227+
const { result, missing: rawMissing } = execResult;
228+
const complete = !rawMissing;
236229

237-
const complete = !missing;
238-
const { result } = execResult;
230+
let missingError: MissingFieldError | undefined;
239231

240232
return {
241233
result:
@@ -246,7 +238,17 @@ export class StoreReader {
246238
: result
247239
: null,
248240
complete,
249-
missing,
241+
get missing() {
242+
if (missingError === void 0 && rawMissing) {
243+
missingError = new MissingFieldError(
244+
firstMissing(rawMissing)!,
245+
rawMissing,
246+
query,
247+
variables
248+
);
249+
}
250+
return missingError;
251+
},
250252
} as Cache.DiffResult<T>;
251253
}
252254

@@ -338,11 +340,16 @@ export class StoreReader {
338340

339341
if (fieldValue === void 0) {
340342
if (!addTypenameToDocument.added(selection)) {
343+
const id =
344+
isReference(objectOrReference) ? objectOrReference.__ref
345+
: objectOrReference ? policies.identify(objectOrReference)[0]
346+
: undefined;
347+
341348
missing = missingMerger.merge(missing, {
342349
[resultName]: `Can't find field '${selection.name.value}' on ${
343-
isReference(objectOrReference) ?
344-
objectOrReference.__ref + " object"
345-
: "object " + JSON.stringify(objectOrReference, null, 2)
350+
id ?
351+
`${id} object`
352+
: `object ${JSON.stringify(objectOrReference || {}, null, 2)}`
346353
}`,
347354
});
348355
}

0 commit comments

Comments
 (0)