From e89e6755082f6b690ea4aaf280716a9fc2e9e9b0 Mon Sep 17 00:00:00 2001
From: Ben Alpert <balpert@fb.com>
Date: Tue, 13 Oct 2015 15:04:32 -0700
Subject: [PATCH] Be more lenient with invalid nodes in the cache

This matches our old behavior. I was a little too aggressive in turning on this
error and it's still possible to trigger it (using the test added here).
---
 src/renderers/dom/client/ReactMount.js        | 24 ++++++++----------
 .../dom/client/__tests__/ReactMount-test.js   | 25 +++++++++++++++++++
 2 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/src/renderers/dom/client/ReactMount.js b/src/renderers/dom/client/ReactMount.js
index 676f486e74..aca81fa2b0 100644
--- a/src/renderers/dom/client/ReactMount.js
+++ b/src/renderers/dom/client/ReactMount.js
@@ -111,9 +111,12 @@ function getReactRootID(container) {
 function getID(node) {
   var id = internalGetID(node);
   if (id) {
+    var cached = nodeCache[id];
+    // TODO: Fix this whole concept of "validity" -- the cache just shouldn't
+    // have nodes that have been unmounted.
     invariant(
-      !nodeCache.hasOwnProperty(id) || nodeCache[id] === node,
-      'ReactMount: Two unequal nodes with the same `%s`: %s',
+      !cached || cached === node || !isValid(cached, id),
+      'ReactMount: Two valid but unequal nodes with the same `%s`: %s',
       ATTR_NAME, id
     );
     nodeCache[id] = node;
@@ -149,18 +152,11 @@ function setID(node, id) {
  */
 function getNodeIfCached(id) {
   var node = nodeCache[id];
-  // TODO: Since this "isValid" business is now just a sanity check, we can
-  // probably drop it with no consequences.
-  invariant(
-    !node || isValid(node, id),
-    'ReactMount: Cached node with `%s`: %s is missing from the document. ' +
-    'This probably means the DOM was unexpectedly mutated -- when removing ' +
-    'React-rendered children from the DOM, rerender without those children ' +
-    'or call ReactDOM.unmountComponentAtNode on the container to unmount an ' +
-    'entire subtree.',
-    ATTR_NAME, id
-  );
-  return node;
+  // TODO: Fix this whole concept of "validity" -- the cache just shouldn't have
+  // nodes that have been unmounted.
+  if (node && isValid(node, id)) {
+    return node;
+  }
 }
 
 /**
diff --git a/src/renderers/dom/client/__tests__/ReactMount-test.js b/src/renderers/dom/client/__tests__/ReactMount-test.js
index bec3a3bf11..6159752454 100644
--- a/src/renderers/dom/client/__tests__/ReactMount-test.js
+++ b/src/renderers/dom/client/__tests__/ReactMount-test.js
@@ -275,4 +275,29 @@ describe('ReactMount', function() {
 
     ReactDOM.unmountComponentAtNode(container);
   });
+
+  it('should not crash in node cache when unmounting, case 2', function() {
+    var A = React.createClass({
+      render: function() {
+        return <a key={this.props.innerKey}>{this.props.innerKey}</a>;
+      },
+    });
+    var Component = React.createClass({
+      render: function() {
+        return (
+          <b>
+            <i>{this.props.step === 1 && <q />}</i>
+            {this.props.step === 1 && <A innerKey={this.props.step} />}
+          </b>
+        );
+      },
+    });
+
+    var container = document.createElement('container');
+
+    ReactDOM.render(<Component step={1} />, container);
+    ReactDOM.render(<Component step={2} />, container);
+    ReactDOM.render(<Component step={1} />, container);
+    ReactMount.getID(container.querySelector('a'));
+  });
 });