Merge pull request #5892 from ianobermiller/children-map-key-slash

ReactChildren.map: only add slash if new child has key

src/isomorphic/children/ReactChildren.js

@@ -117,8 +117,8 @@ function mapSingleChildIntoContext(bookKeeping, child, childKey) {
         // traverseAllChildren used to do for objects as children
         keyPrefix +
         (
-          mappedChild !== child ?
-          escapeUserProvidedKey(mappedChild.key || '') + '/' :
+          (mappedChild.key && (!child || (child.key !== mappedChild.key))) ?
+          escapeUserProvidedKey(mappedChild.key) + '/' :
           ''
         ) +
         childKey

src/isomorphic/children/__tests__/ReactChildren-test.js

@@ -85,7 +85,7 @@ describe('ReactChildren', function() {
     expect(ReactChildren.count(mappedChildren)).toBe(1);
     expect(mappedChildren[0]).not.toBe(simpleKid);
     expect(mappedChildren[0].props.children).toBe(simpleKid);
-    expect(mappedChildren[0].key).toBe('/.$simple');
+    expect(mappedChildren[0].key).toBe('.$simple');
   });
 
   it('should invoke callback with the right context', function() {
@@ -117,17 +117,15 @@ describe('ReactChildren', function() {
     var three = null;
     var four = <div key="keyFour" />;
 
-    var zeroMapped = <div key="giraffe" />;  // Key should be joined to obj key
-    var oneMapped = null;  // Key should be added even if we don't supply it!
-    var twoMapped = <div />;  // Key should be added even if not supplied!
-    var threeMapped = <span />; // Map from null to something.
-    var fourMapped = <div key="keyFour" />;
-
+    var mapped = [
+      <div key="giraffe" />,  // Key should be joined to obj key
+      null,  // Key should be added even if we don't supply it!
+      <div />,  // Key should be added even if not supplied!
+      <span />, // Map from null to something.
+      <div key="keyFour" />,
+    ];
     var callback = jasmine.createSpy().andCallFake(function(kid, index) {
-      return index === 0 ? zeroMapped :
-        index === 1 ? oneMapped :
-        index === 2 ? twoMapped :
-        index === 3 ? threeMapped : fourMapped;
+      return mapped[index];
     });
 
     var instance = (
@@ -159,7 +157,7 @@ describe('ReactChildren', function() {
       mappedChildren[2].key,
       mappedChildren[3].key,
     ]).toEqual(
-      ['giraffe/.$keyZero', '/.$keyTwo', '/.3', 'keyFour/.$keyFour']
+      ['giraffe/.$keyZero', '.$keyTwo', '.3', '.$keyFour']
     );
 
     expect(callback).toHaveBeenCalledWith(zero, 0);
@@ -169,9 +167,9 @@ describe('ReactChildren', function() {
     expect(callback).toHaveBeenCalledWith(four, 4);
 
     expect(mappedChildren[0]).toEqual(<div key="giraffe/.$keyZero" />);
-    expect(mappedChildren[1]).toEqual(<div key="/.$keyTwo" />);
-    expect(mappedChildren[2]).toEqual(<span key="/.3" />);
-    expect(mappedChildren[3]).toEqual(<div key="keyFour/.$keyFour" />);
+    expect(mappedChildren[1]).toEqual(<div key=".$keyTwo" />);
+    expect(mappedChildren[2]).toEqual(<span key=".3" />);
+    expect(mappedChildren[3]).toEqual(<div key=".$keyFour" />);
   });
 
   it('should be called for each child in nested structure', function() {
@@ -241,15 +239,15 @@ describe('ReactChildren', function() {
       mappedChildren[3].key,
     ]).toEqual([
       'giraffe/.0:$firstHalfKey/.$keyZero',
-      '/.0:$firstHalfKey/.$keyTwo',
+      '.0:$firstHalfKey/.$keyTwo',
       'keyFour/.0:$secondHalfKey/.$keyFour',
-      '/.0:$keyFive/.$keyFiveInner',
+      '.0:$keyFive/.$keyFiveInner',
     ]);
 
     expect(mappedChildren[0]).toEqual(<div key="giraffe/.0:$firstHalfKey/.$keyZero" />);
-    expect(mappedChildren[1]).toEqual(<div key="/.0:$firstHalfKey/.$keyTwo" />);
+    expect(mappedChildren[1]).toEqual(<div key=".0:$firstHalfKey/.$keyTwo" />);
     expect(mappedChildren[2]).toEqual(<div key="keyFour/.0:$secondHalfKey/.$keyFour" />);
-    expect(mappedChildren[3]).toEqual(<div key="/.0:$keyFive/.$keyFiveInner" />);
+    expect(mappedChildren[3]).toEqual(<div key=".0:$keyFive/.$keyFiveInner" />);
   });
 
   it('should retain key across two mappings', function() {
@@ -272,15 +270,15 @@ describe('ReactChildren', function() {
       </div>
     );
 
-    var expectedForcedKeys = ['giraffe/.$keyZero', '/.$keyOne'];
+    var expectedForcedKeys = ['giraffe/.$keyZero', '.$keyOne'];
     var mappedChildrenForcedKeys =
       ReactChildren.map(forcedKeys.props.children, mapFn);
     var mappedForcedKeys = mappedChildrenForcedKeys.map((c) => c.key);
     expect(mappedForcedKeys).toEqual(expectedForcedKeys);
 
     var expectedRemappedForcedKeys = [
       'giraffe/.$giraffe/.$keyZero',
-      '/.$/.$keyOne',
+      '.$.$keyOne',
     ];
     var remappedChildrenForcedKeys =
       ReactChildren.map(mappedChildrenForcedKeys, mapFn);
@@ -310,6 +308,46 @@ describe('ReactChildren', function() {
     }).not.toThrow();
   });
 
+  it('should use the same key for a cloned element', function() {
+    var instance = (
+      <div>
+        <div />
+      </div>
+    );
+
+    var mapped = ReactChildren.map(
+      instance.props.children,
+      element => element,
+    );
+
+    var mappedWithClone = ReactChildren.map(
+      instance.props.children,
+      element => React.cloneElement(element),
+    );
+
+    expect(mapped[0].key).toBe(mappedWithClone[0].key);
+  });
+
+  it('should use the same key for a cloned element with key', function() {
+    var instance = (
+      <div>
+        <div key="unique" />
+      </div>
+    );
+
+    var mapped = ReactChildren.map(
+      instance.props.children,
+      element => element,
+    );
+
+    var mappedWithClone = ReactChildren.map(
+      instance.props.children,
+      element => React.cloneElement(element, {key: 'unique'}),
+    );
+
+    expect(mapped[0].key).toBe(mappedWithClone[0].key);
+  });
+
   it('should return 0 for null children', function() {
     var numberOfChildren = ReactChildren.count(null);
     expect(numberOfChildren).toBe(0);