Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/lazy-diff-diagnostics.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@apollo/client": patch
---

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.

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.
128 changes: 125 additions & 3 deletions src/cache/inmemory/__tests__/readFromStore.ts
Comment thread
AmariahAK marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -1612,12 +1612,12 @@ describe("reading from the store", () => {
],
},
missing: new MissingFieldError(
"Can't find field 'id' on object undefined",
"Can't find field 'id' on object {}",
{
ducks: {
2: {
id: "Can't find field 'id' on object undefined",
quacking: "Can't find field 'quacking' on object undefined",
id: "Can't find field 'id' on object {}",
quacking: "Can't find field 'quacking' on object {}",
},
},
},
Expand Down Expand Up @@ -2178,3 +2178,125 @@ describe("reading from the store", () => {
expect(result2.abc).toBe(abc);
});
});

describe("lazy MissingFieldError diagnostics", () => {
it("only constructs MissingFieldError when diff.missing is accessed", () => {
const cache = new InMemoryCache();

const fullQuery = gql`
query {
customer {
id
name
address {
street
city
}
}
}
`;

const partialQuery = gql`
query {
customer {
id
}
}
`;

cache.writeQuery({
query: partialQuery,
data: {
customer: {
__typename: "Customer",
id: "c1",
},
},
});

const diff = cache.diff({
query: fullQuery,
returnPartialData: true,
optimistic: true,
});

// @ts-ignore
const missingSpy = jest.spyOn(diff, "missing", "get");
expect(missingSpy).not.toHaveBeenCalled();

// diff.missing should be lazily constructed only when accessed
expect(diff.missing).toEqual(
new MissingFieldError(
"Can't find field 'name' on Customer:c1 object",
{
customer: {
name: "Can't find field 'name' on Customer:c1 object",
address: "Can't find field 'address' on Customer:c1 object",
},
},
fullQuery,
{}
)
);

expect(missingSpy).toHaveBeenCalledTimes(1);
});

it("missing message uses JSON.stringify for non-normalized embedded parents", () => {
const cache = new InMemoryCache();

const query = gql`
query {
profile {
bio
largeField
}
}
`;

cache.writeQuery({
query: gql`
query {
profile {
bio
}
}
`,
data: {
profile: {
__typename: "Profile",
bio: "a".repeat(10),
},
},
});

const diff = cache.diff({
query,
returnPartialData: true,
optimistic: true,
});

const message = `Can't find field 'largeField' on object ${JSON.stringify(
{ __typename: "Profile", bio: "a".repeat(10) },
null,
2
)}`;

expect(diff).toStrictEqualTyped({
result: {
profile: { __typename: "Profile", bio: "a".repeat(10) },
},
complete: false,
missing: new MissingFieldError(
message,
{
profile: {
largeField: message,
},
},
query,
{}
),
});
});
});
37 changes: 22 additions & 15 deletions src/cache/inmemory/readFromStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,18 +224,10 @@ export class StoreReader {
},
});

let missing: MissingFieldError | undefined;
if (execResult.missing) {
missing = new MissingFieldError(
firstMissing(execResult.missing)!,
execResult.missing,
query,
variables
);
}
const { result, missing: rawMissing } = execResult;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI apologies ahead of time. This code is going to change once I merge in my branch. Unfortunately I happened to be working on a bunch of tweaks to the cache reader at the same time. I'm only saying this so that if you go searching for these changes in main after 4.3 is released, don't be surprised if this looks different!

const complete = !rawMissing;

const complete = !missing;
const { result } = execResult;
let missingError: MissingFieldError | undefined;

return {
result:
Expand All @@ -246,7 +238,17 @@ export class StoreReader {
: result
: null,
complete,
missing,
get missing() {
if (missingError === void 0 && rawMissing) {
missingError = new MissingFieldError(
firstMissing(rawMissing)!,
rawMissing,
query,
variables
);
}
return missingError;
},
} as Cache.DiffResult<T>;
}

Expand Down Expand Up @@ -338,11 +340,16 @@ export class StoreReader {

if (fieldValue === void 0) {
if (!addTypenameToDocument.added(selection)) {
const id =
isReference(objectOrReference) ? objectOrReference.__ref
: objectOrReference ? policies.identify(objectOrReference)[0]
: undefined;

missing = missingMerger.merge(missing, {
[resultName]: `Can't find field '${selection.name.value}' on ${
isReference(objectOrReference) ?
objectOrReference.__ref + " object"
: "object " + JSON.stringify(objectOrReference, null, 2)
id ?
`${id} object`
: `object ${JSON.stringify(objectOrReference || {}, null, 2)}`
}`,
});
}
Expand Down