From 65568814179d2a28cc83d68a5e467c24b86da2f6 Mon Sep 17 00:00:00 2001 From: CommitSyncScript Date: Fri, 28 Jun 2013 13:46:29 -0700 Subject: [PATCH] Throw on Missing Elements This changes React to throw when `ReactID.getNode()` fails to find a node. This method is used by two call sites: - Implements `ReactComponent#getDOMNode`. This method already throws if a component is not mounted, and //all mounted components should be able to find their rendered root nodes//. - Used by `ReactDOMIDOperations`. These call sites aleady assume that `getNode` returns a non-null. Currently, if the node is not found, this is the site that fatals (and the stack trace is much harder to debug). The error message should make it //a lot// easier to debug unexpected DOM trees. In particular, this will help track down all the places where the browser inserts `` unexpectedly. --- src/core/ReactComponent.js | 2 +- src/core/ReactID.js | 13 ++++------ src/core/ReactInstanceHandles.js | 22 ++++++++++++----- src/core/ReactMount.js | 7 +++--- .../__tests__/ReactInstanceHandles-test.js | 24 +++++++++++++++++++ 5 files changed, 48 insertions(+), 20 deletions(-) diff --git a/src/core/ReactComponent.js b/src/core/ReactComponent.js index db672385df..addad00ab5 100644 --- a/src/core/ReactComponent.js +++ b/src/core/ReactComponent.js @@ -276,7 +276,7 @@ var ReactComponent = { /** * Returns the DOM node rendered by this component. * - * @return {?DOMElement} The root node of this component. + * @return {DOMElement} The root node of this component. * @final * @protected */ diff --git a/src/core/ReactID.js b/src/core/ReactID.js index 4be8bdca25..d677b14e8a 100644 --- a/src/core/ReactID.js +++ b/src/core/ReactID.js @@ -85,19 +85,14 @@ function setID(node, id) { * Finds the node with the supplied React-generated DOM ID. * * @param {string} id A React-generated DOM ID. - * @return {?DOMElement} DOM node with the suppled `id`. + * @return {DOMElement} DOM node with the suppled `id`. * @internal */ function getNode(id) { - if (nodeCache.hasOwnProperty(id)) { - var node = nodeCache[id]; - if (isValid(node, id)) { - return node; - } + if (!nodeCache.hasOwnProperty(id) || !isValid(nodeCache[id], id)) { + nodeCache[id] = ReactMount.findReactNodeByID(id); } - - return nodeCache[id] = - ReactMount.findReactRenderedDOMNodeSlow(id); + return nodeCache[id]; } /** diff --git a/src/core/ReactInstanceHandles.js b/src/core/ReactInstanceHandles.js index af508207e3..f9678056ff 100644 --- a/src/core/ReactInstanceHandles.js +++ b/src/core/ReactInstanceHandles.js @@ -280,21 +280,31 @@ var ReactInstanceHandles = { * * @param {DOMEventTarget} ancestorNode Search from this root. * @pararm {string} id ID of the DOM representation of the component. - * @return {?DOMEventTarget} DOM node with the supplied `id`, if one exists. + * @return {DOMEventTarget} DOM node with the supplied `id`. * @internal */ findComponentRoot: function(ancestorNode, id) { var child = ancestorNode.firstChild; while (child) { var childID = ReactID.getID(child); - if (id === childID) { - return child; - } else if (childID && isAncestorIDOf(childID, id)) { - return ReactInstanceHandles.findComponentRoot(child, id); + if (childID) { + if (id === childID) { + return child; + } else if (isAncestorIDOf(childID, id)) { + return ReactInstanceHandles.findComponentRoot(child, id); + } } child = child.nextSibling; } - // Effectively: return null; + invariant( + false, + 'findComponentRoot: Unable to find element by React ID, `%s`. This ' + + 'indicates that someone (or the browser) has mutated the DOM tree in ' + + 'an unexpected way. Try inspecting the child nodes of the element with ' + + 'React ID, `%s`.', + id, + ReactID.getID(ancestorNode) + ); }, /** diff --git a/src/core/ReactMount.js b/src/core/ReactMount.js index 56c5c54782..62d9bc6b13 100644 --- a/src/core/ReactMount.js +++ b/src/core/ReactMount.js @@ -269,13 +269,12 @@ var ReactMount = { }, /** - * Given the ID of a DOM node rendered by a React component, finds the root - * DOM node of the React component. + * Finds an element rendered by React with the supplied ID. * * @param {string} id ID of a DOM node in the React component. - * @return {?DOMElement} Root DOM node of the React component. + * @return {DOMElement} Root DOM node of the React component. */ - findReactRenderedDOMNodeSlow: function(id) { + findReactNodeByID: function(id) { var reactRoot = ReactMount.findReactContainerForID(id); return ReactInstanceHandles.findComponentRoot(reactRoot, id); } diff --git a/src/core/__tests__/ReactInstanceHandles-test.js b/src/core/__tests__/ReactInstanceHandles-test.js index 4daef67300..331b47a183 100644 --- a/src/core/__tests__/ReactInstanceHandles-test.js +++ b/src/core/__tests__/ReactInstanceHandles-test.js @@ -121,6 +121,30 @@ describe('ReactInstanceHandles', function() { ) ).toBe(childNodeB); }); + + it('should throw if a rendered element cannot be found', function() { + var parentNode = document.createElement('table'); + var childNodeA = document.createElement('tbody'); + var childNodeB = document.createElement('tr'); + parentNode.appendChild(childNodeA); + childNodeA.appendChild(childNodeB); + + ReactID.setID(parentNode, '.react[0]'); + // No ID on `childNodeA`, it was "rendered by the browser". + ReactID.setID(childNodeB, '.react[0].1:0'); + + expect(function() { + ReactInstanceHandles.findComponentRoot( + parentNode, + ReactID.getID(childNodeB) + ); + }).toThrow( + 'Invariant Violation: findComponentRoot: Unable to find element by ' + + 'React ID, `.react[0].1:0`. This indicates that someone (or the ' + + 'browser) has mutated the DOM tree in an unexpected way. Try ' + + 'inspecting the child nodes of the element with React ID, `.react[0]`.' + ); + }); }); describe('getReactRootIDFromNodeID', function() {