Avoid mutating incoming data in Policies#applyMerges.

Fixes #6245.

src/cache/inmemory/__tests__/policies.ts

@@ -3145,4 +3145,85 @@ describe("type policies", function () {
       secondFirstBookResult.author.firstBook,
     );
   });
+
+  it("can return existing object from merge function (issue #6245)", function () {
+    const cache = new InMemoryCache({
+      typePolicies: {
+        Person: {
+          fields: {
+            currentTask: {
+              merge(existing, incoming) {
+                // Not a very reasonable merge strategy, but returning
+                // existing here triggers issue #6245, persumably because
+                // the existing data is frozen.
+                return existing || incoming;
+              },
+            },
+          },
+        },
+        Task: {
+          keyFields: false,
+        },
+      },
+    });
+
+    const query = gql`
+      query {
+        person {
+          currentTask {
+            __typename
+            description
+          }
+        }
+      }
+    `;
+
+    cache.writeQuery({
+      query,
+      data: {
+        person: {
+          __typename: "Person",
+          id: 1234,
+          currentTask: {
+            __typename: "Task",
+            description: "writing tests",
+          },
+        },
+      },
+    });
+
+    const snapshot = cache.extract();
+    expect(snapshot).toEqual({
+      "Person:1234": {
+        __typename: "Person",
+        currentTask: {
+          __typename: "Task",
+          description: "writing tests",
+        },
+      },
+      ROOT_QUERY: {
+        __typename: "Query",
+        person: {
+          __ref: "Person:1234",
+        },
+      },
+    });
+
+    cache.writeQuery({
+      query,
+      data: {
+        person: {
+          __typename: "Person",
+          id: 1234,
+          currentTask: {
+            __typename: "Task",
+            description: "polishing knives",
+          },
+        },
+      },
+    });
+
+    // Unchanged because the merge function prefers the existing object.
+    expect(cache.extract()).toEqual(snapshot);
+  });
 });

src/cache/inmemory/helpers.ts

@@ -36,6 +36,15 @@ export interface FieldValueToBeMerged {
   __value: StoreValue;
 }
 
+export function storeValueIsStoreObject(
+  value: StoreValue,
+): value is StoreObject {
+  return value !== null &&
+    typeof value === "object" &&
+    !isReference(value) &&
+    !Array.isArray(value);
+}
+
 export function isFieldValueToBeMerged(
   value: any,
 ): value is FieldValueToBeMerged {

src/cache/inmemory/policies.ts

@@ -29,6 +29,7 @@ import {
   fieldNameFromStoreName,
   FieldValueToBeMerged,
   isFieldValueToBeMerged,
+  storeValueIsStoreObject,
 } from './helpers';
 import { FieldValueGetter, ToReferenceFunction } from './entityStore';
 import { SafeReadonly } from '../core/types/common';
@@ -579,28 +580,24 @@ export class Policies {
       )) as T;
     }
 
-    if (incoming && typeof incoming === "object") {
-      if (isReference(incoming)) {
-        return incoming;
-      }
-
-      if (Array.isArray(incoming)) {
-        return incoming!.map(item => policies.applyMerges(
-          // Items in the same position in different arrays are not
-          // necessarily related to each other, so there is no basis for
-          // merging them. Passing void here means any FieldValueToBeMerged
-          // objects within item will be handled as if there was no existing
-          // data. Also, we do not pass storageKeys because the array itself
-          // is never an entity with a __typename, so its indices can never
-          // have custom read or merge functions.
-          void 0,
-          item,
-          context,
-        )) as T;
-      }
+    if (Array.isArray(incoming)) {
+      return incoming!.map(item => policies.applyMerges(
+        // Items in the same position in different arrays are not
+        // necessarily related to each other, so there is no basis for
+        // merging them. Passing void here means any FieldValueToBeMerged
+        // objects within item will be handled as if there was no existing
+        // data. Also, we do not pass storageKeys because the array itself
+        // is never an entity with a __typename, so its indices can never
+        // have custom read or merge functions.
+        void 0,
+        item,
+        context,
+      )) as T;
+    }
 
+    if (storeValueIsStoreObject(incoming)) {
       const e = existing as StoreObject | Reference;
-      const i = incoming as object as StoreObject;
+      const i = incoming as StoreObject;
 
       // If the existing object is a { __ref } object, e.__ref provides a
       // stable key for looking up the storage object associated with
@@ -613,17 +610,28 @@ export class Policies {
         ? e.__ref
         : typeof e === "object" && e;
 
+      let newFields: StoreObject | undefined;
+
       Object.keys(i).forEach(storeFieldName => {
-        i[storeFieldName] = policies.applyMerges(
+        const incomingValue = i[storeFieldName];
+        const appliedValue = policies.applyMerges(
           context.getFieldValue(e, storeFieldName),
-          i[storeFieldName],
+          incomingValue,
           context,
-          // Avoid enabling storage when firstStorageKey is falsy, which
-          // implies no options.storage object has ever been created for a
-          // read function for this field.
+          // Avoid enabling options.storage when firstStorageKey is falsy,
+          // which implies no options.storage object has ever been created
+          // for a read/merge function for this field.
           firstStorageKey ? [firstStorageKey, storeFieldName] : void 0,
         );
+        if (appliedValue !== incomingValue) {
+          newFields = newFields || Object.create(null);
+          newFields![storeFieldName] = appliedValue;
+        }
       });
+
+      if (newFields) {
+        return { ...i, ...newFields } as typeof incoming;
+      }
     }
 
     return incoming;
@@ -694,8 +702,8 @@ function makeFieldFunctionOptions(
 
         if (
           typesDiffer ||
-          !canBeMerged(existing) ||
-          !canBeMerged(applied)
+          !storeValueIsStoreObject(existing) ||
+          !storeValueIsStoreObject(applied)
         ) {
           return applied;
         }
@@ -708,15 +716,6 @@ function makeFieldFunctionOptions(
   };
 }
 
-function canBeMerged(obj: StoreValue): boolean {
-  return !!(
-    obj &&
-    typeof obj === "object" &&
-    !isReference(obj) &&
-    !Array.isArray(obj)
-  );
-}
-
 function keyArgsFnFromSpecifier(
   specifier: KeySpecifier,
 ): KeyArgsFunction {