Don't crash in event handling when mixing React copies
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.
Ben Alpert committed
Aug 31, 2015 at 15:54 UTC
e7ce46ff28554d98a653b26c0e9a5fdfb63eea89
6 files changed
+77
-47
src/renderers/dom/client/ReactEventListener.js
+4
-2
@@ -112,11 +112,13 @@ function handleTopLevelWithPath(bookKeeping) {
112
var eventsFired = 0;
113
for (var i = 0; i < path.length; i++) {
114
var currentPathElement = path[i];
115
- var currentPathElementID = ReactMount.getID(currentPathElement);
115
if (currentPathElement.nodeType === DOCUMENT_FRAGMENT_NODE_TYPE) {
116
currentNativeTarget = path[i + 1];
117
}
119
- if (ReactMount.isRenderedByReact(currentPathElement)) {
118
+ // TODO: slow
119
+ var reactParent = ReactMount.getFirstReactDOM(currentPathElement);
120
+ if (reactParent === currentPathElement) {
121
+ var currentPathElementID = ReactMount.getID(currentPathElement);
122
var newRootID = ReactInstanceHandles.getReactRootIDFromNodeID(
123
currentPathElementID
124
);
src/renderers/dom/client/ReactMount.js
+46
-29
@@ -35,8 +35,6 @@ var shouldUpdateReactComponent = require('shouldUpdateReactComponent');
35
var validateDOMNesting = require('validateDOMNesting');
36
var warning = require('warning');
37
38
-var SEPARATOR = ReactInstanceHandles.SEPARATOR;
39
-
38
var ATTR_NAME = DOMProperty.ID_ATTRIBUTE_NAME;
39
var nodeCache = {};
40
@@ -367,6 +365,48 @@ function hasNonRootReactChild(node) {
365
ReactInstanceHandles.getReactRootIDFromNodeID(reactRootID) : false;
366
}
367
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
+
410
/**
411
* Temporary (?) hack so that we can store all top-level pending updates on
412
* composites instead of having to worry about different types of components
@@ -602,7 +642,7 @@ var ReactMount = {
642
643
var reactRootElement = getReactRootElementInContainer(container);
644
var containerHasReactMarkup =
605
- reactRootElement && ReactMount.isRenderedByReact(reactRootElement);
645
+ reactRootElement && !!internalGetID(reactRootElement);
646
var containerHasNonRootReactChild = hasNonRootReactChild(container);
647
648
if (__DEV__) {
@@ -617,7 +657,7 @@ var ReactMount = {
657
if (!containerHasReactMarkup || reactRootElement.nextSibling) {
658
var rootElementSibling = reactRootElement;
659
while (rootElementSibling) {
620
- if (ReactMount.isRenderedByReact(rootElementSibling)) {
660
+ if (internalGetID(rootElementSibling)) {
661
warning(
662
false,
663
'render(): Target node has markup rendered by React, but there ' +
@@ -818,39 +858,16 @@ var ReactMount = {
858
return ReactMount.findComponentRoot(reactRoot, id);
859
},
860
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
-
861
/**
862
* 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.
864
*
865
* @param {*} node
866
* @return {?DOMEventTarget}
867
* @internal
868
*/
869
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);
871
},
872
873
/**
src/renderers/dom/client/__tests__/ReactBrowserEventEmitter-test.js
+10
-1
@@ -21,7 +21,8 @@ var setID = function(el, id) {
21
ReactMount.setID(el, id);
22
idToNode[id] = el;
23
};
24
-var oldGetNode = ReactMount.getNode;
24
+var oldGetNode;
25
+var oldGetFirstReactDOM;
26
27
var EventPluginHub;
28
var ReactBrowserEventEmitter;
@@ -83,9 +84,16 @@ describe('ReactBrowserEventEmitter', function() {
84
EventListener = require('EventListener');
85
ReactBrowserEventEmitter = require('ReactBrowserEventEmitter');
86
ReactTestUtils = require('ReactTestUtils');
87
+
88
+ oldGetNode = ReactMount.getNode;
89
+ oldGetFirstReactDOM = ReactMount.oldGetFirstReactDOM;
90
ReactMount.getNode = function(id) {
91
return idToNode[id];
92
};
93
+ ReactMount.getFirstReactDOM = function(node) {
94
+ return node;
95
+ };
96
+
97
idCallOrder = [];
98
tapMoveThreshold = TapEventPlugin.tapMoveThreshold;
99
EventPluginHub.injection.injectEventPluginsByName({
@@ -95,6 +103,7 @@ describe('ReactBrowserEventEmitter', function() {
103
104
afterEach(function() {
105
ReactMount.getNode = oldGetNode;
106
+ ReactMount.getFirstReactDOM = oldGetFirstReactDOM;
107
});
108
109
it('should store a listener correctly', function() {
src/renderers/dom/client/eventPlugins/EnterLeaveEventPlugin.js
+13
-7
@@ -89,15 +89,24 @@ var EnterLeaveEventPlugin = {
89
}
90
}
91
92
- var from, to;
92
+ var from;
93
+ var to;
94
+ var fromID = '';
95
+ var toID = '';
96
if (topLevelType === topLevelTypes.topMouseOut) {
97
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;
106
} else {
107
from = win;
108
to = topLevelTarget;
109
+ toID = topLevelTargetID;
110
}
111
112
if (from === to) {
@@ -105,9 +114,6 @@ var EnterLeaveEventPlugin = {
114
return null;
115
}
116
108
- var fromID = from ? ReactMount.getID(from) : '';
109
- var toID = to ? ReactMount.getID(to) : '';
110
-
117
var leave = SyntheticMouseEvent.getPooled(
118
eventTypes.mouseLeave,
119
fromID,
src/renderers/dom/client/wrappers/ReactDOMInput.js
+4
@@ -141,6 +141,10 @@ function _handleChange(event) {
141
otherNode.form !== rootNode.form) {
142
continue;
143
}
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.
148
var otherID = ReactMount.getID(otherNode);
149
invariant(
150
otherID,
src/renderers/shared/reconciler/__tests__/ReactInstanceHandles-test.js
-8
@@ -74,14 +74,6 @@ describe('ReactInstanceHandles', function() {
74
aggregatedArgs = [];
75
});
76
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
-
77
describe('findComponentRoot', function() {
78
it('should find the correct node with prefix sibling IDs', function() {
79
var parentNode = document.createElement('div');