From 70f704dca6e5679dfda5867c44832a1ba5eb9b9e Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 16 Dec 2016 18:57:58 -0800 Subject: [PATCH] [Fiber] Nesting validation warnings (#8586) * (WIP) Nesting warnings * Remove indirection * Add a note about namespace * Fix Flow and make host context required This makes it easier to avoid accidental nulls. I also added a test for the production case to avoid regressing because of __DEV__ branches. --- scripts/fiber/tests-passing-except-dev.txt | 4 - scripts/fiber/tests-passing.txt | 5 + src/renderers/art/ReactARTFiber.js | 7 +- .../dom/__tests__/ReactDOMProduction-test.js | 59 ++++++ src/renderers/dom/fiber/ReactDOMFiber.js | 82 +++++++- .../dom/fiber/ReactDOMFiberComponent.js | 24 ++- .../dom/fiber/wrappers/ReactDOMFiberOption.js | 9 +- .../__tests__/ReactDOMComponent-test.js | 115 ++++++++--- .../dom/shared/validateDOMNesting.js | 192 ++++++++++-------- .../wrappers/__tests__/ReactDOMOption-test.js | 21 +- src/renderers/native/ReactNativeFiber.js | 15 +- src/renderers/noop/ReactNoop.js | 14 +- .../shared/fiber/ReactFiberCompleteWork.js | 6 +- .../shared/fiber/ReactFiberHostContext.js | 39 ++-- .../shared/fiber/ReactFiberReconciler.js | 9 +- 15 files changed, 420 insertions(+), 181 deletions(-) diff --git a/scripts/fiber/tests-passing-except-dev.txt b/scripts/fiber/tests-passing-except-dev.txt index b112148a6e5e7..63f25e6d56093 100644 --- a/scripts/fiber/tests-passing-except-dev.txt +++ b/scripts/fiber/tests-passing-except-dev.txt @@ -3,10 +3,6 @@ src/addons/transitions/__tests__/ReactTransitionGroup-test.js src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js * should not warn when server-side rendering `onScroll` -* warns on invalid nesting -* warns on invalid nesting at root -* warns nicely for table rows -* gives useful context in warnings * should warn about incorrect casing on properties (ssr) * should warn about incorrect casing on event handlers (ssr) * should warn about class (ssr) diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 78ed9f2cdf4c7..ed67b3389a905 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -509,6 +509,7 @@ src/renderers/dom/__tests__/ReactDOMProduction-test.js * should use prod React * should handle a simple flow * should call lifecycle methods +* should keep track of namespace across portals in production src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js * should render strings as children @@ -690,6 +691,10 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js * should throw when an attack vector is used server-side * should throw when an invalid tag name is used * should throw when an attack vector is used +* warns on invalid nesting +* warns on invalid nesting at root +* warns nicely for table rows +* gives useful context in warnings * should warn about incorrect casing on properties * should warn about incorrect casing on event handlers * should warn about class diff --git a/src/renderers/art/ReactARTFiber.js b/src/renderers/art/ReactARTFiber.js index ffa9cc419cb5f..1c6160d039e4c 100644 --- a/src/renderers/art/ReactARTFiber.js +++ b/src/renderers/art/ReactARTFiber.js @@ -18,6 +18,7 @@ require('art/modes/current').setCurrent( const Mode = require('art/modes/current'); const Transform = require('art/core/transform'); const invariant = require('fbjs/lib/invariant'); +const emptyObject = require('emptyObject'); const React = require('React'); const ReactFiberReconciler = require('ReactFiberReconciler'); @@ -485,8 +486,12 @@ const ARTRenderer = ReactFiberReconciler({ // Noop }, + getRootHostContext() { + return emptyObject; + }, + getChildHostContext() { - return null; + return emptyObject; }, scheduleAnimationCallback: window.requestAnimationFrame, diff --git a/src/renderers/dom/__tests__/ReactDOMProduction-test.js b/src/renderers/dom/__tests__/ReactDOMProduction-test.js index c64a9411808e3..5ddce18b5fe7c 100644 --- a/src/renderers/dom/__tests__/ReactDOMProduction-test.js +++ b/src/renderers/dom/__tests__/ReactDOMProduction-test.js @@ -13,6 +13,8 @@ describe('ReactDOMProduction', () => { var oldProcess; + var ReactDOMFeatureFlags = require('ReactDOMFeatureFlags'); + var React; var ReactDOM; @@ -193,4 +195,61 @@ describe('ReactDOMProduction', () => { ' for full errors and additional helpful warnings.' ); }); + + if (ReactDOMFeatureFlags.useFiber) { + // This test is originally from ReactDOMFiber-test but we replicate it here + // to avoid production-only regressions because of host context differences + // in dev and prod. + it('should keep track of namespace across portals in production', () => { + var svgEls, htmlEls; + var expectSVG = {ref: el => svgEls.push(el)}; + var expectHTML = {ref: el => htmlEls.push(el)}; + var usePortal = function(tree) { + return ReactDOM.unstable_createPortal( + tree, + document.createElement('div') + ); + }; + var assertNamespacesMatch = function(tree) { + var container = document.createElement('div'); + svgEls = []; + htmlEls = []; + ReactDOM.render(tree, container); + svgEls.forEach(el => { + expect(el.namespaceURI).toBe('http://www.w3.org/2000/svg'); + }); + htmlEls.forEach(el => { + expect(el.namespaceURI).toBe('http://www.w3.org/1999/xhtml'); + }); + ReactDOM.unmountComponentAtNode(container); + expect(container.innerHTML).toBe(''); + }; + + assertNamespacesMatch( +
+ + +

+ {usePortal( + + + + + +

+ + {usePortal(

)} + + + + )} +

+ + + +

+ + ); + }); + } }); diff --git a/src/renderers/dom/fiber/ReactDOMFiber.js b/src/renderers/dom/fiber/ReactDOMFiber.js index 19c29643f3b70..3bbc4d52a0cec 100644 --- a/src/renderers/dom/fiber/ReactDOMFiber.js +++ b/src/renderers/dom/fiber/ReactDOMFiber.js @@ -39,6 +39,11 @@ var { } = ReactDOMFiberComponent; var { precacheFiberNode } = ReactDOMComponentTree; +if (__DEV__) { + var validateDOMNesting = require('validateDOMNesting'); + var { updatedAncestorInfo } = validateDOMNesting; +} + const DOCUMENT_NODE = 9; ReactDOMInjection.inject(); @@ -52,17 +57,44 @@ findDOMNode._injectFiber(function(fiber: Fiber) { type DOMContainerElement = Element & { _reactRootContainer: ?Object }; type Container = Element; -type Props = { className ?: string }; +type Props = { children ?: mixed }; type Instance = Element; type TextInstance = Text; +type HostContextDev = { + namespace : string, + ancestorInfo : mixed, +}; +type HostContextProd = string; +type HostContext = HostContextDev | HostContextProd; + let eventsEnabled : ?boolean = null; let selectionInformation : ?mixed = null; var DOMRenderer = ReactFiberReconciler({ - getChildHostContext(parentHostContext : string | null, type : string) { - const parentNamespace = parentHostContext; + getRootHostContext(rootContainerInstance : Container) : HostContext { + const type = rootContainerInstance.tagName.toLowerCase(); + if (__DEV__) { + const namespace = getChildNamespace(null, type); + const isMountingIntoDocument = rootContainerInstance.ownerDocument.documentElement === rootContainerInstance; + const ancestorInfo = updatedAncestorInfo(null, isMountingIntoDocument ? '#document' : type, null); + return {namespace, ancestorInfo}; + } + return getChildNamespace(null, type); + }, + + getChildHostContext( + parentHostContext : HostContext, + type : string, + ) : HostContext { + if (__DEV__) { + const parentHostContextDev = ((parentHostContext : any) : HostContextDev); + const namespace = getChildNamespace(parentHostContextDev.namespace, type); + const ancestorInfo = updatedAncestorInfo(parentHostContextDev.ancestorInfo, type, null); + return {namespace, ancestorInfo}; + } + const parentNamespace = ((parentHostContext : any) : HostContextProd); return getChildNamespace(parentNamespace, type); }, @@ -83,10 +115,26 @@ var DOMRenderer = ReactFiberReconciler({ type : string, props : Props, rootContainerInstance : Container, - hostContext : string | null, + hostContext : HostContext, internalInstanceHandle : Object, ) : Instance { - const domElement : Instance = createElement(type, props, rootContainerInstance, hostContext); + let parentNamespace : string; + if (__DEV__) { + // TODO: take namespace into account when validating. + const hostContextDev = ((hostContext : any) : HostContextDev); + validateDOMNesting(type, null, null, hostContextDev.ancestorInfo); + if ( + typeof props.children === 'string' || + typeof props.children === 'number' + ) { + const ownAncestorInfo = updatedAncestorInfo(hostContextDev.ancestorInfo, type, null); + validateDOMNesting(null, String(props.children), null, ownAncestorInfo); + } + parentNamespace = hostContextDev.namespace; + } else { + parentNamespace = ((hostContext : any) : HostContextProd); + } + const domElement : Instance = createElement(type, props, rootContainerInstance, parentNamespace); precacheFiberNode(internalInstanceHandle, domElement); return domElement; }, @@ -108,8 +156,19 @@ var DOMRenderer = ReactFiberReconciler({ domElement : Instance, type : string, oldProps : Props, - newProps : Props + newProps : Props, + hostContext : HostContext, ) : boolean { + if (__DEV__) { + const hostContextDev = ((hostContext : any) : HostContextDev); + if (typeof newProps.children !== typeof oldProps.children && ( + typeof newProps.children === 'string' || + typeof newProps.children === 'number' + )) { + const ownAncestorInfo = updatedAncestorInfo(hostContextDev.ancestorInfo, type, null); + validateDOMNesting(null, String(newProps.children), null, ownAncestorInfo); + } + } return true; }, @@ -143,7 +202,16 @@ var DOMRenderer = ReactFiberReconciler({ domElement.textContent = ''; }, - createTextInstance(text : string, rootContainerInstance : Container, internalInstanceHandle : Object) : TextInstance { + createTextInstance( + text : string, + rootContainerInstance : Container, + hostContext : HostContext, + internalInstanceHandle : Object + ) : TextInstance { + if (__DEV__) { + const hostContextDev = ((hostContext : any) : HostContextDev); + validateDOMNesting(null, text, null, hostContextDev.ancestorInfo); + } var textNode : TextInstance = document.createTextNode(text); precacheFiberNode(internalInstanceHandle, textNode); return textNode; diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index 497a3f648d50c..49ed51cad9246 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberComponent.js +++ b/src/renderers/dom/fiber/ReactDOMFiberComponent.js @@ -56,6 +56,7 @@ var STYLE = 'style'; var HTML = '__html'; var { + html: HTML_NAMESPACE, svg: SVG_NAMESPACE, mathml: MATH_NAMESPACE, } = DOMNamespaces; @@ -451,26 +452,26 @@ function updateDOMProperties( } // Assumes there is no parent namespace. -function getIntrinsicNamespace(type : string) : string | null { +function getIntrinsicNamespace(type : string) : string { switch (type) { case 'svg': return SVG_NAMESPACE; case 'math': return MATH_NAMESPACE; default: - return null; + return HTML_NAMESPACE; } } var ReactDOMFiberComponent = { - getChildNamespace(parentNamespace : string | null, type : string) : string | null { - if (parentNamespace == null) { - // No parent namespace: potential entry point. + getChildNamespace(parentNamespace : string | null, type : string) : string { + if (parentNamespace == null || parentNamespace === HTML_NAMESPACE) { + // No (or default) parent namespace: potential entry point. return getIntrinsicNamespace(type); } if (parentNamespace === SVG_NAMESPACE && type === 'foreignObject') { // We're leaving SVG. - return null; + return HTML_NAMESPACE; } // By default, pass namespace below. return parentNamespace; @@ -480,14 +481,17 @@ var ReactDOMFiberComponent = { type : string, props : Object, rootContainerElement : Element, - parentNamespace : string | null + parentNamespace : string ) : Element { // We create tags in the namespace of their parent container, except HTML // tags get no namespace. var ownerDocument = rootContainerElement.ownerDocument; var domElement : Element; - var namespaceURI = parentNamespace || getIntrinsicNamespace(type); - if (namespaceURI == null) { + var namespaceURI = parentNamespace; + if (namespaceURI === HTML_NAMESPACE) { + namespaceURI = getIntrinsicNamespace(type); + } + if (namespaceURI === HTML_NAMESPACE) { if (__DEV__) { warning( type === type.toLowerCase() || @@ -649,7 +653,7 @@ var ReactDOMFiberComponent = { tag : string, lastRawProps : Object, nextRawProps : Object, - rootContainerElement : Element + rootContainerElement : Element, ) : void { if (__DEV__) { validatePropertiesInDevelopment(tag, nextRawProps); diff --git a/src/renderers/dom/fiber/wrappers/ReactDOMFiberOption.js b/src/renderers/dom/fiber/wrappers/ReactDOMFiberOption.js index 005323c7a2f48..6df60d3162369 100644 --- a/src/renderers/dom/fiber/wrappers/ReactDOMFiberOption.js +++ b/src/renderers/dom/fiber/wrappers/ReactDOMFiberOption.js @@ -15,25 +15,20 @@ var React = require('React'); var warning = require('warning'); -var didWarnInvalidOptionChildren = false; function flattenChildren(children) { var content = ''; // Flatten children and warn if they aren't strings or numbers; // invalid types are ignored. + // We can silently skip them because invalid DOM nesting warning + // catches these cases in Fiber. React.Children.forEach(children, function(child) { if (child == null) { return; } if (typeof child === 'string' || typeof child === 'number') { content += child; - } else if (!didWarnInvalidOptionChildren) { - didWarnInvalidOptionChildren = true; - warning( - false, - 'Only strings and numbers are supported as

); + var addendum = ReactDOMFeatureFlags.useFiber ? + '\n in tr (at **)' + + '\n in div (at **)' : + ' See div > tr.'; + expectDev(console.error.calls.count()).toBe(1); - expectDev(console.error.calls.argsFor(0)[0]).toBe( + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe( 'Warning: validateDOMNesting(...): cannot appear as a child of ' + - '
. See div > tr.' + '
.' + addendum ); }); @@ -1284,10 +1289,16 @@ describe('ReactDOMComponent', () => { var p = document.createElement('p'); ReactDOM.render(

, p); + var addendum = ReactDOMFeatureFlags.useFiber ? + // There is no outer `p` here because root container is not part of the stack. + '\n in p (at **)' + + '\n in span (at **)' : + ' See p > ... > p.'; + expectDev(console.error.calls.count()).toBe(1); - expectDev(console.error.calls.argsFor(0)[0]).toBe( + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe( 'Warning: validateDOMNesting(...):

cannot appear as a descendant ' + - 'of

. See p > ... > p.' + 'of

.' + addendum ); }); @@ -1307,22 +1318,39 @@ describe('ReactDOMComponent', () => { } ReactTestUtils.renderIntoDocument(); - expectDev(console.error.calls.count()).toBe(3); - expectDev(console.error.calls.argsFor(0)[0]).toBe( + + var addendum1 = ReactDOMFeatureFlags.useFiber ? + '\n in tr (at **)' + + '\n in Row (at **)' + + '\n in table (at **)' + + '\n in Foo (at **)' : + ' See Foo > table > Row > tr.'; + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe( 'Warning: validateDOMNesting(...): cannot appear as a child of ' + - '. See Foo > table > Row > tr. Add a to your code to ' + - 'match the DOM tree generated by the browser.' + '
. Add a to your code to match the DOM tree generated ' + + 'by the browser.' + addendum1 ); - expectDev(console.error.calls.argsFor(1)[0]).toBe( + + var addendum2 = ReactDOMFeatureFlags.useFiber ? + '\n in tr (at **)' + + '\n in Row (at **)' + + '\n in table (at **)' + + '\n in Foo (at **)' : + ' See Row > tr > #text.'; + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toBe( 'Warning: validateDOMNesting(...): Text nodes cannot appear as a ' + - 'child of . See Row > tr > #text.' + 'child of .' + addendum2 ); - expectDev(console.error.calls.argsFor(2)[0]).toBe( + + var addendum3 = ReactDOMFeatureFlags.useFiber ? + '\n in table (at **)' + + '\n in Foo (at **)' : + ' See Foo > table > #text.'; + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(2)[0])).toBe( 'Warning: validateDOMNesting(...): Whitespace text nodes cannot ' + 'appear as a child of
. Make sure you don\'t have any extra ' + - 'whitespace between tags on each line of your source code. See Foo > ' + - 'table > #text.' + 'whitespace between tags on each line of your source code.' + addendum3 ); }); @@ -1355,8 +1383,14 @@ describe('ReactDOMComponent', () => { }); ReactTestUtils.renderIntoDocument(); expectDev(console.error.calls.count()).toBe(1); - expectDev(console.error.calls.argsFor(0)[0]).toContain( - 'See Viz1 > table > FancyRow > Row > tr.' + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toContain( + ReactDOMFeatureFlags.useFiber ? + '\n in tr (at **)' + + '\n in Row (at **)' + + '\n in FancyRow (at **)' + + '\n in table (at **)' + + '\n in Viz1 (at **)' : + 'See Viz1 > table > FancyRow > Row > tr.' ); var Viz2 = React.createClass({ @@ -1367,26 +1401,51 @@ describe('ReactDOMComponent', () => { }); ReactTestUtils.renderIntoDocument(); expectDev(console.error.calls.count()).toBe(2); - expectDev(console.error.calls.argsFor(1)[0]).toContain( - 'See Viz2 > FancyTable > Table > table > FancyRow > Row > tr.' + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toContain( + ReactDOMFeatureFlags.useFiber ? + '\n in tr (at **)' + + '\n in Row (at **)' + + '\n in FancyRow (at **)' + + '\n in table (at **)' + + '\n in Table (at **)' + + '\n in FancyTable (at **)' + + '\n in Viz2 (at **)' : + 'See Viz2 > FancyTable > Table > table > FancyRow > Row > tr.' ); ReactTestUtils.renderIntoDocument(); expectDev(console.error.calls.count()).toBe(3); - expectDev(console.error.calls.argsFor(2)[0]).toContain( - 'See FancyTable > Table > table > FancyRow > Row > tr.' + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(2)[0])).toContain( + ReactDOMFeatureFlags.useFiber ? + '\n in tr (at **)' + + '\n in Row (at **)' + + '\n in FancyRow (at **)' + + '\n in table (at **)' + + '\n in Table (at **)' + + '\n in FancyTable (at **)' : + 'See FancyTable > Table > table > FancyRow > Row > tr.' ); ReactTestUtils.renderIntoDocument(
); expectDev(console.error.calls.count()).toBe(4); - expectDev(console.error.calls.argsFor(3)[0]).toContain( - 'See table > FancyRow > Row > tr.' + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(3)[0])).toContain( + ReactDOMFeatureFlags.useFiber ? + '\n in tr (at **)' + + '\n in Row (at **)' + + '\n in FancyRow (at **)' + + '\n in table (at **)' : + 'See table > FancyRow > Row > tr.' ); ReactTestUtils.renderIntoDocument(); expectDev(console.error.calls.count()).toBe(5); - expectDev(console.error.calls.argsFor(4)[0]).toContain( - 'See FancyTable > Table > table > tr.' + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(4)[0])).toContain( + ReactDOMFeatureFlags.useFiber ? + '\n in tr (at **)' + + '\n in table (at **)' + + '\n in Table (at **)' + + '\n in FancyTable (at **)' : + 'See FancyTable > Table > table > tr.' ); class Link extends React.Component { @@ -1397,8 +1456,14 @@ describe('ReactDOMComponent', () => { ReactTestUtils.renderIntoDocument(

); expectDev(console.error.calls.count()).toBe(6); - expectDev(console.error.calls.argsFor(5)[0]).toContain( - 'See Link > a > ... > Link > a.' + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(5)[0])).toContain( + ReactDOMFeatureFlags.useFiber ? + '\n in a (at **)' + + '\n in Link (at **)' + + '\n in div (at **)' + + '\n in a (at **)' + + '\n in Link (at **)' : + 'See Link > a > ... > Link > a.' ); }); diff --git a/src/renderers/dom/shared/validateDOMNesting.js b/src/renderers/dom/shared/validateDOMNesting.js index 3a732efe70938..8fa3216f70ce0 100644 --- a/src/renderers/dom/shared/validateDOMNesting.js +++ b/src/renderers/dom/shared/validateDOMNesting.js @@ -18,6 +18,8 @@ var warning = require('warning'); var validateDOMNesting = emptyFunction; if (__DEV__) { + var { getCurrentFiberStackAddendum } = require('ReactDebugCurrentFiber'); + // This validation code was written based on the HTML5 parsing spec: // https://html.spec.whatwg.org/multipage/syntax.html#has-an-element-in-scope // @@ -316,6 +318,49 @@ if (__DEV__) { return stack; }; + var getOwnerInfo = function(childInstance, childTag, ancestorInstance, ancestorTag, isParent) { + var childOwner = childInstance && childInstance._currentElement._owner; + var ancestorOwner = ancestorInstance && ancestorInstance._currentElement._owner; + + var childOwners = findOwnerStack(childOwner); + var ancestorOwners = findOwnerStack(ancestorOwner); + + var minStackLen = Math.min(childOwners.length, ancestorOwners.length); + var i; + + var deepestCommon = -1; + for (i = 0; i < minStackLen; i++) { + if (childOwners[i] === ancestorOwners[i]) { + deepestCommon = i; + } else { + break; + } + } + + var UNKNOWN = '(unknown)'; + var childOwnerNames = childOwners.slice(deepestCommon + 1).map( + (inst) => getComponentName(inst) || UNKNOWN + ); + var ancestorOwnerNames = ancestorOwners.slice(deepestCommon + 1).map( + (inst) => getComponentName(inst) || UNKNOWN + ); + var ownerInfo = [].concat( + // If the parent and child instances have a common owner ancestor, start + // with that -- otherwise we just start with the parent's owners. + deepestCommon !== -1 ? + getComponentName(childOwners[deepestCommon]) || UNKNOWN : + [], + ancestorOwnerNames, + ancestorTag, + // If we're warning about an invalid (non-parent) ancestry, add '...' + isParent ? [] : ['...'], + childOwnerNames, + childTag + ).join(' > '); + + return ownerInfo; + }; + var didWarn = {}; validateDOMNesting = function( @@ -340,101 +385,74 @@ if (__DEV__) { isTagValidWithParent(childTag, parentTag) ? null : parentInfo; var invalidAncestor = invalidParent ? null : findInvalidAncestorForTag(childTag, ancestorInfo); - var problematic = invalidParent || invalidAncestor; - - if (problematic) { - var ancestorTag = problematic.tag; - var ancestorInstance = problematic.instance; - - var childOwner = childInstance && childInstance._currentElement._owner; - var ancestorOwner = - ancestorInstance && ancestorInstance._currentElement._owner; + var invalidParentOrAncestor = invalidParent || invalidAncestor; + if (!invalidParentOrAncestor) { + return; + } - var childOwners = findOwnerStack(childOwner); - var ancestorOwners = findOwnerStack(ancestorOwner); + var ancestorInstance = invalidParentOrAncestor.instance; + var ancestorTag = invalidParentOrAncestor.tag; + var addendum; - var minStackLen = Math.min(childOwners.length, ancestorOwners.length); - var i; + if (childInstance != null) { + addendum = ' See ' + getOwnerInfo( + childInstance, + childTag, + ancestorInstance, + ancestorTag, + Boolean(invalidParent) + ) + '.'; + } else { + addendum = getCurrentFiberStackAddendum(); + } - var deepestCommon = -1; - for (i = 0; i < minStackLen; i++) { - if (childOwners[i] === ancestorOwners[i]) { - deepestCommon = i; - } else { - break; - } - } + var warnKey = + !!invalidParent + '|' + childTag + '|' + ancestorTag + '|' + addendum; + if (didWarn[warnKey]) { + return; + } + didWarn[warnKey] = true; - var UNKNOWN = '(unknown)'; - var childOwnerNames = childOwners.slice(deepestCommon + 1).map( - (inst) => getComponentName(inst) || UNKNOWN - ); - var ancestorOwnerNames = ancestorOwners.slice(deepestCommon + 1).map( - (inst) => getComponentName(inst) || UNKNOWN - ); - var ownerInfo = [].concat( - // If the parent and child instances have a common owner ancestor, start - // with that -- otherwise we just start with the parent's owners. - deepestCommon !== -1 ? - getComponentName(childOwners[deepestCommon]) || UNKNOWN : - [], - ancestorOwnerNames, - ancestorTag, - // If we're warning about an invalid (non-parent) ancestry, add '...' - invalidAncestor ? ['...'] : [], - childOwnerNames, - childTag - ).join(' > '); - - var warnKey = - !!invalidParent + '|' + childTag + '|' + ancestorTag + '|' + ownerInfo; - if (didWarn[warnKey]) { - return; - } - didWarn[warnKey] = true; - - var tagDisplayName = childTag; - 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.'; - } + var tagDisplayName = childTag; + var whitespaceInfo = ''; + if (childTag === '#text') { + if (/\S/.test(childText)) { + tagDisplayName = 'Text nodes'; } else { - tagDisplayName = '<' + childTag + '>'; + 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 + '>'; + } - if (invalidParent) { - var info = ''; - if (ancestorTag === 'table' && childTag === 'tr') { - info += - ' Add a to your code to match the DOM tree generated by ' + - 'the browser.'; - } - warning( - false, - 'validateDOMNesting(...): %s cannot appear as a child of <%s>.%s ' + - 'See %s.%s', - tagDisplayName, - ancestorTag, - whitespaceInfo, - ownerInfo, - info - ); - } else { - warning( - false, - 'validateDOMNesting(...): %s cannot appear as a descendant of ' + - '<%s>. See %s.', - tagDisplayName, - ancestorTag, - ownerInfo - ); + if (invalidParent) { + var info = ''; + if (ancestorTag === 'table' && childTag === 'tr') { + info += + ' Add a to your code to match the DOM tree generated by ' + + 'the browser.'; } + warning( + false, + 'validateDOMNesting(...): %s cannot appear as a child of <%s>.%s%s%s', + tagDisplayName, + ancestorTag, + whitespaceInfo, + info, + addendum + ); + } else { + warning( + false, + 'validateDOMNesting(...): %s cannot appear as a descendant of ' + + '<%s>.%s', + tagDisplayName, + ancestorTag, + addendum + ); } }; diff --git a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMOption-test.js b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMOption-test.js index cb7e8fee253c3..8c5e74c74e012 100644 --- a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMOption-test.js +++ b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMOption-test.js @@ -13,13 +13,19 @@ describe('ReactDOMOption', () => { + function normalizeCodeLocInfo(str) { + return str && str.replace(/\(at .+?:\d+\)/g, '(at **)'); + } + var React; var ReactDOM; + var ReactDOMFeatureFlags; var ReactTestUtils; beforeEach(() => { React = require('React'); ReactDOM = require('ReactDOM'); + ReactDOMFeatureFlags = require('ReactDOMFeatureFlags'); ReactTestUtils = require('ReactTestUtils'); }); @@ -33,15 +39,18 @@ describe('ReactDOMOption', () => { it('should ignore and warn invalid children types', () => { spyOn(console, 'error'); - var stub =