diff --git a/src/addons/transitions/ReactTransitionChildMapping.js b/src/addons/transitions/ReactTransitionChildMapping.js index 28583432403d5..a01bd42526282 100644 --- a/src/addons/transitions/ReactTransitionChildMapping.js +++ b/src/addons/transitions/ReactTransitionChildMapping.js @@ -19,12 +19,18 @@ var ReactTransitionChildMapping = { * simple syntactic sugar around flattenChildren(). * * @param {*} children `this.props.children` + * @param {number=} selfDebugID Optional debugID of the current internal instance. * @return {object} Mapping of key to child */ - getChildMapping: function(children) { + getChildMapping: function(children, selfDebugID) { if (!children) { return children; } + + if (__DEV__) { + return flattenChildren(children, selfDebugID); + } + return flattenChildren(children); }, diff --git a/src/addons/transitions/ReactTransitionGroup.js b/src/addons/transitions/ReactTransitionGroup.js index 33e32f02e48b5..dd87ada1babae 100644 --- a/src/addons/transitions/ReactTransitionGroup.js +++ b/src/addons/transitions/ReactTransitionGroup.js @@ -12,6 +12,7 @@ 'use strict'; var React = require('React'); +var ReactInstanceMap = require('ReactInstanceMap'); var ReactTransitionChildMapping = require('ReactTransitionChildMapping'); var emptyFunction = require('emptyFunction'); @@ -38,6 +39,7 @@ var ReactTransitionGroup = React.createClass({ getInitialState: function() { return { + // TODO: can we get useful debug information to show at this point? children: ReactTransitionChildMapping.getChildMapping(this.props.children), }; }, @@ -58,9 +60,17 @@ var ReactTransitionGroup = React.createClass({ }, componentWillReceiveProps: function(nextProps) { - var nextChildMapping = ReactTransitionChildMapping.getChildMapping( - nextProps.children - ); + var nextChildMapping; + if (__DEV__) { + nextChildMapping = ReactTransitionChildMapping.getChildMapping( + nextProps.children, + ReactInstanceMap.get(this)._debugID + ); + } else { + nextChildMapping = ReactTransitionChildMapping.getChildMapping( + nextProps.children + ); + } var prevChildMapping = this.state.children; this.setState({ @@ -123,9 +133,17 @@ var ReactTransitionGroup = React.createClass({ delete this.currentlyTransitioningKeys[key]; - var currentChildMapping = ReactTransitionChildMapping.getChildMapping( - this.props.children - ); + var currentChildMapping; + if (__DEV__) { + currentChildMapping = ReactTransitionChildMapping.getChildMapping( + this.props.children, + ReactInstanceMap.get(this)._debugID + ); + } else { + currentChildMapping = ReactTransitionChildMapping.getChildMapping( + this.props.children + ); + } if (!currentChildMapping || !currentChildMapping.hasOwnProperty(key)) { // This was removed before it had fully appeared. Remove it. @@ -155,9 +173,17 @@ var ReactTransitionGroup = React.createClass({ delete this.currentlyTransitioningKeys[key]; - var currentChildMapping = ReactTransitionChildMapping.getChildMapping( - this.props.children - ); + var currentChildMapping; + if (__DEV__) { + currentChildMapping = ReactTransitionChildMapping.getChildMapping( + this.props.children, + ReactInstanceMap.get(this)._debugID + ); + } else { + currentChildMapping = ReactTransitionChildMapping.getChildMapping( + this.props.children + ); + } if (!currentChildMapping || !currentChildMapping.hasOwnProperty(key)) { // This was removed before it had fully entered. Remove it. @@ -188,9 +214,17 @@ var ReactTransitionGroup = React.createClass({ delete this.currentlyTransitioningKeys[key]; - var currentChildMapping = ReactTransitionChildMapping.getChildMapping( - this.props.children - ); + var currentChildMapping; + if (__DEV__) { + currentChildMapping = ReactTransitionChildMapping.getChildMapping( + this.props.children, + ReactInstanceMap.get(this)._debugID + ); + } else { + currentChildMapping = ReactTransitionChildMapping.getChildMapping( + this.props.children + ); + } if (currentChildMapping && currentChildMapping.hasOwnProperty(key)) { // This entered again before it fully left. Add it again. diff --git a/src/addons/transitions/__tests__/ReactTransitionGroup-test.js b/src/addons/transitions/__tests__/ReactTransitionGroup-test.js index f98302be10c6e..9617c57f4bbca 100644 --- a/src/addons/transitions/__tests__/ReactTransitionGroup-test.js +++ b/src/addons/transitions/__tests__/ReactTransitionGroup-test.js @@ -20,6 +20,10 @@ var ReactTransitionGroup; describe('ReactTransitionGroup', function() { var container; + function normalizeCodeLocInfo(str) { + return str.replace(/\(at .+?:\d+\)/g, '(at **)'); + } + beforeEach(function() { React = require('React'); ReactDOM = require('ReactDOM'); @@ -269,4 +273,33 @@ describe('ReactTransitionGroup', function() { 'willLeave2', 'didLeave2', 'willUnmount0', 'willUnmount1', 'willUnmount2', ]); }); + + it('should warn for duplicated keys with component stack info', function() { + spyOn(console, 'error'); + + var Component = React.createClass({ + render: function() { + var children = [
,
]; + return {children}; + }, + }); + + ReactDOM.render(, container); + + expect(console.error.argsForCall.length).toBe(2); + expect(console.error.argsForCall[0][0]).toBe( + 'Warning: flattenChildren(...): ' + + 'Encountered two children with the same key, `1`. ' + + 'Child keys must be unique; when two children share a key, ' + + 'only the first child will be used.' + ); + expect(normalizeCodeLocInfo(console.error.argsForCall[1][0])).toBe( + 'Warning: flattenChildren(...): ' + + 'Encountered two children with the same key, `1`. ' + + 'Child keys must be unique; when two children share a key, ' + + 'only the first child will be used.\n' + + ' in ReactTransitionGroup (at **)\n' + + ' in Component (at **)' + ); + }); }); diff --git a/src/isomorphic/classic/element/ReactElementValidator.js b/src/isomorphic/classic/element/ReactElementValidator.js index 9467160708914..bd04c8f122f7b 100644 --- a/src/isomorphic/classic/element/ReactElementValidator.js +++ b/src/isomorphic/classic/element/ReactElementValidator.js @@ -48,11 +48,25 @@ var ownerHasKeyUseWarning = {}; var loggedTypeFailures = {}; +function getCurrentComponentErrorInfo(parentType) { + var info = getDeclarationErrorAddendum(); + + if (!info) { + var parentName = typeof parentType === 'string' ? + parentType : parentType.displayName || parentType.name; + if (parentName) { + info = ` Check the top-level render call using <${parentName}>.`; + } + } + return info; +} + /** * Warn if the element doesn't have an explicit key assigned to it. * This element is in an array. The array could grow and shrink or be * reordered. All children that haven't already been validated are required to - * have a "key" property assigned to it. + * have a "key" property assigned to it. Error statuses are cached so a warning + * will only be shown once. * * @internal * @param {ReactElement} element Element that requires a key. @@ -64,67 +78,36 @@ function validateExplicitKey(element, parentType) { } element._store.validated = true; - var addenda = getAddendaForKeyUse('uniqueKey', element, parentType); - if (addenda === null) { - // we already showed the warning - return; - } - warning( - false, - 'Each child in an array or iterator should have a unique "key" prop.' + - '%s%s%s', - addenda.parentOrOwner || '', - addenda.childOwner || '', - addenda.url || '' + var memoizer = ownerHasKeyUseWarning.uniqueKey || ( + ownerHasKeyUseWarning.uniqueKey = {} ); -} - -/** - * Shared warning and monitoring code for the key warnings. - * - * @internal - * @param {string} messageType A key used for de-duping warnings. - * @param {ReactElement} element Component that requires a key. - * @param {*} parentType element's parent's type. - * @returns {?object} A set of addenda to use in the warning message, or null - * if the warning has already been shown before (and shouldn't be shown again). - */ -function getAddendaForKeyUse(messageType, element, parentType) { - var addendum = getDeclarationErrorAddendum(); - if (!addendum) { - var parentName = typeof parentType === 'string' ? - parentType : parentType.displayName || parentType.name; - if (parentName) { - addendum = ` Check the top-level render call using <${parentName}>.`; - } - } - var memoizer = ownerHasKeyUseWarning[messageType] || ( - ownerHasKeyUseWarning[messageType] = {} - ); - if (memoizer[addendum]) { - return null; + var currentComponentErrorInfo = getCurrentComponentErrorInfo(parentType); + if (memoizer[currentComponentErrorInfo]) { + return; } - memoizer[addendum] = true; - - var addenda = { - parentOrOwner: addendum, - url: ' See https://fb.me/react-warning-keys for more information.', - childOwner: null, - }; + memoizer[currentComponentErrorInfo] = true; // Usually the current owner is the offender, but if it accepts children as a // property, it may be the creator of the child that's responsible for // assigning it a key. + var childOwner = ''; if (element && element._owner && element._owner !== ReactCurrentOwner.current) { // Give the component that originally created this child. - addenda.childOwner = + childOwner = ` It was passed a child from ${element._owner.getName()}.`; } - return addenda; + warning( + false, + 'Each child in an array or iterator should have a unique "key" prop.' + + '%s%s See https://fb.me/react-warning-keys for more information.%s', + currentComponentErrorInfo, + childOwner, + ReactComponentTreeDevtool.getCurrentStackAddendum(element) + ); } /** diff --git a/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js b/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js index 64c7a4fc1ef68..5f640d5e96fd7 100644 --- a/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js +++ b/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js @@ -19,6 +19,10 @@ var ReactDOM; var ReactTestUtils; describe('ReactElementValidator', function() { + function normalizeCodeLocInfo(str) { + return str.replace(/\(at .+?:\d+\)/g, '(at **)'); + } + var ComponentClass; beforeEach(function() { @@ -95,9 +99,10 @@ describe('ReactElementValidator', function() { ReactTestUtils.renderIntoDocument({divs}); expect(console.error.argsForCall.length).toBe(1); - expect(console.error.argsForCall[0][0]).toBe( + expect(normalizeCodeLocInfo(console.error.argsForCall[0][0])).toBe( 'Warning: Each child in an array or iterator should have a unique ' + - '"key" prop. See https://fb.me/react-warning-keys for more information.' + '"key" prop. See https://fb.me/react-warning-keys for more information.\n' + + ' in div (at **)' ); }); @@ -111,10 +116,46 @@ describe('ReactElementValidator', function() { ReactTestUtils.renderIntoDocument(
{divs}
); expect(console.error.argsForCall.length).toBe(1); - expect(console.error.argsForCall[0][0]).toBe( + expect(normalizeCodeLocInfo(console.error.argsForCall[0][0])).toBe( 'Warning: Each child in an array or iterator should have a unique ' + '"key" prop. Check the top-level render call using
. See ' + - 'https://fb.me/react-warning-keys for more information.' + 'https://fb.me/react-warning-keys for more information.\n' + + ' in div (at **)' + ); + }); + + it('warns for keys with component stack info', function() { + spyOn(console, 'error'); + + var Component = React.createClass({ + render: function() { + return
{[
,
]}
; + }, + }); + + var Parent = React.createClass({ + render: function() { + return React.cloneElement(this.props.child); + }, + }); + + var GrandParent = React.createClass({ + render: function() { + return } />; + }, + }); + + ReactTestUtils.renderIntoDocument(); + + expect(console.error.argsForCall.length).toBe(1); + expect(normalizeCodeLocInfo(console.error.argsForCall[0][0])).toBe( + 'Warning: Each child in an array or iterator should have a unique ' + + '"key" prop. Check the render method of `Component`. See ' + + 'https://fb.me/react-warning-keys for more information.\n' + + ' in div (at **)\n' + + ' in Component (at **)\n' + + ' in Parent (at **)\n' + + ' in GrandParent (at **)' ); }); diff --git a/src/renderers/shared/devtools/ReactComponentTreeDevtool.js b/src/renderers/shared/devtools/ReactComponentTreeDevtool.js index 25d537e2e829b..30bcd881bd053 100644 --- a/src/renderers/shared/devtools/ReactComponentTreeDevtool.js +++ b/src/renderers/shared/devtools/ReactComponentTreeDevtool.js @@ -48,6 +48,28 @@ function purgeDeep(id) { } } +function describeComponentFrame(name, source, ownerName) { + return '\n in ' + name + ( + source ? + ' (at ' + source.fileName.replace(/^.*[\\\/]/, '') + ':' + + source.lineNumber + ')' : + ownerName ? + ' (created by ' + ownerName + ')' : + '' + ); +} + +function describeID(id) { + var name = ReactComponentTreeDevtool.getDisplayName(id); + var element = ReactComponentTreeDevtool.getElement(id); + var ownerID = ReactComponentTreeDevtool.getOwnerID(id); + var ownerName; + if (ownerID) { + ownerName = ReactComponentTreeDevtool.getDisplayName(ownerID); + } + return describeComponentFrame(name, element._source, ownerName); +} + var ReactComponentTreeDevtool = { onSetDisplayName(id, displayName) { updateTree(id, item => item.displayName = displayName); @@ -154,28 +176,6 @@ var ReactComponentTreeDevtool = { }, getCurrentStackAddendum(topElement) { - function describeComponentFrame(name, source, ownerName) { - return '\n in ' + name + ( - source ? - ' (at ' + source.fileName.replace(/^.*[\\\/]/, '') + ':' + - source.lineNumber + ')' : - ownerName ? - ' (created by ' + ownerName + ')' : - '' - ); - } - - function describeID(id) { - var name = ReactComponentTreeDevtool.getDisplayName(id); - var element = ReactComponentTreeDevtool.getElement(id); - var ownerID = ReactComponentTreeDevtool.getOwnerID(id); - var ownerName; - if (ownerID) { - ownerName = ReactComponentTreeDevtool.getDisplayName(ownerID); - } - return describeComponentFrame(name, element._source, ownerName); - } - var info = ''; if (topElement) { var type = topElement.type; @@ -192,11 +192,17 @@ var ReactComponentTreeDevtool = { var currentOwner = ReactCurrentOwner.current; var id = currentOwner && currentOwner._debugID; + + info += ReactComponentTreeDevtool.getStackAddendumByID(id); + return info; + }, + + getStackAddendumByID(id) { + var info = ''; while (id) { info += describeID(id); id = ReactComponentTreeDevtool.getParentID(id); } - return info; }, diff --git a/src/renderers/shared/stack/reconciler/ReactChildReconciler.js b/src/renderers/shared/stack/reconciler/ReactChildReconciler.js index 98a075f4615c5..ee1ce8217a161 100644 --- a/src/renderers/shared/stack/reconciler/ReactChildReconciler.js +++ b/src/renderers/shared/stack/reconciler/ReactChildReconciler.js @@ -13,13 +13,14 @@ var ReactReconciler = require('ReactReconciler'); +var ReactComponentTreeDevtool = require('ReactComponentTreeDevtool'); var instantiateReactComponent = require('instantiateReactComponent'); var KeyEscapeUtils = require('KeyEscapeUtils'); var shouldUpdateReactComponent = require('shouldUpdateReactComponent'); var traverseAllChildren = require('traverseAllChildren'); var warning = require('warning'); -function instantiateChild(childInstances, child, name) { +function instantiateChild(childInstances, child, name, selfDebugID) { // We found a component instance. var keyUnique = (childInstances[name] === undefined); if (__DEV__) { @@ -27,8 +28,9 @@ function instantiateChild(childInstances, child, name) { keyUnique, 'flattenChildren(...): Encountered two children with the same key, ' + '`%s`. Child keys must be unique; when two children share a key, only ' + - 'the first child will be used.', - KeyEscapeUtils.unescape(name) + 'the first child will be used.%s', + KeyEscapeUtils.unescape(name), + ReactComponentTreeDevtool.getStackAddendumByID(selfDebugID) ); } if (child != null && keyUnique) { @@ -50,12 +52,31 @@ var ReactChildReconciler = { * @return {?object} A set of child instances. * @internal */ - instantiateChildren: function(nestedChildNodes, transaction, context) { + instantiateChildren: function( + nestedChildNodes, + transaction, + context, + selfDebugID // __DEV__ only + ) { if (nestedChildNodes == null) { return null; } var childInstances = {}; - traverseAllChildren(nestedChildNodes, instantiateChild, childInstances); + + if (__DEV__) { + traverseAllChildren( + nestedChildNodes, + (childInsts, child, name) => instantiateChild( + childInsts, + child, + name, + selfDebugID + ), + childInstances + ); + } else { + traverseAllChildren(nestedChildNodes, instantiateChild, childInstances); + } return childInstances; }, diff --git a/src/renderers/shared/stack/reconciler/ReactMultiChild.js b/src/renderers/shared/stack/reconciler/ReactMultiChild.js index 8b4c95f0c2bf9..01014e80f92ce 100644 --- a/src/renderers/shared/stack/reconciler/ReactMultiChild.js +++ b/src/renderers/shared/stack/reconciler/ReactMultiChild.js @@ -178,7 +178,7 @@ var ReactMultiChild = { try { ReactCurrentOwner.current = this._currentElement._owner; return ReactChildReconciler.instantiateChildren( - nestedChildren, transaction, context + nestedChildren, transaction, context, this._debugID ); } finally { ReactCurrentOwner.current = null; @@ -202,7 +202,7 @@ var ReactMultiChild = { if (this._currentElement) { try { ReactCurrentOwner.current = this._currentElement._owner; - nextChildren = flattenChildren(nextNestedChildrenElements); + nextChildren = flattenChildren(nextNestedChildrenElements, this._debugID); } finally { ReactCurrentOwner.current = null; } diff --git a/src/renderers/shared/stack/reconciler/__tests__/ReactChildReconciler-test.js b/src/renderers/shared/stack/reconciler/__tests__/ReactChildReconciler-test.js new file mode 100644 index 0000000000000..3b4078823bb1f --- /dev/null +++ b/src/renderers/shared/stack/reconciler/__tests__/ReactChildReconciler-test.js @@ -0,0 +1,84 @@ +/** + * Copyright 2013-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + * @emails react-core + */ + +// NOTE: We're explicitly not using JSX here. This is intended to test +// the current stack addendum without having source location added by babel. + +'use strict'; + +var React; +var ReactTestUtils; + +describe('ReactChildReconciler', function() { + function normalizeCodeLocInfo(str) { + return str.replace(/\(at .+?:\d+\)/g, '(at **)'); + } + + beforeEach(function() { + jest.resetModuleRegistry(); + + React = require('React'); + ReactTestUtils = require('ReactTestUtils'); + }); + + it('warns for duplicated keys', function() { + spyOn(console, 'error'); + + var Component = React.createClass({ + render() { + return
{[
,
]}
; + }, + }); + + ReactTestUtils.renderIntoDocument(); + + expect(console.error.argsForCall.length).toBe(1); + expect(console.error.argsForCall[0][0]).toContain( + 'Child keys must be unique; when two children share a key, only the first child will be used.' + ); + }); + + it('warns for duplicated keys with component stack info', function() { + spyOn(console, 'error'); + + var Component = React.createClass({ + render: function() { + return
{[
,
]}
; + }, + }); + + var Parent = React.createClass({ + render: function() { + return React.cloneElement(this.props.child); + }, + }); + + var GrandParent = React.createClass({ + render: function() { + return } />; + }, + }); + + ReactTestUtils.renderIntoDocument(); + + expect(console.error.argsForCall.length).toBe(1); + expect(normalizeCodeLocInfo(console.error.argsForCall[0][0])).toBe( + 'Warning: flattenChildren(...): ' + + 'Encountered two children with the same key, `1`. ' + + 'Child keys must be unique; when two children share a key, ' + + 'only the first child will be used.\n' + + ' in div (at **)\n' + + ' in Component (at **)\n' + + ' in Parent (at **)\n' + + ' in GrandParent (at **)' + ); + }); +}); diff --git a/src/renderers/shared/stack/reconciler/__tests__/ReactMultiChild-test.js b/src/renderers/shared/stack/reconciler/__tests__/ReactMultiChild-test.js index 079eec06d515d..71063cb33c77d 100644 --- a/src/renderers/shared/stack/reconciler/__tests__/ReactMultiChild-test.js +++ b/src/renderers/shared/stack/reconciler/__tests__/ReactMultiChild-test.js @@ -12,8 +12,11 @@ 'use strict'; describe('ReactMultiChild', function() { - var React; + function normalizeCodeLocInfo(str) { + return str.replace(/\(at .+?:\d+\)/g, '(at **)'); + } + var React; var ReactDOM; beforeEach(function() { @@ -148,5 +151,51 @@ describe('ReactMultiChild', function() { expect(mockMount.mock.calls.length).toBe(2); expect(mockUnmount.mock.calls.length).toBe(1); }); + + it('should warn for duplicated keys with component stack info', function() { + spyOn(console, 'error'); + + var container = document.createElement('div'); + + var WrapperComponent = React.createClass({ + render: function() { + return
{this.props.children}
; + }, + }); + + var Parent = React.createClass({ + render: function() { + return ( +
+ + {this.props.children} + +
+ ); + }, + }); + + ReactDOM.render( + {[
]}, + container + ); + + ReactDOM.render( + {[
,
]}, + container + ); + + expect(console.error.argsForCall.length).toBe(1); + expect(normalizeCodeLocInfo(console.error.argsForCall[0][0])).toBe( + 'Warning: flattenChildren(...): ' + + 'Encountered two children with the same key, `1`. ' + + 'Child keys must be unique; when two children share a key, ' + + 'only the first child will be used.\n' + + ' in div (at **)\n' + + ' in WrapperComponent (at **)\n' + + ' in div (at **)\n' + + ' in Parent (at **)' + ); + }); }); }); diff --git a/src/shared/utils/flattenChildren.js b/src/shared/utils/flattenChildren.js index 771134edf8776..b529f25c90ff6 100644 --- a/src/shared/utils/flattenChildren.js +++ b/src/shared/utils/flattenChildren.js @@ -11,6 +11,7 @@ 'use strict'; +var ReactComponentTreeDevtool = require('ReactComponentTreeDevtool'); var KeyEscapeUtils = require('KeyEscapeUtils'); var traverseAllChildren = require('traverseAllChildren'); var warning = require('warning'); @@ -19,8 +20,9 @@ var warning = require('warning'); * @param {function} traverseContext Context passed through traversal. * @param {?ReactComponent} child React child component. * @param {!string} name String name of key path to child. + * @param {number=} selfDebugID Optional debugID of the current internal instance. */ -function flattenSingleChildIntoContext(traverseContext, child, name) { +function flattenSingleChildIntoContext(traverseContext, child, name, selfDebugID) { // We found a component instance. var result = traverseContext; var keyUnique = (result[name] === undefined); @@ -29,8 +31,9 @@ function flattenSingleChildIntoContext(traverseContext, child, name) { keyUnique, 'flattenChildren(...): Encountered two children with the same key, ' + '`%s`. Child keys must be unique; when two children share a key, only ' + - 'the first child will be used.', - KeyEscapeUtils.unescape(name) + 'the first child will be used.%s', + KeyEscapeUtils.unescape(name), + ReactComponentTreeDevtool.getStackAddendumByID(selfDebugID) ); } if (keyUnique && child != null) { @@ -43,12 +46,26 @@ function flattenSingleChildIntoContext(traverseContext, child, name) { * children will not be included in the resulting object. * @return {!object} flattened children keyed by name. */ -function flattenChildren(children) { +function flattenChildren(children, selfDebugID) { if (children == null) { return children; } var result = {}; - traverseAllChildren(children, flattenSingleChildIntoContext, result); + + if (__DEV__) { + traverseAllChildren( + children, + (traverseContext, child, name) => flattenSingleChildIntoContext( + traverseContext, + child, + name, + selfDebugID + ), + result + ); + } else { + traverseAllChildren(children, flattenSingleChildIntoContext, result); + } return result; }