From 9d93693b2e668a7d33e33083aa04909bebc58ec2 Mon Sep 17 00:00:00 2001 From: Ben Alpert Date: Thu, 3 Sep 2015 16:59:27 -0700 Subject: [PATCH] Update wording and style for #2065 --- src/renderers/dom/client/ReactMount.js | 63 ++++++++++--------- .../dom/client/__tests__/ReactMount-test.js | 6 +- .../__tests__/ReactMountDestruction-test.js | 11 ++-- 3 files changed, 44 insertions(+), 36 deletions(-) diff --git a/src/renderers/dom/client/ReactMount.js b/src/renderers/dom/client/ReactMount.js index 9e29f4dde1..e2ba7ebd4f 100644 --- a/src/renderers/dom/client/ReactMount.js +++ b/src/renderers/dom/client/ReactMount.js @@ -351,6 +351,22 @@ function unmountComponentFromNode(instance, container) { } } +/** + * True if the supplied DOM node has a direct React-rendered child that is + * not a React root element. Useful for warning in `render`, + * `unmountComponentAtNode`, etc. + * + * @param {?DOMElement} node The candidate DOM node. + * @return {boolean} True if the DOM element contains a direct child that was + * rendered by React but is not a root element. + * @internal + */ +function hasNonRootReactChild(node) { + var reactRootID = getReactRootID(node); + return reactRootID ? reactRootID !== + ReactInstanceHandles.getReactRootIDFromNodeID(reactRootID) : false; +} + /** * Temporary (?) hack so that we can store all top-level pending updates on * composites instead of having to worry about different types of components @@ -587,14 +603,15 @@ var ReactMount = { var reactRootElement = getReactRootElementInContainer(container); var containerHasReactMarkup = reactRootElement && ReactMount.isRenderedByReact(reactRootElement); - var containerHasNonRootReactChild = - ReactMount.hasNonRootReactChild(container); + var containerHasNonRootReactChild = hasNonRootReactChild(container); if (__DEV__) { warning( !containerHasNonRootReactChild, - 'renderComponent(...): Replacing React-rendered children with a new ' + - 'root component.' + '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.' ); if (!containerHasReactMarkup || reactRootElement.nextSibling) { @@ -709,23 +726,27 @@ var ReactMount = { if (!component) { // Check if the node being unmounted was rendered by React, but isn't a // root node. - var containerHasNonRootReactChild = ReactMount.hasNonRootReactChild( - container); + var containerHasNonRootReactChild = hasNonRootReactChild(container); // Check if the container itself is a React root node. - var containerID = ReactMount.getID(container); - var containerRootID = ReactInstanceHandles.getReactRootIDFromNodeID( - containerID); + var containerID = internalGetID(container); var isContainerReactRoot = - containerID && containerRootID && containerID === containerRootID; + containerID && + containerID === + ReactInstanceHandles.getReactRootIDFromNodeID(containerID); if (__DEV__) { warning( !containerHasNonRootReactChild, 'unmountComponentAtNode(): The node you\'re attempting to unmount ' + - 'is not a valid React root node, and thus cannot be unmounted.%s', - (isContainerReactRoot ? ' You may have passed in a React root ' + - 'node as argument, rather than its container.' : '') + 'was rendered by React and is not a top-level container. %s', + ( + isContainerReactRoot ? + 'You may have accidentally passed in a React root node instead ' + + 'of its container.' : + 'Instead, have the parent component update its state and ' + + 'rerender in order to remove this component.' + ) ); } @@ -813,22 +834,6 @@ var ReactMount = { return id ? id.charAt(0) === SEPARATOR : false; }, - /** - * True if the supplied DOM node has a direct React-rendered child that is - * not a React root element. Useful for warning in renderComponent`, - * `unmountComponentAtNode`, etc. - * - * @param {?DOMElement} node The candidate DOM node. - * @return {boolean} True if the DOM element contains a direct child that was - * rendered by React but is not a root element. - * @internal - */ - hasNonRootReactChild: function(node) { - var reactRootID = getReactRootID(node); - return reactRootID ? reactRootID !== - ReactInstanceHandles.getReactRootIDFromNodeID(reactRootID) : false; - }, - /** * Traverses up the ancestors of the supplied node to find a node that is a * DOM representation of a React component. diff --git a/src/renderers/dom/client/__tests__/ReactMount-test.js b/src/renderers/dom/client/__tests__/ReactMount-test.js index 15ae6ddcd7..0849ad3f39 100644 --- a/src/renderers/dom/client/__tests__/ReactMount-test.js +++ b/src/renderers/dom/client/__tests__/ReactMount-test.js @@ -240,8 +240,10 @@ describe('ReactMount', function() { React.render(, rootNode); expect(console.error.callCount).toBe(1); expect(console.error.mostRecentCall.args[0]).toBe( - 'Warning: renderComponent(...): Replacing React-rendered children ' + - 'with a new root component.' + 'Warning: 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.' ); }); }); diff --git a/src/renderers/dom/client/__tests__/ReactMountDestruction-test.js b/src/renderers/dom/client/__tests__/ReactMountDestruction-test.js index 053f654327..cafc10d340 100644 --- a/src/renderers/dom/client/__tests__/ReactMountDestruction-test.js +++ b/src/renderers/dom/client/__tests__/ReactMountDestruction-test.js @@ -56,9 +56,9 @@ describe('ReactMount', function() { expect(console.error.callCount).toBe(1); expect(console.error.mostRecentCall.args[0]).toBe( 'Warning: unmountComponentAtNode(): The node you\'re attempting to ' + - 'unmount is not a valid React root node, and thus cannot be ' + - 'unmounted. You may have passed in a React root node as argument, ' + - 'rather than its container.' + 'unmount was rendered by React and is not a top-level container. You ' + + 'may have accidentally passed in a React root node instead of its ' + + 'container.' ); }); @@ -80,8 +80,9 @@ describe('ReactMount', function() { expect(console.error.callCount).toBe(1); expect(console.error.mostRecentCall.args[0]).toBe( 'Warning: unmountComponentAtNode(): The node you\'re attempting to ' + - 'unmount is not a valid React root node, and thus cannot be ' + - 'unmounted.' + 'unmount was rendered by React and is not a top-level container. ' + + 'Instead, have the parent component update its state and rerender in ' + + 'order to remove this component.' ); }); });