fix #2049 - dont try to modify frozen objects

facebook · Mar 17, 2018 · 4561712 · 4561712
1 parent 17cc27b
commit 4561712
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 5 deletions.
diff --git a/packages/react-relay/classic/legacy/store/__tests__/recycleNodesInto-test.js b/packages/react-relay/classic/legacy/store/__tests__/recycleNodesInto-test.js
@@ -91,7 +91,7 @@ describe('recycleNodesInto', () => {
       const recycled = recycleNodesInto(prevData, nextData);
 
       expect(recycled).not.toBe(prevData);
-      expect(recycled.bar).toBe(prevData.bar);
+      expect(recycled.foo).toBe(prevData.foo);
     });
 
     it('recycles identical objects', () => {
@@ -112,6 +112,28 @@ describe('recycleNodesInto', () => {
       expect(recycled).toBe(prevData);
     });
 
+    it('does not mutate frozen equal parent objects with equal leaf objects', () => {
+      const prevData = {foo: {bar: 1}};
+      const nextData = {foo: {bar: 1}};
+      Object.freeze(nextData);
+      Object.freeze(nextData.foo);
+      const recycled = recycleNodesInto(prevData, nextData);
+
+      expect(recycled).toBe(prevData);
+      expect(recycled.foo).toBe(prevData.foo);
+    });
+
+    it('does not mutate frozen unequal parent objects with equal leaf objects', () => {
+      const prevData = {foo: {bar: 1}, baz: 2};
+      const nextData = {foo: {bar: 1}, baz: 200};
+      Object.freeze(nextData);
+      Object.freeze(nextData.foo);
+      const recycled = recycleNodesInto(prevData, nextData);
+
+      expect(recycled).not.toBe(prevData);
+      expect(recycled.foo).not.toBe(prevData.foo);
+    });
+
     it('does not recycle arrays as objects', () => {
       const prevData = [1, 2];
       const nextData = {0: 1, 1: 2};
@@ -142,6 +164,13 @@ describe('recycleNodesInto', () => {
       expect(recycleNodesInto(prevData, nextData)).toBe(prevData);
     });
 
+    it('recycles arrays with equal objects without mutating frozen `nextData`', () => {
+      const prevData = [{foo: 1}, {bar: 2}];
+      const nextData = [{foo: 1}, {bar: 2}];
+      Object.freeze(nextData);
+      expect(recycleNodesInto(prevData, nextData)).toBe(prevData);
+    });
+
     it('recycles arrays without mutating `prevData`', () => {
       const prevItem = {foo: 1};
       const prevData = [prevItem];

diff --git a/packages/relay-runtime/util/recycleNodesInto.js b/packages/relay-runtime/util/recycleNodesInto.js
@@ -30,29 +30,31 @@ function recycleNodesInto<T>(prevData: T, nextData: T): T {
   const prevArray = Array.isArray(prevData) ? prevData : null;
   const nextArray = Array.isArray(nextData) ? nextData : null;
   if (prevArray && nextArray) {
+    const isFrozen = __DEV__ && Object.isFrozen(nextArray);
     canRecycle =
       nextArray.reduce((wasEqual, nextItem, ii) => {
         const prevValue = prevArray[ii];
         const nextValue = recycleNodesInto(prevValue, nextItem);
-        if (nextValue !== nextArray[ii]) {
+        if (nextValue !== nextArray[ii] && !isFrozen) {
           nextArray[ii] = nextValue;
         }
-        return wasEqual && nextArray[ii] === prevArray[ii];
+        return wasEqual && nextValue === prevArray[ii];
       }, true) && prevArray.length === nextArray.length;
   } else if (!prevArray && !nextArray) {
     // Assign local variables to preserve Flow type refinement.
     const prevObject = prevData;
     const nextObject = nextData;
     const prevKeys = Object.keys(prevObject);
     const nextKeys = Object.keys(nextObject);
+    const isFrozen = __DEV__ && Object.isFrozen(nextObject);
     canRecycle =
       nextKeys.reduce((wasEqual, key) => {
         const prevValue = prevObject[key];
         const nextValue = recycleNodesInto(prevValue, nextObject[key]);
-        if (nextValue !== nextObject[key]) {
+        if (nextValue !== nextObject[key] && !isFrozen) {
           nextObject[key] = nextValue;
         }
-        return wasEqual && nextObject[key] === prevObject[key];
+        return wasEqual && nextValue === prevObject[key];
       }, true) && prevKeys.length === nextKeys.length;
   }
   return canRecycle ? prevData : nextData;