From 1ee5b01762ad1091555c8175999b0a01eb1f68f9 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Wed, 7 Jan 2026 16:42:01 +0100 Subject: [PATCH] Fix `readField` during merge of objects sandwiched between circular objects. --- .changeset/pink-jobs-sing.md | 5 + src/cache/inmemory/__tests__/policies.ts | 118 +++++++++++++++++++++++ src/cache/inmemory/entityStore.ts | 2 +- src/cache/inmemory/policies.ts | 19 ++-- 4 files changed, 137 insertions(+), 7 deletions(-) create mode 100644 .changeset/pink-jobs-sing.md diff --git a/.changeset/pink-jobs-sing.md b/.changeset/pink-jobs-sing.md new file mode 100644 index 00000000000..c9054c1c043 --- /dev/null +++ b/.changeset/pink-jobs-sing.md @@ -0,0 +1,5 @@ +--- +"@apollo/client": patch +--- + +Fix `readField` during merge of objects sandwiched between circular objects. diff --git a/src/cache/inmemory/__tests__/policies.ts b/src/cache/inmemory/__tests__/policies.ts index ea20048f845..3389d90dbde 100644 --- a/src/cache/inmemory/__tests__/policies.ts +++ b/src/cache/inmemory/__tests__/policies.ts @@ -2795,6 +2795,124 @@ describe("type policies", function () { }); }); + it("`readField` in `merge` with indirectly self-nested object works", () => { + let messageIdFields: string[]; + let aliasedMessageTitleFields: string[]; + const conversationMergePolicy = jest.fn((existing, incoming) => incoming); + const cache = new InMemoryCache({ + typePolicies: { + Conversation: { + merge: conversationMergePolicy, + fields: { + messages: { + keyArgs: false, + merge: (existing, incoming, { readField }) => { + messageIdFields = incoming.map((m: any) => + readField("id", m) + ); + aliasedMessageTitleFields = incoming.map((m: any) => + // aliased fields can be read by their original name + // even if they haven't been written to the cache yet + // due to a merge function within a circular reference. + readField("title", m) + ); + return incoming; + }, + }, + }, + }, + }, + }); + + const query = gql` + query GetConversation { + conversation { + id + title + messages { + id + aliased: title + conversation { + id + meta + } + } + } + } + `; + + const conv1 = { + id: "c-1", + __typename: "Conversation", + }; + + cache.writeQuery({ + query, + data: { + conversation: { + ...conv1, + title: "Conversation 1", + messages: [ + { + id: "msg-1", + aliased: "Message 1", + conversation: { ...conv1, meta: "meta-1" }, + __typename: "Message", + }, + { + id: "msg-2", + aliased: "Message 2", + conversation: { ...conv1 }, + __typename: "Message", + }, + ], + }, + }, + }); + + expect(messageIdFields!).toEqual(["msg-1", "msg-2"]); + expect(aliasedMessageTitleFields!).toEqual(["Message 1", "Message 2"]); + // The merge function for the parent-and-child-nested Conversation + // object should have been called with the fully merged object as `incoming`. + // The fact that this is called 4 times, the first time to a self-reference and then three times + // without existing data, is something to be improved in a future PR. + expect(conversationMergePolicy).toHaveBeenNthCalledWith( + 1, + // TODO: fix this in a future PR + { __ref: "Conversation:c-1" }, + { + __typename: "Conversation", + id: "c-1", + title: "Conversation 1", + meta: "meta-1", + messages: expect.any(Array), + }, + expect.any(Object) + ); + expect(conversationMergePolicy).toHaveBeenNthCalledWith( + 2, + // TODO: fix this in a future PR + undefined, + { __ref: "Conversation:c-1" }, + expect.any(Object) + ); + expect(conversationMergePolicy).toHaveBeenNthCalledWith( + 3, + // TODO: fix this in a future PR + undefined, + { __ref: "Conversation:c-1" }, + expect.any(Object) + ); + expect(conversationMergePolicy).toHaveBeenNthCalledWith( + 4, + // TODO: fix this in a future PR + undefined, + { __ref: "Conversation:c-1" }, + expect.any(Object) + ); + expect(conversationMergePolicy).toHaveBeenCalledTimes(4); + }); + it("readField helper function calls custom read functions", function () { using _consoleSpies = spyOnConsole.takeSnapshots("error"); // Rather than writing ownTime data into the cache, we maintain it diff --git a/src/cache/inmemory/entityStore.ts b/src/cache/inmemory/entityStore.ts index ce139e603f9..b44fd279b8c 100644 --- a/src/cache/inmemory/entityStore.ts +++ b/src/cache/inmemory/entityStore.ts @@ -565,7 +565,7 @@ export abstract class EntityStore implements NormalizedCache { isReference(objectOrReference) ? this.get(objectOrReference.__ref, storeFieldName) : objectOrReference && objectOrReference[storeFieldName] - ) as SafeReadonly; + ) as SafeReadonly | undefined; // Returns true for non-normalized StoreObjects and non-dangling // References, indicating that readField(name, objOrRef) has a chance of diff --git a/src/cache/inmemory/policies.ts b/src/cache/inmemory/policies.ts index fa166735a3d..d63fdeadf21 100644 --- a/src/cache/inmemory/policies.ts +++ b/src/cache/inmemory/policies.ts @@ -891,7 +891,7 @@ export class Policies { public readField( options: ReadFieldOptions, - context: ReadMergeModifyContext + context: (ReadMergeModifyContext & { incomingById?: never }) | WriteContext ): SafeReadonly | undefined { const objectOrReference = options.from; if (!objectOrReference) return; @@ -899,20 +899,27 @@ export class Policies { const nameOrField = options.field || options.fieldName; if (!nameOrField) return; + const incomingObject = + isReference(objectOrReference) ? + context.incomingById?.get(objectOrReference.__ref)?.storeObject + : undefined; + if (options.typename === void 0) { - const typename = context.store.getFieldValue( - objectOrReference, - "__typename" - ); + const typename = + context.store.getFieldValue(objectOrReference, "__typename") ?? + incomingObject?.["__typename"]; if (typename) options.typename = typename; } const storeFieldName = this.getStoreFieldName(options); const fieldName = fieldNameFromStoreName(storeFieldName); - const existing = context.store.getFieldValue( + let existing = context.store.getFieldValue( objectOrReference, storeFieldName ); + if (existing === undefined) { + existing = incomingObject?.[storeFieldName] as typeof existing; + } const policy = this.getFieldPolicy(options.typename, fieldName); const read = policy && policy.read;