Skip to content

Commit

Permalink
Don't crash in event handling when mixing React copies
Browse files Browse the repository at this point in the history
Should fix #1939.

Test Plan:
With two copies of React, render a div using React1 and use that as a container to render a div with React2. Add onMouseEnter/onMouseLeave to both divs that log. Mouse around and see correct logs (as if each React was isolated), no errors.
  • Loading branch information
sophiebits committed Sep 9, 2015
1 parent 10ab0c8 commit 0af75f4
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 46 deletions.
6 changes: 4 additions & 2 deletions src/renderers/dom/client/ReactEventListener.js
Expand Up @@ -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
);
Expand Down
73 changes: 45 additions & 28 deletions src/renderers/dom/client/ReactMount.js
Expand Up @@ -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 = {};

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -602,7 +642,7 @@ var ReactMount = {

var reactRootElement = getReactRootElementInContainer(container);
var containerHasReactMarkup =
reactRootElement && ReactMount.isRenderedByReact(reactRootElement);
reactRootElement && !!internalGetID(reactRootElement);
var containerHasNonRootReactChild = hasNonRootReactChild(container);

if (__DEV__) {
Expand All @@ -617,7 +657,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 ' +
Expand Down Expand Up @@ -818,22 +858,6 @@ 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.
Expand All @@ -843,14 +867,7 @@ var ReactMount = {
* @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);
},

/**
Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -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({
Expand All @@ -95,6 +103,7 @@ describe('ReactBrowserEventEmitter', function() {

afterEach(function() {
ReactMount.getNode = oldGetNode;
ReactMount.getFirstReactDOM = oldGetFirstReactDOM;
});

it('should store a listener correctly', function() {
Expand Down
20 changes: 13 additions & 7 deletions src/renderers/dom/client/eventPlugins/EnterLeaveEventPlugin.js
Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions src/renderers/dom/client/wrappers/ReactDOMInput.js
Expand Up @@ -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,
Expand Down
Expand Up @@ -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');
Expand Down

0 comments on commit 0af75f4

Please sign in to comment.