Add component stack info to key validation warnings

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);
   },
 

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.

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 = [<div key="1"/>, <div key="1" />];
+        return <ReactTransitionGroup>{children}</ReactTransitionGroup>;
+      },
+    });
+
+    ReactDOM.render(<Component />, 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 **)'
+    );
+  });
 });

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)
+  );
 }
 
 /**

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(<Anonymous>{divs}</Anonymous>);
 
     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(<div>{divs}</div>);
 
     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 <div>. 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 <div>{[<div />, <div />]}</div>;
+      },
+    });
+
+    var Parent = React.createClass({
+      render: function() {
+        return React.cloneElement(this.props.child);
+      },
+    });
+
+    var GrandParent = React.createClass({
+      render: function() {
+        return <Parent child={<Component />} />;
+      },
+    });
+
+    ReactTestUtils.renderIntoDocument(<GrandParent />);
+
+    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 **)'
     );
   });