diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index 06137eac049..bd380aefce6 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -18,8 +18,6 @@ src/renderers/dom/shared/__tests__/ReactDOMTextComponent-test.js * can reconcile text arbitrarily split into multiple nodes on some substitutions only src/renderers/dom/shared/__tests__/ReactMount-test.js -* throws when given a string -* throws when given a factory * tracks root instances * marks top-level mounts diff --git a/scripts/fiber/tests-passing-except-dev.txt b/scripts/fiber/tests-passing-except-dev.txt index decc5427129..90cc310b424 100644 --- a/scripts/fiber/tests-passing-except-dev.txt +++ b/scripts/fiber/tests-passing-except-dev.txt @@ -7,10 +7,7 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js src/renderers/dom/shared/__tests__/ReactMount-test.js * should warn if mounting into dirty rendered markup -* should warn when mounting into document.body * should account for escaping on a checksum mismatch -* should warn if render removes React-rendered children -* should warn if the unmounted node was rendered by another copy of React src/renderers/dom/shared/__tests__/ReactMountDestruction-test.js * should warn when unmounting a non-container root node diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 3b85124db56..0393d3c71e9 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -769,10 +769,15 @@ src/renderers/dom/shared/__tests__/ReactEventListener-test.js src/renderers/dom/shared/__tests__/ReactMount-test.js * throws when given a non-node +* throws when given a string +* throws when given a factory * should render different components in same root * should unmount and remount if the key changes * should reuse markup if rendering to the same target twice * should not warn if mounting into non-empty node +* should warn when mounting into document.body +* should warn if render removes React-rendered children +* should warn if the unmounted node was rendered by another copy of React * passes the correct callback context src/renderers/dom/shared/__tests__/ReactMountDestruction-test.js diff --git a/src/renderers/dom/fiber/ReactDOMFiber.js b/src/renderers/dom/fiber/ReactDOMFiber.js index 8708b20a0e4..cebe0414812 100644 --- a/src/renderers/dom/fiber/ReactDOMFiber.js +++ b/src/renderers/dom/fiber/ReactDOMFiber.js @@ -108,6 +108,18 @@ function validateContainer(container) { } } +function getReactRootElementInContainer(container : any) { + if (!container) { + return null; + } + + if (container.nodeType === DOC_NODE_TYPE) { + return container.documentElement; + } else { + return container.firstChild; + } +} + function shouldAutoFocusHostComponent( type : string, props : Props, @@ -366,9 +378,59 @@ var ReactDOM = { // Top-level check occurs here instead of inside child reconciler because // because requirements vary between renderers. E.g. React Art // allows arrays. - invariant( - isValidElement(element), - 'render(): Invalid component element.' + if (!isValidElement(element)) { + if (typeof element === 'string') { + invariant( + false, + 'ReactDOM.render(): Invalid component element. Instead of ' + + 'passing a string like \'div\', pass ' + + 'React.createElement(\'div\') or
.' + ); + } else if (typeof element === 'function') { + invariant( + false, + 'ReactDOM.render(): Invalid component element. Instead of ' + + 'passing a class like Foo, pass React.createElement(Foo) ' + + 'or .' + ); + } else if (element != null && typeof element.props !== 'undefined') { + // Check if it quacks like an element + invariant( + false, + 'ReactDOM.render(): Invalid component element. This may be ' + + 'caused by unintentionally loading two independent copies ' + + 'of React.' + ); + } else { + invariant( + false, + 'ReactDOM.render(): Invalid component element.' + ); + } + } + } + + if (__DEV__) { + const isRootRenderedBySomeReact = Boolean(container._reactRootContainer); + const rootEl = getReactRootElementInContainer(container); + const hasNonRootReactChild = Boolean(rootEl && ReactDOMComponentTree.getInstanceFromNode(rootEl)); + + warning( + !hasNonRootReactChild || + isRootRenderedBySomeReact, + 'render(...): Replacing React-rendered children with a new root ' + + 'component. If you intended to update the children of this node, ' + + 'you should instead have the existing children update their state ' + + 'and render the new components instead of calling ReactDOM.render.' + ); + + warning( + !container.tagName || container.tagName.toUpperCase() !== 'BODY', + 'render(): Rendering components directly into document.body is ' + + 'discouraged, since its children are often manipulated by third-party ' + + 'scripts and browser extensions. This may lead to subtle ' + + 'reconciliation issues. Try rendering into a container element created ' + + 'for your app.' ); } @@ -394,7 +456,18 @@ var ReactDOM = { 'unmountComponentAtNode(...): Target container is not a DOM element.' ); warnAboutUnstableUse(); + if (container._reactRootContainer) { + if (__DEV__) { + const rootEl = getReactRootElementInContainer(container); + const renderedByDifferentReact = rootEl && !ReactDOMComponentTree.getInstanceFromNode(rootEl); + warning( + !renderedByDifferentReact, + 'unmountComponentAtNode(): The node you\'re attempting to unmount ' + + 'was rendered by another copy of React.' + ); + } + // Unmount should not be batched. return DOMRenderer.unbatchedUpdates(() => { return renderSubtreeIntoContainer(null, null, container, () => { diff --git a/src/renderers/dom/stack/client/ReactMount.js b/src/renderers/dom/stack/client/ReactMount.js index 424cc4f678b..cd3fe685035 100644 --- a/src/renderers/dom/stack/client/ReactMount.js +++ b/src/renderers/dom/stack/client/ReactMount.js @@ -434,23 +434,36 @@ var ReactMount = { _renderSubtreeIntoContainer: function(parentComponent, nextElement, container, callback) { validateCallback(callback, 'ReactDOM.render'); - invariant( - React.isValidElement(nextElement), - 'ReactDOM.render(): Invalid component element.%s', - ( - typeof nextElement === 'string' ? - ' Instead of passing a string like \'div\', pass ' + - 'React.createElement(\'div\') or
.' : - typeof nextElement === 'function' ? - ' Instead of passing a class like Foo, pass ' + - 'React.createElement(Foo) or .' : + if (!React.isValidElement(nextElement)) { + if (typeof nextElement === 'string') { + invariant( + false, + 'ReactDOM.render(): Invalid component element. Instead of ' + + 'passing a string like \'div\', pass ' + + 'React.createElement(\'div\') or
.' + ); + } else if (typeof nextElement === 'function') { + invariant( + false, + 'ReactDOM.render(): Invalid component element. Instead of ' + + 'passing a class like Foo, pass React.createElement(Foo) ' + + 'or .' + ); + } else if (nextElement != null && typeof nextElement.props !== 'undefined') { // Check if it quacks like an element - nextElement != null && nextElement.props !== undefined ? - ' This may be caused by unintentionally loading two independent ' + - 'copies of React.' : - '' - ) - ); + invariant( + false, + 'ReactDOM.render(): Invalid component element. This may be ' + + 'caused by unintentionally loading two independent copies ' + + 'of React.' + ); + } else { + invariant( + false, + 'ReactDOM.render(): Invalid component element.' + ); + } + } warning( !container || !container.tagName ||