Merge pull request #4747 from spicyj/gh-1939

Don't crash in event handling when mixing React copies

Author: sophiebits

src/renderers/dom/client/ReactEventListener.js

@@ -112,11 +112,13 @@ function handleTopLevelWithPath(bookKeeping) {
   var eventsFired = 0;
   for (var i = 0; i < path.length; i++) {
     var currentPathElement = path[i];
-    var currentPathElementID = ReactMount.getID(currentPathElement);
     if (currentPathElement.nodeType === DOCUMENT_FRAGMENT_NODE_TYPE) {
       currentNativeTarget = path[i + 1];
     }
-    if (ReactMount.isRenderedByReact(currentPathElement)) {
+    // TODO: slow
+    var reactParent = ReactMount.getFirstReactDOM(currentPathElement);
+    if (reactParent === currentPathElement) {
+      var currentPathElementID = ReactMount.getID(currentPathElement);
       var newRootID = ReactInstanceHandles.getReactRootIDFromNodeID(
         currentPathElementID
       );

src/renderers/dom/client/ReactMount.js

@@ -35,8 +35,6 @@ var shouldUpdateReactComponent = require('shouldUpdateReactComponent');
 var validateDOMNesting = require('validateDOMNesting');
 var warning = require('warning');
 
-var SEPARATOR = ReactInstanceHandles.SEPARATOR;
-
 var ATTR_NAME = DOMProperty.ID_ATTRIBUTE_NAME;
 var nodeCache = {};
 
@@ -367,6 +365,48 @@ function hasNonRootReactChild(node) {
     ReactInstanceHandles.getReactRootIDFromNodeID(reactRootID) : false;
 }
 
+/**
+ * Returns the first (deepest) ancestor of a node which is rendered by this copy
+ * of React.
+ */
+function findFirstReactDOMImpl(node) {
+  // This node might be from another React instance, so we make sure not to
+  // examine the node cache here
+  for (; node && node.parentNode !== node; node = node.parentNode) {
+    if (node.nodeType !== 1) {
+      // Not a DOMElement, therefore not a React component
+      continue;
+    }
+    var nodeID = internalGetID(node);
+    if (!nodeID) {
+      continue;
+    }
+    var reactRootID = ReactInstanceHandles.getReactRootIDFromNodeID(nodeID);
+
+    // If containersByReactRootID contains the container we find by crawling up
+    // the tree, we know that this instance of React rendered the node.
+    // nb. isValid's strategy (with containsNode) does not work because render
+    // trees may be nested and we don't want a false positive in that case.
+    var current = node;
+    var lastID;
+    do {
+      lastID = internalGetID(current);
+      current = current.parentNode;
+      invariant(
+        current != null,
+        'findFirstReactDOMImpl(...): Unexpected detached subtree found when ' +
+        'traversing DOM from node `%s`.',
+        nodeID
+      );
+    } while (lastID !== reactRootID);
+
+    if (current === containersByReactRootID[reactRootID]) {
+      return node;
+    }
+  }
+  return null;
+}
+
 /**
  * Temporary (?) hack so that we can store all top-level pending updates on
  * composites instead of having to worry about different types of components
@@ -606,7 +646,7 @@ var ReactMount = {
 
     var reactRootElement = getReactRootElementInContainer(container);
     var containerHasReactMarkup =
-      reactRootElement && ReactMount.isRenderedByReact(reactRootElement);
+      reactRootElement && !!internalGetID(reactRootElement);
     var containerHasNonRootReactChild = hasNonRootReactChild(container);
 
     if (__DEV__) {
@@ -621,7 +661,7 @@ var ReactMount = {
       if (!containerHasReactMarkup || reactRootElement.nextSibling) {
         var rootElementSibling = reactRootElement;
         while (rootElementSibling) {
-          if (ReactMount.isRenderedByReact(rootElementSibling)) {
+          if (internalGetID(rootElementSibling)) {
             warning(
               false,
               'render(): Target node has markup rendered by React, but there ' +
@@ -822,39 +862,16 @@ var ReactMount = {
     return ReactMount.findComponentRoot(reactRoot, id);
   },
 
-  /**
-   * True if the supplied `node` is rendered by React.
-   *
-   * @param {*} node DOM Element to check.
-   * @return {boolean} True if the DOM Element appears to be rendered by React.
-   * @internal
-   */
-  isRenderedByReact: function(node) {
-    if (node.nodeType !== 1) {
-      // Not a DOMElement, therefore not a React component
-      return false;
-    }
-    var id = ReactMount.getID(node);
-    return id ? id.charAt(0) === SEPARATOR : false;
-  },
-
   /**
    * Traverses up the ancestors of the supplied node to find a node that is a
-   * DOM representation of a React component.
+   * DOM representation of a React component rendered by this copy of React.
    *
    * @param {*} node
    * @return {?DOMEventTarget}
    * @internal
    */
   getFirstReactDOM: function(node) {
-    var current = node;
-    while (current && current.parentNode !== current) {
-      if (ReactMount.isRenderedByReact(current)) {
-        return current;
-      }
-      current = current.parentNode;
-    }
-    return null;
+    return findFirstReactDOMImpl(node);
   },
 
   /**

src/renderers/dom/client/__tests__/ReactBrowserEventEmitter-test.js

@@ -21,7 +21,8 @@ var setID = function(el, id) {
   ReactMount.setID(el, id);
   idToNode[id] = el;
 };
-var oldGetNode = ReactMount.getNode;
+var oldGetNode;
+var oldGetFirstReactDOM;
 
 var EventPluginHub;
 var ReactBrowserEventEmitter;
@@ -83,9 +84,16 @@ describe('ReactBrowserEventEmitter', function() {
     EventListener = require('EventListener');
     ReactBrowserEventEmitter = require('ReactBrowserEventEmitter');
     ReactTestUtils = require('ReactTestUtils');
+
+    oldGetNode = ReactMount.getNode;
+    oldGetFirstReactDOM = ReactMount.oldGetFirstReactDOM;
     ReactMount.getNode = function(id) {
       return idToNode[id];
     };
+    ReactMount.getFirstReactDOM = function(node) {
+      return node;
+    };
+
     idCallOrder = [];
     tapMoveThreshold = TapEventPlugin.tapMoveThreshold;
     EventPluginHub.injection.injectEventPluginsByName({
@@ -95,6 +103,7 @@ describe('ReactBrowserEventEmitter', function() {
 
   afterEach(function() {
     ReactMount.getNode = oldGetNode;
+    ReactMount.getFirstReactDOM = oldGetFirstReactDOM;
   });
 
   it('should store a listener correctly', function() {

src/renderers/dom/client/eventPlugins/EnterLeaveEventPlugin.js

@@ -89,25 +89,31 @@ var EnterLeaveEventPlugin = {
       }
     }
 
-    var from, to;
+    var from;
+    var to;
+    var fromID = '';
+    var toID = '';
     if (topLevelType === topLevelTypes.topMouseOut) {
       from = topLevelTarget;
-      to =
-        getFirstReactDOM(nativeEvent.relatedTarget || nativeEvent.toElement) ||
-        win;
+      fromID = topLevelTargetID;
+      to = getFirstReactDOM(nativeEvent.relatedTarget || nativeEvent.toElement);
+      if (to) {
+        toID = ReactMount.getID(to);
+      } else {
+        to = win;
+      }
+      to = to || win;
     } else {
       from = win;
       to = topLevelTarget;
+      toID = topLevelTargetID;
     }
 
     if (from === to) {
       // Nothing pertains to our managed components.
       return null;
     }
 
-    var fromID = from ? ReactMount.getID(from) : '';
-    var toID = to ? ReactMount.getID(to) : '';
-
     var leave = SyntheticMouseEvent.getPooled(
       eventTypes.mouseLeave,
       fromID,

src/renderers/dom/client/wrappers/ReactDOMInput.js

@@ -141,6 +141,10 @@ function _handleChange(event) {
           otherNode.form !== rootNode.form) {
         continue;
       }
+      // This will throw if radio buttons rendered by different copies of React
+      // and the same name are rendered into the same form (same as #1939).
+      // That's probably okay; we don't support it just as we don't support
+      // mixing React with non-React.
       var otherID = ReactMount.getID(otherNode);
       invariant(
         otherID,

src/renderers/shared/reconciler/__tests__/ReactInstanceHandles-test.js

@@ -74,14 +74,6 @@ describe('ReactInstanceHandles', function() {
     aggregatedArgs = [];
   });
 
-  describe('isRenderedByReact', function() {
-    it('should not crash on text nodes', function() {
-      expect(function() {
-        ReactMount.isRenderedByReact(document.createTextNode('yolo'));
-      }).not.toThrow();
-    });
-  });
-
   describe('findComponentRoot', function() {
     it('should find the correct node with prefix sibling IDs', function() {
       var parentNode = document.createElement('div');