Warn if using React.unmountComponentAtNode on a different React instance's tree. (#7456)
* Warn when using React.unmountComponentAtNode on a different React instance's tree https://github.com/facebook/react/issues/3787 * Adding tests. * Implementing recommended changes. https://github.com/facebook/react/issues/3787
ventuno committed
Aug 17, 2016 at 17:20 UTC
a9e681a82818bc92a512383caf060b4114a9eb97
2 files changed
+78
-15
src/renderers/dom/client/ReactMount.js
+50
-15
@@ -207,6 +207,45 @@ function hasNonRootReactChild(container) {
207
}
208
}
209
210
+/**
211
+ * True if the supplied DOM node is a React DOM element and
212
+ * it has been rendered by another copy of React.
213
+ *
214
+ * @param {?DOMElement} node The candidate DOM node.
215
+ * @return {boolean} True if the DOM has been rendered by another copy of React
216
+ * @internal
217
+ */
218
+function nodeIsRenderedByOtherInstance(container) {
219
+ var rootEl = getReactRootElementInContainer(container);
220
+ return !!(rootEl && isReactNode(rootEl) && !ReactDOMComponentTree.getInstanceFromNode(rootEl));
221
+}
222
+
223
+/**
224
+ * True if the supplied DOM node is a valid node element.
225
+ *
226
+ * @param {?DOMElement} node The candidate DOM node.
227
+ * @return {boolean} True if the DOM is a valid DOM node.
228
+ * @internal
229
+ */
230
+function isValidContainer(node) {
231
+ return !!(node && (
232
+ node.nodeType === ELEMENT_NODE_TYPE ||
233
+ node.nodeType === DOC_NODE_TYPE ||
234
+ node.nodeType === DOCUMENT_FRAGMENT_NODE_TYPE
235
+ ));
236
+}
237
+
238
+/**
239
+ * True if the supplied DOM node is a valid React node element.
240
+ *
241
+ * @param {?DOMElement} node The candidate DOM node.
242
+ * @return {boolean} True if the DOM is a valid React DOM node.
243
+ * @internal
244
+ */
245
+function isReactNode(node) {
246
+ return isValidContainer(node) && (node.hasAttribute(ROOT_ATTR_NAME) || node.hasAttribute(ATTR_NAME));
247
+}
248
+
249
function getHostRootInstanceInContainer(container) {
250
var rootEl = getReactRootElementInContainer(container);
251
var prevHostInstance =
@@ -330,11 +369,7 @@ var ReactMount = {
369
);
370
371
invariant(
333
- container && (
334
- container.nodeType === ELEMENT_NODE_TYPE ||
335
- container.nodeType === DOC_NODE_TYPE ||
336
- container.nodeType === DOCUMENT_FRAGMENT_NODE_TYPE
337
- ),
372
+ isValidContainer(container),
373
'_registerComponent(...): Target container is not a DOM element.'
374
);
375
@@ -540,14 +575,18 @@ var ReactMount = {
575
);
576
577
invariant(
543
- container && (
544
- container.nodeType === ELEMENT_NODE_TYPE ||
545
- container.nodeType === DOC_NODE_TYPE ||
546
- container.nodeType === DOCUMENT_FRAGMENT_NODE_TYPE
547
- ),
578
+ isValidContainer(container),
579
'unmountComponentAtNode(...): Target container is not a DOM element.'
580
);
581
582
+ if (__DEV__) {
583
+ warning(
584
+ !nodeIsRenderedByOtherInstance(container),
585
+ 'unmountComponentAtNode(): The node you\'re attempting to unmount ' +
586
+ 'was rendered by another copy of React.'
587
+ );
588
+ }
589
+
590
var prevComponent = getTopLevelWrapperInContainer(container);
591
if (!prevComponent) {
592
// Check if the node being unmounted was rendered by React, but isn't a
@@ -593,11 +632,7 @@ var ReactMount = {
632
transaction
633
) {
634
invariant(
596
- container && (
597
- container.nodeType === ELEMENT_NODE_TYPE ||
598
- container.nodeType === DOC_NODE_TYPE ||
599
- container.nodeType === DOCUMENT_FRAGMENT_NODE_TYPE
600
- ),
635
+ isValidContainer(container),
636
'mountComponentIntoNode(...): Target container is not valid.'
637
);
638
src/renderers/dom/client/__tests__/ReactMount-test.js
+28
@@ -229,6 +229,34 @@ describe('ReactMount', function() {
229
);
230
});
231
232
+ it('should warn if the unmounted node was rendered by another copy of React', function() {
233
+ jest.resetModuleRegistry();
234
+ var ReactDOMOther = require('ReactDOM');
235
+ var container = document.createElement('div');
236
+
237
+ class Component extends React.Component {
238
+ render() {
239
+ return <div><div /></div>;
240
+ }
241
+ }
242
+
243
+ ReactDOM.render(<Component />, container);
244
+ // Make sure ReactDOM and ReactDOMOther are different copies
245
+ expect(ReactDOM).not.toEqual(ReactDOMOther);
246
+
247
+ spyOn(console, 'error');
248
+ ReactDOMOther.unmountComponentAtNode(container);
249
+ expect(console.error.calls.count()).toBe(1);
250
+ expect(console.error.calls.argsFor(0)[0]).toBe(
251
+ 'Warning: unmountComponentAtNode(): The node you\'re attempting to unmount ' +
252
+ 'was rendered by another copy of React.'
253
+ );
254
+
255
+ // Don't throw a warning if the correct React copy unmounts the node
256
+ ReactDOM.unmountComponentAtNode(container);
257
+ expect(console.error.calls.count()).toBe(1);
258
+ });
259
+
260
it('passes the correct callback context', function() {
261
var container = document.createElement('div');
262
var calls = 0;