@samitouri / QOS-React-2 / commits / 6556881417
Fork 0

The library for web and native user interfaces.

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 `<tbody>` unexpectedly.

CommitSyncScript committed Jun 28, 2013 at 13:46 UTC 65568814179d2a28cc83d68a5e467c24b86da2f6
5 files changed +48 -20
src/core/ReactComponent.js
+1 -1
@@ -276,7 +276,7 @@ var ReactComponent = {
276 /**
277 * Returns the DOM node rendered by this component.
278 *
279 - * @return {?DOMElement} The root node of this component.
279 + * @return {DOMElement} The root node of this component.
280 * @final
281 * @protected
282 */
src/core/ReactID.js
+4 -9
@@ -85,19 +85,14 @@ function setID(node, id) {
85 * Finds the node with the supplied React-generated DOM ID.
86 *
87 * @param {string} id A React-generated DOM ID.
88 - * @return {?DOMElement} DOM node with the suppled `id`.
88 + * @return {DOMElement} DOM node with the suppled `id`.
89 * @internal
90 */
91 function getNode(id) {
92 - if (nodeCache.hasOwnProperty(id)) {
93 - var node = nodeCache[id];
94 - if (isValid(node, id)) {
95 - return node;
96 - }
92 + if (!nodeCache.hasOwnProperty(id) || !isValid(nodeCache[id], id)) {
93 + nodeCache[id] = ReactMount.findReactNodeByID(id);
94 }
98 -
99 - return nodeCache[id] =
100 - ReactMount.findReactRenderedDOMNodeSlow(id);
95 + return nodeCache[id];
96 }
97
98 /**
src/core/ReactInstanceHandles.js
+16 -6
@@ -280,21 +280,31 @@ var ReactInstanceHandles = {
280 *
281 * @param {DOMEventTarget} ancestorNode Search from this root.
282 * @pararm {string} id ID of the DOM representation of the component.
283 - * @return {?DOMEventTarget} DOM node with the supplied `id`, if one exists.
283 + * @return {DOMEventTarget} DOM node with the supplied `id`.
284 * @internal
285 */
286 findComponentRoot: function(ancestorNode, id) {
287 var child = ancestorNode.firstChild;
288 while (child) {
289 var childID = ReactID.getID(child);
290 - if (id === childID) {
291 - return child;
292 - } else if (childID && isAncestorIDOf(childID, id)) {
293 - return ReactInstanceHandles.findComponentRoot(child, id);
290 + if (childID) {
291 + if (id === childID) {
292 + return child;
293 + } else if (isAncestorIDOf(childID, id)) {
294 + return ReactInstanceHandles.findComponentRoot(child, id);
295 + }
296 }
297 child = child.nextSibling;
298 }
297 - // Effectively: return null;
299 + invariant(
300 + false,
301 + 'findComponentRoot: Unable to find element by React ID, `%s`. This ' +
302 + 'indicates that someone (or the browser) has mutated the DOM tree in ' +
303 + 'an unexpected way. Try inspecting the child nodes of the element with ' +
304 + 'React ID, `%s`.',
305 + id,
306 + ReactID.getID(ancestorNode)
307 + );
308 },
309
310 /**
src/core/ReactMount.js
+3 -4
@@ -269,13 +269,12 @@ var ReactMount = {
269 },
270
271 /**
272 - * Given the ID of a DOM node rendered by a React component, finds the root
273 - * DOM node of the React component.
272 + * Finds an element rendered by React with the supplied ID.
273 *
274 * @param {string} id ID of a DOM node in the React component.
276 - * @return {?DOMElement} Root DOM node of the React component.
275 + * @return {DOMElement} Root DOM node of the React component.
276 */
278 - findReactRenderedDOMNodeSlow: function(id) {
277 + findReactNodeByID: function(id) {
278 var reactRoot = ReactMount.findReactContainerForID(id);
279 return ReactInstanceHandles.findComponentRoot(reactRoot, id);
280 }
src/core/__tests__/ReactInstanceHandles-test.js
+24
@@ -121,6 +121,30 @@ describe('ReactInstanceHandles', function() {
121 )
122 ).toBe(childNodeB);
123 });
124 +
125 + it('should throw if a rendered element cannot be found', function() {
126 + var parentNode = document.createElement('table');
127 + var childNodeA = document.createElement('tbody');
128 + var childNodeB = document.createElement('tr');
129 + parentNode.appendChild(childNodeA);
130 + childNodeA.appendChild(childNodeB);
131 +
132 + ReactID.setID(parentNode, '.react[0]');
133 + // No ID on `childNodeA`, it was "rendered by the browser".
134 + ReactID.setID(childNodeB, '.react[0].1:0');
135 +
136 + expect(function() {
137 + ReactInstanceHandles.findComponentRoot(
138 + parentNode,
139 + ReactID.getID(childNodeB)
140 + );
141 + }).toThrow(
142 + 'Invariant Violation: findComponentRoot: Unable to find element by ' +
143 + 'React ID, `.react[0].1:0`. This indicates that someone (or the ' +
144 + 'browser) has mutated the DOM tree in an unexpected way. Try ' +
145 + 'inspecting the child nodes of the element with React ID, `.react[0]`.'
146 + );
147 + });
148 });
149
150 describe('getReactRootIDFromNodeID', function() {