From 4173a5e149a073d792dbed48d32ede2365ba7b3e Mon Sep 17 00:00:00 2001
From: Mike Marcacci <mike.marcacci@gmail.com>
Date: Fri, 25 Jan 2019 15:23:01 -0800
Subject: [PATCH] fix #2192 - dont recycle frozen objects (#2193)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Summary:
This is one possible fix for #2192, which simply checks if objects are frozen before attempting to modify them.

Do note that while this fixes my use case and any others that are serialized as simple JSON-compatible types, this won't fix cases where a custom scalar is [hydrated to a more complex type](https://github.com/facebook/relay/issues/91), such as a `Date` object. For this, you may want to consider adding a check to skip the recycle if the inputs aren't simple objects or arrays:

```es6
if (
  Object.getPrototypeOf(nextData) !== Object.getPrototypeOf(prevData) ||
  (
    Object.getPrototypeOf(nextData) !== Object.prototype &&
    Object.getPrototypeOf(nextData) !== Array.prototype
  )
) {
  return nextData;
}
```
But, since this may have more consequences, and I would really like my use case fixed ASAP 😜 I've omitted that "fix" from this PR. Please let me know if you'd like me to open a separate issue for that.
Pull Request resolved: https://github.com/facebook/relay/pull/2193

Reviewed By: josephsavona

Differential Revision: D6845724

Pulled By: jstejada

fbshipit-source-id: 2a96bb45b349334db51304bac393b7539ee00448
---
 .../util/__tests__/recycleNodesInto-test.js   | 31 ++++++++++++++++++-
 .../relay-runtime/util/recycleNodesInto.js    | 20 +++++++++---
 2 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/packages/relay-runtime/util/__tests__/recycleNodesInto-test.js b/packages/relay-runtime/util/__tests__/recycleNodesInto-test.js
index ab9cbf34da8d9..78cafd740868e 100644
--- a/packages/relay-runtime/util/__tests__/recycleNodesInto-test.js
+++ b/packages/relay-runtime/util/__tests__/recycleNodesInto-test.js
@@ -84,7 +84,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', () => {
@@ -105,6 +105,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};
@@ -135,6 +157,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
index 24a562ffee410..46c5af88719c9 100644
--- a/packages/relay-runtime/util/recycleNodesInto.js
+++ b/packages/relay-runtime/util/recycleNodesInto.js
@@ -34,9 +34,15 @@ function recycleNodesInto<T>(prevData: T, nextData: T): T {
         const prevValue = prevArray[ii];
         const nextValue = recycleNodesInto(prevValue, nextItem);
         if (nextValue !== nextArray[ii]) {
-          nextArray[ii] = nextValue;
+          if (__DEV__) {
+            if (!Object.isFrozen(nextArray)) {
+              nextArray[ii] = nextValue;
+            }
+          } else {
+            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.
@@ -49,9 +55,15 @@ function recycleNodesInto<T>(prevData: T, nextData: T): T {
         const prevValue = prevObject[key];
         const nextValue = recycleNodesInto(prevValue, nextObject[key]);
         if (nextValue !== nextObject[key]) {
-          nextObject[key] = nextValue;
+          if (__DEV__) {
+            if (!Object.isFrozen(nextObject)) {
+              nextObject[key] = nextValue;
+            }
+          } else {
+            nextObject[key] = nextValue;
+          }
         }
-        return wasEqual && nextObject[key] === prevObject[key];
+        return wasEqual && nextValue === prevObject[key];
       }, true) && prevKeys.length === nextKeys.length;
   }
   return canRecycle ? prevData : nextData;