From 68b31070623e36c2e17b446f80471ef33680f53f Mon Sep 17 00:00:00 2001 From: Ben Alpert Date: Wed, 17 Aug 2016 14:00:21 -0700 Subject: [PATCH] Improve validateDOMNesting message for whitespace For #5071. --- .../dom/client/validateDOMNesting.js | 30 +++++++++++++++++-- src/renderers/dom/shared/ReactDOMComponent.js | 19 ++++++------ .../dom/shared/ReactDOMTextComponent.js | 2 +- .../__tests__/ReactDOMComponent-test.js | 16 ++++++---- 4 files changed, 49 insertions(+), 18 deletions(-) diff --git a/src/renderers/dom/client/validateDOMNesting.js b/src/renderers/dom/client/validateDOMNesting.js index 9422a079e1462..860e37da1fbaf 100644 --- a/src/renderers/dom/client/validateDOMNesting.js +++ b/src/renderers/dom/client/validateDOMNesting.js @@ -322,11 +322,24 @@ if (__DEV__) { var didWarn = {}; - validateDOMNesting = function(childTag, childInstance, ancestorInfo) { + validateDOMNesting = function( + childTag, + childText, + childInstance, + ancestorInfo + ) { ancestorInfo = ancestorInfo || emptyAncestorInfo; var parentInfo = ancestorInfo.current; var parentTag = parentInfo && parentInfo.tag; + if (childText != null) { + warning( + childTag == null, + 'validateDOMNesting: when childText is passed, childTag should be null' + ); + childTag = '#text'; + } + var invalidParent = isTagValidWithParent(childTag, parentTag) ? null : parentInfo; var invalidAncestor = @@ -385,7 +398,17 @@ if (__DEV__) { didWarn[warnKey] = true; var tagDisplayName = childTag; - if (childTag !== '#text') { + var whitespaceInfo = ''; + if (childTag === '#text') { + if (/\S/.test(childText)) { + tagDisplayName = 'Text nodes'; + } else { + tagDisplayName = 'Whitespace text nodes'; + whitespaceInfo = + ' Make sure you don\'t have any extra whitespace between tags on ' + + 'each line of your source code.'; + } + } else { tagDisplayName = '<' + childTag + '>'; } @@ -398,10 +421,11 @@ if (__DEV__) { } warning( false, - 'validateDOMNesting(...): %s cannot appear as a child of <%s>. ' + + 'validateDOMNesting(...): %s cannot appear as a child of <%s>.%s ' + 'See %s.%s', tagDisplayName, ancestorTag, + whitespaceInfo, ownerInfo, info ); diff --git a/src/renderers/dom/shared/ReactDOMComponent.js b/src/renderers/dom/shared/ReactDOMComponent.js index 8db1b8f5956ad..b5233e021cdc0 100644 --- a/src/renderers/dom/shared/ReactDOMComponent.js +++ b/src/renderers/dom/shared/ReactDOMComponent.js @@ -254,9 +254,9 @@ function optionPostMount() { ReactDOMOption.postMountWrapper(inst); } -var setContentChildForInstrumentation = emptyFunction; +var setAndValidateContentChildDev = emptyFunction; if (__DEV__) { - setContentChildForInstrumentation = function(content) { + setAndValidateContentChildDev = function(content) { var hasExistingContent = this._contentDebugID != null; var debugID = this._debugID; // This ID represents the inlined child that has no backing instance: @@ -270,6 +270,7 @@ if (__DEV__) { return; } + validateDOMNesting(null, String(content), this, this._ancestorInfo); this._contentDebugID = contentDebugID; if (hasExistingContent) { ReactInstrumentation.debugTool.onBeforeUpdateComponent(contentDebugID, content); @@ -497,7 +498,7 @@ function ReactDOMComponent(element) { this._flags = 0; if (__DEV__) { this._ancestorInfo = null; - setContentChildForInstrumentation.call(this, null); + setAndValidateContentChildDev.call(this, null); } } @@ -605,7 +606,7 @@ ReactDOMComponent.Mixin = { if (parentInfo) { // parentInfo should always be present except for the top-level // component when server rendering - validateDOMNesting(this._tag, this, parentInfo); + validateDOMNesting(this._tag, null, this, parentInfo); } this._ancestorInfo = validateDOMNesting.updatedAncestorInfo(parentInfo, this._tag, this); @@ -801,7 +802,7 @@ ReactDOMComponent.Mixin = { // TODO: Validate that text is allowed as a child of this node ret = escapeTextContentForBrowser(contentToUse); if (__DEV__) { - setContentChildForInstrumentation.call(this, contentToUse); + setAndValidateContentChildDev.call(this, contentToUse); } } else if (childrenToUse != null) { var mountImages = this.mountChildren( @@ -843,7 +844,7 @@ ReactDOMComponent.Mixin = { if (contentToUse != null) { // TODO: Validate that text is allowed as a child of this node if (__DEV__) { - setContentChildForInstrumentation.call(this, contentToUse); + setAndValidateContentChildDev.call(this, contentToUse); } DOMLazyTree.queueText(lazyTree, contentToUse); } else if (childrenToUse != null) { @@ -1117,7 +1118,7 @@ ReactDOMComponent.Mixin = { if (lastContent !== nextContent) { this.updateTextContent('' + nextContent); if (__DEV__) { - setContentChildForInstrumentation.call(this, nextContent); + setAndValidateContentChildDev.call(this, nextContent); } } } else if (nextHtml != null) { @@ -1129,7 +1130,7 @@ ReactDOMComponent.Mixin = { } } else if (nextChildren != null) { if (__DEV__) { - setContentChildForInstrumentation.call(this, null); + setAndValidateContentChildDev.call(this, null); } this.updateChildren(nextChildren, transaction, context); @@ -1196,7 +1197,7 @@ ReactDOMComponent.Mixin = { this._wrapperState = null; if (__DEV__) { - setContentChildForInstrumentation.call(this, null); + setAndValidateContentChildDev.call(this, null); } }, diff --git a/src/renderers/dom/shared/ReactDOMTextComponent.js b/src/renderers/dom/shared/ReactDOMTextComponent.js index 24b0027d2600e..860bc33496171 100644 --- a/src/renderers/dom/shared/ReactDOMTextComponent.js +++ b/src/renderers/dom/shared/ReactDOMTextComponent.js @@ -75,7 +75,7 @@ Object.assign(ReactDOMTextComponent.prototype, { if (parentInfo) { // parentInfo should always be present except for the top-level // component when server rendering - validateDOMNesting('#text', this, parentInfo); + validateDOMNesting(null, this._stringText, this, parentInfo); } } diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js index 7688623e435b3..e9cb267da961b 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js @@ -736,7 +736,7 @@ describe('ReactDOMComponent', function() { }); it('should work error event on element', function() { - spyOn(console, 'error'); + spyOn(console, 'error'); var container = document.createElement('div'); ReactDOM.render(