Do not mutate a shallowly frozen subtree

Reviewed By: alunyov Differential Revision: D51048239 fbshipit-source-id: e78087c3a52888cce18217f4a1ae16d2b0cdea2b
facebook · Nov 15, 2023 · 7f54255 · 7f54255
1 parent 4f7739a
commit 7f54255
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 5 deletions.
diff --git a/packages/relay-runtime/util/__tests__/recycleNodesInto-test.js b/packages/relay-runtime/util/__tests__/recycleNodesInto-test.js
@@ -348,7 +348,7 @@ describe('recycleNodesInto', () => {
     });
   });
 
-  describe('deepFreeze', () => {
+  describe('freeze', () => {
     it('does not mutate deeply frozen array in `nextData`', () => {
       const prevData = [[{x: 1}], 1];
       const nextData = [[{x: 1}], 2];
@@ -378,6 +378,25 @@ describe('recycleNodesInto', () => {
       expect(recycled.a.b).toBe(nextItem);
     });
 
+    it('does not mutate into frozen object in `nextData`', () => {
+      const nextItem = {
+        c: 1,
+      };
+      const nextObject = {
+        b: nextItem,
+      };
+      const nextData = {
+        a: nextObject,
+      };
+      const prevData = {a: {b: {c: 1}}, d: 1};
+
+      Object.freeze(nextData);
+      const recycled = recycleNodesInto(prevData, nextData);
+      expect(recycled).toBe(nextData);
+      expect(recycled.a).toBe(nextObject);
+      expect(recycled.a.b).toBe(nextItem);
+    });
+
     it('reuse prevData and does not mutate deeply frozen array in `nextData`', () => {
       const nextItem = {x: 1};
       const nextArray = [nextItem];
@@ -414,4 +433,28 @@ describe('recycleNodesInto', () => {
       expect(nextData.a.b).toBe(nextItem);
     });
   });
+
+  it('reuse prevData and does not mutate frozen object in `nextData`', () => {
+    const nextItem = {
+      c: 1,
+    };
+    const nextObject = {
+      b: nextItem,
+    };
+    const nextData = {
+      a: nextObject,
+    };
+    const prevData = {
+      a: {
+        b: {
+          c: 1,
+        },
+      },
+    };
+    Object.freeze(nextData);
+    const recycled = recycleNodesInto(prevData, nextData);
+    expect(recycled).toBe(prevData);
+    expect(nextData.a).toBe(nextObject);
+    expect(nextData.a.b).toBe(nextItem);
+  });
 });
diff --git a/packages/relay-runtime/util/recycleNodesInto.js b/packages/relay-runtime/util/recycleNodesInto.js
@@ -13,8 +13,17 @@
 
 /**
  * Recycles subtrees from `prevData` by replacing equal subtrees in `nextData`.
+ * Does not mutate a frozen subtree.
  */
 function recycleNodesInto<T>(prevData: T, nextData: T): T {
+  return recycleNodesIntoImpl(prevData, nextData, true);
+}
+
+function recycleNodesIntoImpl<T>(
+  prevData: T,
+  nextData: T,
+  canMutate: boolean,
+): T {
   if (
     prevData === nextData ||
     typeof prevData !== 'object' ||
@@ -32,11 +41,16 @@ function recycleNodesInto<T>(prevData: T, nextData: T): T {
   const prevArray: ?Array<mixed> = Array.isArray(prevData) ? prevData : null;
   const nextArray: ?Array<mixed> = Array.isArray(nextData) ? nextData : null;
   if (prevArray && nextArray) {
+    const canMutateNext = canMutate && !Object.isFrozen(nextArray);
     canRecycle =
       nextArray.reduce((wasEqual, nextItem, ii) => {
         const prevValue = prevArray[ii];
-        const nextValue = recycleNodesInto(prevValue, nextItem);
-        if (nextValue !== nextArray[ii] && !Object.isFrozen(nextArray)) {
+        const nextValue = recycleNodesIntoImpl(
+          prevValue,
+          nextItem,
+          canMutateNext,
+        );
+        if (nextValue !== nextArray[ii] && canMutateNext) {
           nextArray[ii] = nextValue;
         }
         return wasEqual && nextValue === prevArray[ii];
@@ -47,11 +61,16 @@ function recycleNodesInto<T>(prevData: T, nextData: T): T {
     const nextObject = nextData;
     const prevKeys = Object.keys(prevObject);
     const nextKeys = Object.keys(nextObject);
+    const canMutateNext = canMutate && !Object.isFrozen(nextObject);
     canRecycle =
       nextKeys.reduce((wasEqual, key) => {
         const prevValue = prevObject[key];
-        const nextValue = recycleNodesInto(prevValue, nextObject[key]);
-        if (nextValue !== nextObject[key] && !Object.isFrozen(nextObject)) {
+        const nextValue = recycleNodesIntoImpl(
+          prevValue,
+          nextObject[key],
+          canMutateNext,
+        );
+        if (nextValue !== nextObject[key] && canMutateNext) {
           // $FlowFixMe[cannot-write]
           nextObject[key] = nextValue;
         }