@samitouri / QOS-React / commits / 52a2365f19

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.

Sebastian Markbåge committed Jun 27, 2017 at 17:15 UTC 52a2365f19a71666e3131e03ace56ef64d9a6341
3 files changed +44 -6
scripts/fiber/tests-passing-except-dev.txt
-4
@@ -150,9 +150,5 @@ src/renderers/dom/shared/__tests__/ReactMount-test.js
150 * should warn if mounting into dirty rendered markup
151 * should account for escaping on a checksum mismatch
152
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 -
153 src/renderers/dom/shared/__tests__/ReactServerRendering-test.js
154 * should have the correct mounting behavior
scripts/fiber/tests-passing.txt
+2
@@ -1436,6 +1436,8 @@ src/renderers/dom/shared/__tests__/ReactMount-test.js
1436
1437 src/renderers/dom/shared/__tests__/ReactMountDestruction-test.js
1438 * 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
1441
1442 src/renderers/dom/shared/__tests__/ReactRenderDocument-test.js
1443 * should be able to adopt server markup
src/renderers/dom/fiber/ReactDOMFiberEntry.js
+42 -2
@@ -499,8 +499,25 @@ function renderSubtreeIntoContainer(
499 // First clear any existing content.
500 // TODO: Figure out the best heuristic here.
501 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);
521 }
522 }
523 const newRoot = DOMRenderer.createContainer(container);
@@ -602,6 +619,29 @@ var ReactDOMFiber = {
619 // get `true` twice. That's probably fine?
620 return true;
621 } 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 +
645 return false;
646 }
647 },