From 9d4e2ffce0fb49e9f357c3eea09da42cfbd26bb2 Mon Sep 17 00:00:00 2001
From: Keyan Zhang <keyanz@fb.com>
Date: Wed, 18 May 2016 14:08:04 -0700
Subject: [PATCH 1/8] Added component stack info to key validation warnings

---
 .../classic/element/ReactElementValidator.js  |  5 +-
 .../__tests__/ReactElementValidator-test.js   | 58 ++++++++++--
 .../stack/reconciler/ReactChildReconciler.js  |  6 +-
 .../__tests__/ReactChildReconciler-test.js    | 91 +++++++++++++++++++
 .../__tests__/ReactMultiChild-test.js         | 55 +++++++++++
 src/shared/utils/flattenChildren.js           |  6 +-
 6 files changed, 208 insertions(+), 13 deletions(-)
 create mode 100644 src/renderers/shared/stack/reconciler/__tests__/ReactChildReconciler-test.js

diff --git a/src/isomorphic/classic/element/ReactElementValidator.js b/src/isomorphic/classic/element/ReactElementValidator.js
index 9467160708914..6f196e386910f 100644
--- a/src/isomorphic/classic/element/ReactElementValidator.js
+++ b/src/isomorphic/classic/element/ReactElementValidator.js
@@ -72,10 +72,11 @@ function validateExplicitKey(element, parentType) {
   warning(
     false,
     'Each child in an array or iterator should have a unique "key" prop.' +
-    '%s%s%s',
+    '%s%s%s%s',
     addenda.parentOrOwner || '',
     addenda.childOwner || '',
-    addenda.url || ''
+    addenda.url || '',
+    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..2dfb27c8ace88 100644
--- a/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js
+++ b/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js
@@ -84,20 +84,21 @@ describe('ReactElementValidator', function() {
     var Anonymous = React.createClass({
       displayName: undefined,
       render: function() {
-        return <div />;
+        return React.createElement('div');
       },
     });
 
     var divs = [
-      <div />,
-      <div />,
+      React.createElement('div'),
+      React.createElement('div'),
     ];
     ReactTestUtils.renderIntoDocument(<Anonymous>{divs}</Anonymous>);
 
     expect(console.error.argsForCall.length).toBe(1);
     expect(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'
     );
   });
 
@@ -105,8 +106,8 @@ describe('ReactElementValidator', function() {
     spyOn(console, 'error');
 
     var divs = [
-      <div />,
-      <div />,
+      React.createElement('div'),
+      React.createElement('div'),
     ];
     ReactTestUtils.renderIntoDocument(<div>{divs}</div>);
 
@@ -114,7 +115,50 @@ describe('ReactElementValidator', function() {
     expect(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'
+    );
+  });
+
+  it('warns for keys with component stack info', function() {
+    spyOn(console, 'error');
+
+    var Component = React.createClass({
+      render: function() {
+        // <div>{[<div />, <div />]}</div>
+        return React.createElement('div', null, [
+          React.createElement('div'),
+          React.createElement('div'),
+        ]);
+      },
+    });
+
+    var Parent = React.createClass({
+      render: function() {
+        return React.cloneElement(this.props.child);
+      },
+    });
+
+    var GrandParent = React.createClass({
+      render: function() {
+        // <Parent child={<Component />} />
+        return React.createElement(Parent, {
+          child: React.createElement(Component),
+        });
+      },
+    });
+
+    ReactTestUtils.renderIntoDocument(React.createElement(GrandParent));
+
+    expect(console.error.argsForCall.length).toBe(1);
+    expect(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 (created by Component)\n' +
+      '    in Component (created by GrandParent)\n' +
+      '    in Parent (created by GrandParent)\n' +
+      '    in GrandParent'
     );
   });
 
diff --git a/src/renderers/shared/stack/reconciler/ReactChildReconciler.js b/src/renderers/shared/stack/reconciler/ReactChildReconciler.js
index 98a075f4615c5..f32d62115c4b4 100644
--- a/src/renderers/shared/stack/reconciler/ReactChildReconciler.js
+++ b/src/renderers/shared/stack/reconciler/ReactChildReconciler.js
@@ -13,6 +13,7 @@
 
 var ReactReconciler = require('ReactReconciler');
 
+var ReactComponentTreeDevtool = require('ReactComponentTreeDevtool');
 var instantiateReactComponent = require('instantiateReactComponent');
 var KeyEscapeUtils = require('KeyEscapeUtils');
 var shouldUpdateReactComponent = require('shouldUpdateReactComponent');
@@ -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.getCurrentStackAddendum(childInstances[name])
     );
   }
   if (child != null && keyUnique) {
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..15e787173f084
--- /dev/null
+++ b/src/renderers/shared/stack/reconciler/__tests__/ReactChildReconciler-test.js
@@ -0,0 +1,91 @@
+/**
+ * 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() {
+  beforeEach(function() {
+    jest.resetModuleRegistry();
+
+    React = require('React');
+    ReactTestUtils = require('ReactTestUtils');
+  });
+
+  it('warns for duplicated keys', function() {
+    spyOn(console, 'error');
+
+    var Component = React.createClass({
+      render() {
+        // <div>{[<div key="1" />, <div key="1" />]}</div>
+        return React.createElement('div', null, [
+          React.createElement('div', { key: '1' }),
+          React.createElement('div', { key: '1' }),
+        ]);
+      },
+    });
+
+    ReactTestUtils.renderIntoDocument(<Component />);
+
+    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() {
+        // <div>{[<div key="1" />, <div key="1" />]}</div>
+        return React.createElement('div', null, [
+          React.createElement('div', { key: '1' }),
+          React.createElement('div', { key: '1' }),
+        ]);
+      },
+    });
+
+    var Parent = React.createClass({
+      render: function() {
+        return React.cloneElement(this.props.child);
+      },
+    });
+
+    var GrandParent = React.createClass({
+      render: function() {
+        // <Parent child={<Component />} />
+        return React.createElement(Parent, {
+          child: React.createElement(Component),
+        });
+      },
+    });
+
+    ReactTestUtils.renderIntoDocument(React.createElement(GrandParent));
+
+    expect(console.error.argsForCall.length).toBe(1);
+    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.\n' +
+      '    in Unknown\n' + // cuz we are using `ReactTestUtils.renderIntoDocument`
+      '    in Component (created by GrandParent)\n' +
+      '    in Parent (created by GrandParent)\n' +
+      '    in GrandParent'
+    );
+  });
+});
diff --git a/src/renderers/shared/stack/reconciler/__tests__/ReactMultiChild-test.js b/src/renderers/shared/stack/reconciler/__tests__/ReactMultiChild-test.js
index 079eec06d515d..5af0b2410d52b 100644
--- a/src/renderers/shared/stack/reconciler/__tests__/ReactMultiChild-test.js
+++ b/src/renderers/shared/stack/reconciler/__tests__/ReactMultiChild-test.js
@@ -148,5 +148,60 @@ 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() {
+      // NOTE: We're explicitly not using JSX here. This is intended to test
+      // the current stack addendum without having source location added by babel.
+      spyOn(console, 'error');
+
+      var container = document.createElement('div');
+
+      var WrapperComponent = React.createClass({
+        render: function() {
+          // <div>{this.props.children}</div>
+          return React.createElement('div', null, this.props.children);
+        },
+      });
+
+      var Parent = React.createClass({
+        render: function() {
+          // <div><WrapperComponent>{this.props.children}</WrapperComponent></div>
+          return React.createElement(
+            'div',
+            null,
+            React.createElement(WrapperComponent, null, this.props.children)
+          );
+        },
+      });
+
+      ReactDOM.render(
+        // <Parent>{[<div key="1"/>]}</Parent>,
+        React.createElement(Parent, null, [
+          React.createElement('div', { key: '1' }),
+        ]),
+        container
+      );
+
+      ReactDOM.render(
+        // <Parent>{[<div key="1"/>, <div key="1"/>]}</Parent>,
+        React.createElement(Parent, null, [
+          React.createElement('div', { key: '1' }),
+          React.createElement('div', { key: '1' }),
+        ]),
+        container
+      );
+
+      expect(console.error.argsForCall.length).toBe(1);
+      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.\n' +
+        '    in div\n' +
+        '    in WrapperComponent (created by Parent)\n' +
+        '    in div (created by Parent)\n' +
+        '    in Parent'
+      );
+    });
   });
 });
diff --git a/src/shared/utils/flattenChildren.js b/src/shared/utils/flattenChildren.js
index 771134edf8776..7c0f0542b977f 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');
@@ -29,8 +30,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.getCurrentStackAddendum(result[name])
     );
   }
   if (keyUnique && child != null) {

From 53d8ca3cbb004349c07002973baa6e26a1f5d40a Mon Sep 17 00:00:00 2001
From: Keyan Zhang <keyanz@fb.com>
Date: Thu, 19 May 2016 14:22:32 -0700
Subject: [PATCH 2/8] Switch tests back to JSX and normalize code location info
 in tests

---
 .../__tests__/ReactElementValidator-test.js   | 45 +++++++++----------
 .../__tests__/ReactChildReconciler-test.js    |  2 +-
 .../__tests__/ReactMultiChild-test.js         | 42 ++++++++---------
 3 files changed, 40 insertions(+), 49 deletions(-)

diff --git a/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js b/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js
index 2dfb27c8ace88..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() {
@@ -84,21 +88,21 @@ describe('ReactElementValidator', function() {
     var Anonymous = React.createClass({
       displayName: undefined,
       render: function() {
-        return React.createElement('div');
+        return <div />;
       },
     });
 
     var divs = [
-      React.createElement('div'),
-      React.createElement('div'),
+      <div />,
+      <div />,
     ];
     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.\n' +
-      '    in div'
+      '    in div (at **)'
     );
   });
 
@@ -106,17 +110,17 @@ describe('ReactElementValidator', function() {
     spyOn(console, 'error');
 
     var divs = [
-      React.createElement('div'),
-      React.createElement('div'),
+      <div />,
+      <div />,
     ];
     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.\n' +
-      '    in div'
+      '    in div (at **)'
     );
   });
 
@@ -125,11 +129,7 @@ describe('ReactElementValidator', function() {
 
     var Component = React.createClass({
       render: function() {
-        // <div>{[<div />, <div />]}</div>
-        return React.createElement('div', null, [
-          React.createElement('div'),
-          React.createElement('div'),
-        ]);
+        return <div>{[<div />, <div />]}</div>;
       },
     });
 
@@ -141,24 +141,21 @@ describe('ReactElementValidator', function() {
 
     var GrandParent = React.createClass({
       render: function() {
-        // <Parent child={<Component />} />
-        return React.createElement(Parent, {
-          child: React.createElement(Component),
-        });
+        return <Parent child={<Component />} />;
       },
     });
 
-    ReactTestUtils.renderIntoDocument(React.createElement(GrandParent));
+    ReactTestUtils.renderIntoDocument(<GrandParent />);
 
     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 render method of `Component`. See ' +
       'https://fb.me/react-warning-keys for more information.\n' +
-      '    in div (created by Component)\n' +
-      '    in Component (created by GrandParent)\n' +
-      '    in Parent (created by GrandParent)\n' +
-      '    in GrandParent'
+      '    in div (at **)\n' +
+      '    in Component (at **)\n' +
+      '    in Parent (at **)\n' +
+      '    in GrandParent (at **)'
     );
   });
 
diff --git a/src/renderers/shared/stack/reconciler/__tests__/ReactChildReconciler-test.js b/src/renderers/shared/stack/reconciler/__tests__/ReactChildReconciler-test.js
index 15e787173f084..3a099a6d34740 100644
--- a/src/renderers/shared/stack/reconciler/__tests__/ReactChildReconciler-test.js
+++ b/src/renderers/shared/stack/reconciler/__tests__/ReactChildReconciler-test.js
@@ -82,7 +82,7 @@ describe('ReactChildReconciler', function() {
       '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 Unknown\n' + // cuz we are using `ReactTestUtils.renderIntoDocument`
+      '    in div (created by Component)\n' +
       '    in Component (created by GrandParent)\n' +
       '    in Parent (created by GrandParent)\n' +
       '    in GrandParent'
diff --git a/src/renderers/shared/stack/reconciler/__tests__/ReactMultiChild-test.js b/src/renderers/shared/stack/reconciler/__tests__/ReactMultiChild-test.js
index 5af0b2410d52b..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() {
@@ -150,57 +153,48 @@ describe('ReactMultiChild', function() {
     });
 
     it('should warn for duplicated keys with component stack info', function() {
-      // NOTE: We're explicitly not using JSX here. This is intended to test
-      // the current stack addendum without having source location added by babel.
       spyOn(console, 'error');
 
       var container = document.createElement('div');
 
       var WrapperComponent = React.createClass({
         render: function() {
-          // <div>{this.props.children}</div>
-          return React.createElement('div', null, this.props.children);
+          return <div>{this.props.children}</div>;
         },
       });
 
       var Parent = React.createClass({
         render: function() {
-          // <div><WrapperComponent>{this.props.children}</WrapperComponent></div>
-          return React.createElement(
-            'div',
-            null,
-            React.createElement(WrapperComponent, null, this.props.children)
+          return (
+            <div>
+              <WrapperComponent>
+                {this.props.children}
+              </WrapperComponent>
+            </div>
           );
         },
       });
 
       ReactDOM.render(
-        // <Parent>{[<div key="1"/>]}</Parent>,
-        React.createElement(Parent, null, [
-          React.createElement('div', { key: '1' }),
-        ]),
+        <Parent>{[<div key="1"/>]}</Parent>,
         container
       );
 
       ReactDOM.render(
-        // <Parent>{[<div key="1"/>, <div key="1"/>]}</Parent>,
-        React.createElement(Parent, null, [
-          React.createElement('div', { key: '1' }),
-          React.createElement('div', { key: '1' }),
-        ]),
+        <Parent>{[<div key="1"/>, <div key="1"/>]}</Parent>,
         container
       );
 
       expect(console.error.argsForCall.length).toBe(1);
-      expect(console.error.argsForCall[0][0]).toBe(
+      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\n' +
-        '    in WrapperComponent (created by Parent)\n' +
-        '    in div (created by Parent)\n' +
-        '    in Parent'
+        '    in div (at **)\n' +
+        '    in WrapperComponent (at **)\n' +
+        '    in div (at **)\n' +
+        '    in Parent (at **)'
       );
     });
   });

From 3f112afa2d622efc97b0b704f135d72ad8293f71 Mon Sep 17 00:00:00 2001
From: Keyan Zhang <keyanz@fb.com>
Date: Thu, 19 May 2016 14:24:15 -0700
Subject: [PATCH 3/8] add ReactComponentTreeDevtool.getStackAddendumByID

---
 .../devtools/ReactComponentTreeDevtool.js     | 52 +++++++++++--------
 1 file changed, 29 insertions(+), 23 deletions(-)

diff --git a/src/isomorphic/devtools/ReactComponentTreeDevtool.js b/src/isomorphic/devtools/ReactComponentTreeDevtool.js
index 25d537e2e829b..30bcd881bd053 100644
--- a/src/isomorphic/devtools/ReactComponentTreeDevtool.js
+++ b/src/isomorphic/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;
   },
 

From af4a32aa342efdba91fc93af42fc1ad8fd8f6ff2 Mon Sep 17 00:00:00 2001
From: Keyan Zhang <keyanz@fb.com>
Date: Thu, 19 May 2016 14:41:21 -0700
Subject: [PATCH 4/8] Let ReactChildReconciler use debugID

---
 .../stack/reconciler/ReactChildReconciler.js  | 27 ++++++++++++++++---
 .../stack/reconciler/ReactMultiChild.js       |  2 +-
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/src/renderers/shared/stack/reconciler/ReactChildReconciler.js b/src/renderers/shared/stack/reconciler/ReactChildReconciler.js
index f32d62115c4b4..ee1ce8217a161 100644
--- a/src/renderers/shared/stack/reconciler/ReactChildReconciler.js
+++ b/src/renderers/shared/stack/reconciler/ReactChildReconciler.js
@@ -20,7 +20,7 @@ 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__) {
@@ -30,7 +30,7 @@ function instantiateChild(childInstances, child, name) {
       '`%s`. Child keys must be unique; when two children share a key, only ' +
       'the first child will be used.%s',
       KeyEscapeUtils.unescape(name),
-      ReactComponentTreeDevtool.getCurrentStackAddendum(childInstances[name])
+      ReactComponentTreeDevtool.getStackAddendumByID(selfDebugID)
     );
   }
   if (child != null && keyUnique) {
@@ -52,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..23f8cde6bb0e8 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;

From 14dbe15e297c882acdaa94fa83a073c6b00d19b5 Mon Sep 17 00:00:00 2001
From: Keyan Zhang <keyanz@fb.com>
Date: Thu, 19 May 2016 15:53:11 -0700
Subject: [PATCH 5/8] Let flattenChildren use debugID

---
 .../ReactTransitionChildMapping.js            |  3 ++
 .../stack/reconciler/ReactMultiChild.js       |  2 +-
 .../__tests__/ReactChildReconciler-test.js    | 33 ++++++++-----------
 src/shared/utils/flattenChildren.js           | 22 ++++++++++---
 4 files changed, 35 insertions(+), 25 deletions(-)

diff --git a/src/addons/transitions/ReactTransitionChildMapping.js b/src/addons/transitions/ReactTransitionChildMapping.js
index 28583432403d5..a31111ff762db 100644
--- a/src/addons/transitions/ReactTransitionChildMapping.js
+++ b/src/addons/transitions/ReactTransitionChildMapping.js
@@ -25,6 +25,9 @@ var ReactTransitionChildMapping = {
     if (!children) {
       return children;
     }
+    // TODO: `flattenChildren` now takes an extra `selfDebugID` argument
+    // for looking up the component stack in dev build.
+    // Need to figure out a way to get the `_debugID` here.
     return flattenChildren(children);
   },
 
diff --git a/src/renderers/shared/stack/reconciler/ReactMultiChild.js b/src/renderers/shared/stack/reconciler/ReactMultiChild.js
index 23f8cde6bb0e8..01014e80f92ce 100644
--- a/src/renderers/shared/stack/reconciler/ReactMultiChild.js
+++ b/src/renderers/shared/stack/reconciler/ReactMultiChild.js
@@ -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
index 3a099a6d34740..3b4078823bb1f 100644
--- a/src/renderers/shared/stack/reconciler/__tests__/ReactChildReconciler-test.js
+++ b/src/renderers/shared/stack/reconciler/__tests__/ReactChildReconciler-test.js
@@ -18,6 +18,10 @@ var React;
 var ReactTestUtils;
 
 describe('ReactChildReconciler', function() {
+  function normalizeCodeLocInfo(str) {
+    return str.replace(/\(at .+?:\d+\)/g, '(at **)');
+  }
+
   beforeEach(function() {
     jest.resetModuleRegistry();
 
@@ -30,11 +34,7 @@ describe('ReactChildReconciler', function() {
 
     var Component = React.createClass({
       render() {
-        // <div>{[<div key="1" />, <div key="1" />]}</div>
-        return React.createElement('div', null, [
-          React.createElement('div', { key: '1' }),
-          React.createElement('div', { key: '1' }),
-        ]);
+        return <div>{[<div key="1" />, <div key="1" />]}</div>;
       },
     });
 
@@ -51,11 +51,7 @@ describe('ReactChildReconciler', function() {
 
     var Component = React.createClass({
       render: function() {
-        // <div>{[<div key="1" />, <div key="1" />]}</div>
-        return React.createElement('div', null, [
-          React.createElement('div', { key: '1' }),
-          React.createElement('div', { key: '1' }),
-        ]);
+        return <div>{[<div key="1" />, <div key="1" />]}</div>;
       },
     });
 
@@ -67,25 +63,22 @@ describe('ReactChildReconciler', function() {
 
     var GrandParent = React.createClass({
       render: function() {
-        // <Parent child={<Component />} />
-        return React.createElement(Parent, {
-          child: React.createElement(Component),
-        });
+        return <Parent child={<Component />} />;
       },
     });
 
-    ReactTestUtils.renderIntoDocument(React.createElement(GrandParent));
+    ReactTestUtils.renderIntoDocument(<GrandParent />);
 
     expect(console.error.argsForCall.length).toBe(1);
-    expect(console.error.argsForCall[0][0]).toBe(
+    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 (created by Component)\n' +
-      '    in Component (created by GrandParent)\n' +
-      '    in Parent (created by GrandParent)\n' +
-      '    in GrandParent'
+      '    in div (at **)\n' +
+      '    in Component (at **)\n' +
+      '    in Parent (at **)\n' +
+      '    in GrandParent (at **)'
     );
   });
 });
diff --git a/src/shared/utils/flattenChildren.js b/src/shared/utils/flattenChildren.js
index 7c0f0542b977f..de50072580702 100644
--- a/src/shared/utils/flattenChildren.js
+++ b/src/shared/utils/flattenChildren.js
@@ -21,7 +21,7 @@ var warning = require('warning');
  * @param {?ReactComponent} child React child component.
  * @param {!string} name String name of key path to child.
  */
-function flattenSingleChildIntoContext(traverseContext, child, name) {
+function flattenSingleChildIntoContext(traverseContext, child, name, selfDebugID) {
   // We found a component instance.
   var result = traverseContext;
   var keyUnique = (result[name] === undefined);
@@ -32,7 +32,7 @@ function flattenSingleChildIntoContext(traverseContext, child, name) {
       '`%s`. Child keys must be unique; when two children share a key, only ' +
       'the first child will be used.%s',
       KeyEscapeUtils.unescape(name),
-      ReactComponentTreeDevtool.getCurrentStackAddendum(result[name])
+      ReactComponentTreeDevtool.getStackAddendumByID(selfDebugID)
     );
   }
   if (keyUnique && child != null) {
@@ -45,12 +45,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;
 }
 

From 570fe383d44b1533cd2bf366312b9de5cf9c5481 Mon Sep 17 00:00:00 2001
From: Keyan Zhang <keyanz@fb.com>
Date: Thu, 19 May 2016 18:11:24 -0700
Subject: [PATCH 6/8] Refactored ReactElementValidator

---
 .../classic/element/ReactElementValidator.js  | 80 +++++++------------
 1 file changed, 31 insertions(+), 49 deletions(-)

diff --git a/src/isomorphic/classic/element/ReactElementValidator.js b/src/isomorphic/classic/element/ReactElementValidator.js
index 6f196e386910f..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,68 +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%s',
-    addenda.parentOrOwner || '',
-    addenda.childOwner || '',
-    addenda.url || '',
-    ReactComponentTreeDevtool.getCurrentStackAddendum(element)
+  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)
+  );
 }
 
 /**

From 7274283d05698185a47d2aaa080ad53ef4a327d1 Mon Sep 17 00:00:00 2001
From: Keyan Zhang <keyanz@fb.com>
Date: Thu, 19 May 2016 18:32:58 -0700
Subject: [PATCH 7/8] added a jsdoc annotation

---
 src/shared/utils/flattenChildren.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/shared/utils/flattenChildren.js b/src/shared/utils/flattenChildren.js
index de50072580702..b529f25c90ff6 100644
--- a/src/shared/utils/flattenChildren.js
+++ b/src/shared/utils/flattenChildren.js
@@ -20,6 +20,7 @@ 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, selfDebugID) {
   // We found a component instance.

From 1a0f6ca9fb8b2061b3eff0390d00204122286cf1 Mon Sep 17 00:00:00 2001
From: Keyan Zhang <keyanz@fb.com>
Date: Fri, 20 May 2016 13:56:06 -0700
Subject: [PATCH 8/8] Feed `debugID` to
 `ReactTransitionChildMapping.getChildMapping`

---
 .../ReactTransitionChildMapping.js            | 11 ++--
 .../transitions/ReactTransitionGroup.js       | 58 +++++++++++++++----
 .../__tests__/ReactTransitionGroup-test.js    | 33 +++++++++++
 3 files changed, 86 insertions(+), 16 deletions(-)

diff --git a/src/addons/transitions/ReactTransitionChildMapping.js b/src/addons/transitions/ReactTransitionChildMapping.js
index a31111ff762db..a01bd42526282 100644
--- a/src/addons/transitions/ReactTransitionChildMapping.js
+++ b/src/addons/transitions/ReactTransitionChildMapping.js
@@ -19,15 +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;
     }
-    // TODO: `flattenChildren` now takes an extra `selfDebugID` argument
-    // for looking up the component stack in dev build.
-    // Need to figure out a way to get the `_debugID` here.
+
+    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 = [<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 **)'
+    );
+  });
 });