Skip to content

Commit 52a2365

Browse files
authored
Fix mount and unmount warnings in Fiber (#10056)
* Warn if unmounting a non-container * Warn if the 2nd+ child is a "root" element but not first This triggers our non-reuse mode. This is covered by ReactMount already but the test doesn't pass yet without also landing #10026.
1 parent f8a3a39 commit 52a2365

3 files changed

Lines changed: 44 additions & 6 deletions

File tree

scripts/fiber/tests-passing-except-dev.txt

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -150,9 +150,5 @@ src/renderers/dom/shared/__tests__/ReactMount-test.js
150150
* should warn if mounting into dirty rendered markup
151151
* should account for escaping on a checksum mismatch
152152

153-
src/renderers/dom/shared/__tests__/ReactMountDestruction-test.js
154-
* should warn when unmounting a non-container root node
155-
* should warn when unmounting a non-container, non-root node
156-
157153
src/renderers/dom/shared/__tests__/ReactServerRendering-test.js
158154
* should have the correct mounting behavior

scripts/fiber/tests-passing.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1436,6 +1436,8 @@ src/renderers/dom/shared/__tests__/ReactMount-test.js
14361436

14371437
src/renderers/dom/shared/__tests__/ReactMountDestruction-test.js
14381438
* should destroy a react root upon request
1439+
* should warn when unmounting a non-container root node
1440+
* should warn when unmounting a non-container, non-root node
14391441

14401442
src/renderers/dom/shared/__tests__/ReactRenderDocument-test.js
14411443
* should be able to adopt server markup

src/renderers/dom/fiber/ReactDOMFiberEntry.js

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -499,8 +499,25 @@ function renderSubtreeIntoContainer(
499499
// First clear any existing content.
500500
// TODO: Figure out the best heuristic here.
501501
if (!shouldReuseContent(container)) {
502-
while (container.lastChild) {
503-
container.removeChild(container.lastChild);
502+
let warned = false;
503+
let rootSibling;
504+
while ((rootSibling = container.lastChild)) {
505+
if (__DEV__) {
506+
if (
507+
!warned &&
508+
rootSibling.nodeType === ELEMENT_NODE &&
509+
(rootSibling: any).hasAttribute(ID_ATTRIBUTE_NAME)
510+
) {
511+
warned = true;
512+
warning(
513+
false,
514+
'render(): Target node has markup rendered by React, but there ' +
515+
'are unrelated nodes as well. This is most commonly caused by ' +
516+
'white-space inserted around server-rendered markup.',
517+
);
518+
}
519+
}
520+
container.removeChild(rootSibling);
504521
}
505522
}
506523
const newRoot = DOMRenderer.createContainer(container);
@@ -602,6 +619,29 @@ var ReactDOMFiber = {
602619
// get `true` twice. That's probably fine?
603620
return true;
604621
} else {
622+
if (__DEV__) {
623+
const rootEl = getReactRootElementInContainer(container);
624+
const hasNonRootReactChild = !!(rootEl &&
625+
ReactDOMComponentTree.getInstanceFromNode(rootEl));
626+
627+
// Check if the container itself is a React root node.
628+
const isContainerReactRoot =
629+
container.nodeType === 1 &&
630+
isValidContainer(container.parentNode) &&
631+
!!container.parentNode._reactRootContainer;
632+
633+
warning(
634+
!hasNonRootReactChild,
635+
"unmountComponentAtNode(): The node you're attempting to unmount " +
636+
'was rendered by React and is not a top-level container. %s',
637+
isContainerReactRoot
638+
? 'You may have accidentally passed in a React root node instead ' +
639+
'of its container.'
640+
: 'Instead, have the parent component update its state and ' +
641+
'rerender in order to remove this component.',
642+
);
643+
}
644+
605645
return false;
606646
}
607647
},

0 commit comments

Comments
 (0)