Warn when passing invalid containers to render and unmountComponentAtNode
Charles Marsh committed
Aug 13, 2014 at 16:03 UTC
270a805369c7aa488ea0bc0604d860da465eb561
3 files changed
+117
-10
src/renderers/dom/client/ReactMount.js
+50
-2
@@ -587,8 +587,16 @@ var ReactMount = {
587
var reactRootElement = getReactRootElementInContainer(container);
588
var containerHasReactMarkup =
589
reactRootElement && ReactMount.isRenderedByReact(reactRootElement);
590
+ var containerHasNonRootReactChild =
591
+ ReactMount.hasNonRootReactChild(container);
592
593
if (__DEV__) {
594
+ warning(
595
+ !containerHasNonRootReactChild,
596
+ 'renderComponent(...): Replacing React-rendered children with a new ' +
597
+ 'root component.'
598
+ );
599
+
600
if (!containerHasReactMarkup || reactRootElement.nextSibling) {
601
var rootElementSibling = reactRootElement;
602
while (rootElementSibling) {
@@ -601,13 +609,15 @@ var ReactMount = {
609
);
610
break;
611
}
604
-
612
rootElementSibling = rootElementSibling.nextSibling;
613
}
614
}
615
}
616
610
- var shouldReuseMarkup = containerHasReactMarkup && !prevComponent;
617
+ var shouldReuseMarkup =
618
+ containerHasReactMarkup &&
619
+ !prevComponent &&
620
+ !containerHasNonRootReactChild;
621
var component = ReactMount._renderNewRootComponent(
622
nextWrappedElement,
623
container,
@@ -697,6 +707,28 @@ var ReactMount = {
707
var reactRootID = getReactRootID(container);
708
var component = instancesByReactRootID[reactRootID];
709
if (!component) {
710
+ // Check if the node being unmounted was rendered by React, but isn't a
711
+ // root node.
712
+ var containerHasNonRootReactChild = ReactMount.hasNonRootReactChild(
713
+ container);
714
+
715
+ // Check if the container itself is a React root node.
716
+ var containerID = ReactMount.getID(container);
717
+ var containerRootID = ReactInstanceHandles.getReactRootIDFromNodeID(
718
+ containerID);
719
+ var isContainerReactRoot =
720
+ containerID && containerRootID && containerID === containerRootID;
721
+
722
+ if (__DEV__) {
723
+ warning(
724
+ !containerHasNonRootReactChild,
725
+ 'unmountComponentAtNode(): The node you\'re attempting to unmount ' +
726
+ 'is not a valid React root node, and thus cannot be unmounted.%s',
727
+ (isContainerReactRoot ? ' You may have passed in a React root ' +
728
+ 'node as argument, rather than its container.' : '')
729
+ );
730
+ }
731
+
732
return false;
733
}
734
ReactUpdates.batchedUpdates(
@@ -781,6 +813,22 @@ var ReactMount = {
813
return id ? id.charAt(0) === SEPARATOR : false;
814
},
815
816
+ /**
817
+ * True if the supplied DOM node has a direct React-rendered child that is
818
+ * not a React root element. Useful for warning in renderComponent`,
819
+ * `unmountComponentAtNode`, etc.
820
+ *
821
+ * @param {?DOMElement} node The candidate DOM node.
822
+ * @return {boolean} True if the DOM element contains a direct child that was
823
+ * rendered by React but is not a root element.
824
+ * @internal
825
+ */
826
+ hasNonRootReactChild: function(node) {
827
+ var reactRootID = getReactRootID(node);
828
+ return reactRootID ? reactRootID !==
829
+ ReactInstanceHandles.getReactRootIDFromNodeID(reactRootID) : false;
830
+ },
831
+
832
/**
833
* Traverses up the ancestors of the supplied node to find a node that is a
834
* DOM representation of a React component.
src/renderers/dom/client/__tests__/ReactMount-test.js
+20
@@ -224,4 +224,24 @@ describe('ReactMount', function() {
224
expect(console.error.mock.calls.length).toBe(1);
225
expect(console.error.mock.calls[0][0]).toContain('two copies of React');
226
});
227
+
228
+ it('should warn if render removes React-rendered children', function() {
229
+ var container = document.createElement('container');
230
+ var Component = React.createClass({
231
+ render: function() {
232
+ return <div><div /></div>;
233
+ },
234
+ });
235
+ React.render(<Component />, container);
236
+
237
+ // Test that blasting away children throws a warning
238
+ spyOn(console, 'error');
239
+ var rootNode = container.firstChild;
240
+ React.render(<span />, rootNode);
241
+ expect(console.error.callCount).toBe(1);
242
+ expect(console.error.mostRecentCall.args[0]).toBe(
243
+ 'Warning: renderComponent(...): Replacing React-rendered children ' +
244
+ 'with a new root component.'
245
+ );
246
+ });
247
});
src/renderers/dom/client/__tests__/ReactMountDestruction-test.js
+47
-8
@@ -19,18 +19,12 @@ describe('ReactMount', function() {
19
var mainContainerDiv = document.createElement('div');
20
document.body.appendChild(mainContainerDiv);
21
22
- var instanceOne = (
23
- <div className="firstReactDiv">
24
- </div>
25
- );
22
+ var instanceOne = <div className="firstReactDiv" />;
23
var firstRootDiv = document.createElement('div');
24
mainContainerDiv.appendChild(firstRootDiv);
25
ReactDOM.render(instanceOne, firstRootDiv);
26
30
- var instanceTwo = (
31
- <div className="secondReactDiv">
32
- </div>
33
- );
27
+ var instanceTwo = <div className="secondReactDiv" />;
28
var secondRootDiv = document.createElement('div');
29
mainContainerDiv.appendChild(secondRootDiv);
30
ReactDOM.render(instanceTwo, secondRootDiv);
@@ -45,4 +39,49 @@ describe('ReactMount', function() {
39
ReactDOM.unmountComponentAtNode(secondRootDiv);
40
expect(secondRootDiv.firstChild).toBeNull();
41
});
42
+
43
+ it('should warn when unmounting a non-container root node', function() {
44
+ var mainContainerDiv = document.createElement('div');
45
+
46
+ var component =
47
+ <div>
48
+ <div />
49
+ </div>;
50
+ ReactDOM.render(component, mainContainerDiv);
51
+
52
+ // Test that unmounting at a root node gives a helpful warning
53
+ var rootDiv = mainContainerDiv.firstChild;
54
+ spyOn(console, 'error');
55
+ ReactDOM.unmountComponentAtNode(rootDiv);
56
+ expect(console.error.callCount).toBe(1);
57
+ expect(console.error.mostRecentCall.args[0]).toBe(
58
+ 'Warning: unmountComponentAtNode(): The node you\'re attempting to ' +
59
+ 'unmount is not a valid React root node, and thus cannot be ' +
60
+ 'unmounted. You may have passed in a React root node as argument, ' +
61
+ 'rather than its container.'
62
+ );
63
+ });
64
+
65
+ it('should warn when unmounting a non-container, non-root node', function() {
66
+ var mainContainerDiv = document.createElement('div');
67
+
68
+ var component =
69
+ <div>
70
+ <div>
71
+ <div />
72
+ </div>
73
+ </div>;
74
+ ReactDOM.render(component, mainContainerDiv);
75
+
76
+ // Test that unmounting at a non-root node gives a different warning
77
+ var nonRootDiv = mainContainerDiv.firstChild.firstChild;
78
+ spyOn(console, 'error');
79
+ ReactDOM.unmountComponentAtNode(nonRootDiv);
80
+ expect(console.error.callCount).toBe(1);
81
+ expect(console.error.mostRecentCall.args[0]).toBe(
82
+ 'Warning: unmountComponentAtNode(): The node you\'re attempting to ' +
83
+ 'unmount is not a valid React root node, and thus cannot be ' +
84
+ 'unmounted.'
85
+ );
86
+ });
87
});