Skip to content

Commit e7ce46f

Browse files
committed
Don't crash in event handling when mixing React copies
Should fix facebook#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.
1 parent 10ab0c8 commit e7ce46f

File tree

6 files changed

+77
-47
lines changed

6 files changed

+77
-47
lines changed

src/renderers/dom/client/ReactEventListener.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,11 +112,13 @@ function handleTopLevelWithPath(bookKeeping) {
112112
var eventsFired = 0;
113113
for (var i = 0; i < path.length; i++) {
114114
var currentPathElement = path[i];
115-
var currentPathElementID = ReactMount.getID(currentPathElement);
116115
if (currentPathElement.nodeType === DOCUMENT_FRAGMENT_NODE_TYPE) {
117116
currentNativeTarget = path[i + 1];
118117
}
119-
if (ReactMount.isRenderedByReact(currentPathElement)) {
118+
// TODO: slow
119+
var reactParent = ReactMount.getFirstReactDOM(currentPathElement);
120+
if (reactParent === currentPathElement) {
121+
var currentPathElementID = ReactMount.getID(currentPathElement);
120122
var newRootID = ReactInstanceHandles.getReactRootIDFromNodeID(
121123
currentPathElementID
122124
);

src/renderers/dom/client/ReactMount.js

Lines changed: 46 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,6 @@ var shouldUpdateReactComponent = require('shouldUpdateReactComponent');
3535
var validateDOMNesting = require('validateDOMNesting');
3636
var warning = require('warning');
3737

38-
var SEPARATOR = ReactInstanceHandles.SEPARATOR;
39-
4038
var ATTR_NAME = DOMProperty.ID_ATTRIBUTE_NAME;
4139
var nodeCache = {};
4240

@@ -367,6 +365,48 @@ function hasNonRootReactChild(node) {
367365
ReactInstanceHandles.getReactRootIDFromNodeID(reactRootID) : false;
368366
}
369367

368+
/**
369+
* Returns the first (deepest) ancestor of a node which is rendered by this copy
370+
* of React.
371+
*/
372+
function findFirstReactDOMImpl(node) {
373+
// This node might be from another React instance, so we make sure not to
374+
// examine the node cache here
375+
for (; node && node.parentNode !== node; node = node.parentNode) {
376+
if (node.nodeType !== 1) {
377+
// Not a DOMElement, therefore not a React component
378+
continue;
379+
}
380+
var nodeID = internalGetID(node);
381+
if (!nodeID) {
382+
continue;
383+
}
384+
var reactRootID = ReactInstanceHandles.getReactRootIDFromNodeID(nodeID);
385+
386+
// If containersByReactRootID contains the container we find by crawling up
387+
// the tree, we know that this instance of React rendered the node.
388+
// nb. isValid's strategy (with containsNode) does not work because render
389+
// trees may be nested and we don't want a false positive in that case.
390+
var current = node;
391+
var lastID;
392+
do {
393+
lastID = internalGetID(current);
394+
current = current.parentNode;
395+
invariant(
396+
current != null,
397+
'findFirstReactDOMImpl(...): Unexpected detached subtree found when ' +
398+
'traversing DOM from node `%s`.',
399+
nodeID
400+
);
401+
} while (lastID !== reactRootID);
402+
403+
if (current === containersByReactRootID[reactRootID]) {
404+
return node;
405+
}
406+
}
407+
return null;
408+
}
409+
370410
/**
371411
* Temporary (?) hack so that we can store all top-level pending updates on
372412
* composites instead of having to worry about different types of components
@@ -602,7 +642,7 @@ var ReactMount = {
602642

603643
var reactRootElement = getReactRootElementInContainer(container);
604644
var containerHasReactMarkup =
605-
reactRootElement && ReactMount.isRenderedByReact(reactRootElement);
645+
reactRootElement && !!internalGetID(reactRootElement);
606646
var containerHasNonRootReactChild = hasNonRootReactChild(container);
607647

608648
if (__DEV__) {
@@ -617,7 +657,7 @@ var ReactMount = {
617657
if (!containerHasReactMarkup || reactRootElement.nextSibling) {
618658
var rootElementSibling = reactRootElement;
619659
while (rootElementSibling) {
620-
if (ReactMount.isRenderedByReact(rootElementSibling)) {
660+
if (internalGetID(rootElementSibling)) {
621661
warning(
622662
false,
623663
'render(): Target node has markup rendered by React, but there ' +
@@ -818,39 +858,16 @@ var ReactMount = {
818858
return ReactMount.findComponentRoot(reactRoot, id);
819859
},
820860

821-
/**
822-
* True if the supplied `node` is rendered by React.
823-
*
824-
* @param {*} node DOM Element to check.
825-
* @return {boolean} True if the DOM Element appears to be rendered by React.
826-
* @internal
827-
*/
828-
isRenderedByReact: function(node) {
829-
if (node.nodeType !== 1) {
830-
// Not a DOMElement, therefore not a React component
831-
return false;
832-
}
833-
var id = ReactMount.getID(node);
834-
return id ? id.charAt(0) === SEPARATOR : false;
835-
},
836-
837861
/**
838862
* Traverses up the ancestors of the supplied node to find a node that is a
839-
* DOM representation of a React component.
863+
* DOM representation of a React component rendered by this copy of React.
840864
*
841865
* @param {*} node
842866
* @return {?DOMEventTarget}
843867
* @internal
844868
*/
845869
getFirstReactDOM: function(node) {
846-
var current = node;
847-
while (current && current.parentNode !== current) {
848-
if (ReactMount.isRenderedByReact(current)) {
849-
return current;
850-
}
851-
current = current.parentNode;
852-
}
853-
return null;
870+
return findFirstReactDOMImpl(node);
854871
},
855872

856873
/**

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ var setID = function(el, id) {
2121
ReactMount.setID(el, id);
2222
idToNode[id] = el;
2323
};
24-
var oldGetNode = ReactMount.getNode;
24+
var oldGetNode;
25+
var oldGetFirstReactDOM;
2526

2627
var EventPluginHub;
2728
var ReactBrowserEventEmitter;
@@ -83,9 +84,16 @@ describe('ReactBrowserEventEmitter', function() {
8384
EventListener = require('EventListener');
8485
ReactBrowserEventEmitter = require('ReactBrowserEventEmitter');
8586
ReactTestUtils = require('ReactTestUtils');
87+
88+
oldGetNode = ReactMount.getNode;
89+
oldGetFirstReactDOM = ReactMount.oldGetFirstReactDOM;
8690
ReactMount.getNode = function(id) {
8791
return idToNode[id];
8892
};
93+
ReactMount.getFirstReactDOM = function(node) {
94+
return node;
95+
};
96+
8997
idCallOrder = [];
9098
tapMoveThreshold = TapEventPlugin.tapMoveThreshold;
9199
EventPluginHub.injection.injectEventPluginsByName({
@@ -95,6 +103,7 @@ describe('ReactBrowserEventEmitter', function() {
95103

96104
afterEach(function() {
97105
ReactMount.getNode = oldGetNode;
106+
ReactMount.getFirstReactDOM = oldGetFirstReactDOM;
98107
});
99108

100109
it('should store a listener correctly', function() {

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

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -89,25 +89,31 @@ var EnterLeaveEventPlugin = {
8989
}
9090
}
9191

92-
var from, to;
92+
var from;
93+
var to;
94+
var fromID = '';
95+
var toID = '';
9396
if (topLevelType === topLevelTypes.topMouseOut) {
9497
from = topLevelTarget;
95-
to =
96-
getFirstReactDOM(nativeEvent.relatedTarget || nativeEvent.toElement) ||
97-
win;
98+
fromID = topLevelTargetID;
99+
to = getFirstReactDOM(nativeEvent.relatedTarget || nativeEvent.toElement);
100+
if (to) {
101+
toID = ReactMount.getID(to);
102+
} else {
103+
to = win;
104+
}
105+
to = to || win;
98106
} else {
99107
from = win;
100108
to = topLevelTarget;
109+
toID = topLevelTargetID;
101110
}
102111

103112
if (from === to) {
104113
// Nothing pertains to our managed components.
105114
return null;
106115
}
107116

108-
var fromID = from ? ReactMount.getID(from) : '';
109-
var toID = to ? ReactMount.getID(to) : '';
110-
111117
var leave = SyntheticMouseEvent.getPooled(
112118
eventTypes.mouseLeave,
113119
fromID,

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,10 @@ function _handleChange(event) {
141141
otherNode.form !== rootNode.form) {
142142
continue;
143143
}
144+
// This will throw if radio buttons rendered by different copies of React
145+
// and the same name are rendered into the same form (same as #1939).
146+
// That's probably okay; we don't support it just as we don't support
147+
// mixing React with non-React.
144148
var otherID = ReactMount.getID(otherNode);
145149
invariant(
146150
otherID,

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

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -74,14 +74,6 @@ describe('ReactInstanceHandles', function() {
7474
aggregatedArgs = [];
7575
});
7676

77-
describe('isRenderedByReact', function() {
78-
it('should not crash on text nodes', function() {
79-
expect(function() {
80-
ReactMount.isRenderedByReact(document.createTextNode('yolo'));
81-
}).not.toThrow();
82-
});
83-
});
84-
8577
describe('findComponentRoot', function() {
8678
it('should find the correct node with prefix sibling IDs', function() {
8779
var parentNode = document.createElement('div');

0 commit comments

Comments
 (0)