ReactChildren.map: only add slash if new child has key
See the new test for the scenario I am trying to fix; if you clone an element in React.cloneElement, vs just returning it directly, you will get a different key (with a slash in front) even though the two children are identical.
Ian Obermiller committed
Jan 20, 2016 at 16:14 UTC
30f7641873127fbf92ac47795e01e93314c1bbe7
2 files changed
+61
-23
src/isomorphic/children/ReactChildren.js
+2
-2
@@ -117,8 +117,8 @@ function mapSingleChildIntoContext(bookKeeping, child, childKey) {
117
// traverseAllChildren used to do for objects as children
118
keyPrefix +
119
(
120
- mappedChild !== child ?
121
- escapeUserProvidedKey(mappedChild.key || '') + '/' :
120
+ (mappedChild.key && (!child || (child.key !== mappedChild.key))) ?
121
+ escapeUserProvidedKey(mappedChild.key) + '/' :
122
''
123
) +
124
childKey
src/isomorphic/children/__tests__/ReactChildren-test.js
+59
-21
@@ -85,7 +85,7 @@ describe('ReactChildren', function() {
85
expect(ReactChildren.count(mappedChildren)).toBe(1);
86
expect(mappedChildren[0]).not.toBe(simpleKid);
87
expect(mappedChildren[0].props.children).toBe(simpleKid);
88
- expect(mappedChildren[0].key).toBe('/.$simple');
88
+ expect(mappedChildren[0].key).toBe('.$simple');
89
});
90
91
it('should invoke callback with the right context', function() {
@@ -117,17 +117,15 @@ describe('ReactChildren', function() {
117
var three = null;
118
var four = <div key="keyFour" />;
119
120
- var zeroMapped = <div key="giraffe" />; // Key should be joined to obj key
121
- var oneMapped = null; // Key should be added even if we don't supply it!
122
- var twoMapped = <div />; // Key should be added even if not supplied!
123
- var threeMapped = <span />; // Map from null to something.
124
- var fourMapped = <div key="keyFour" />;
125
-
120
+ var mapped = [
121
+ <div key="giraffe" />, // Key should be joined to obj key
122
+ null, // Key should be added even if we don't supply it!
123
+ <div />, // Key should be added even if not supplied!
124
+ <span />, // Map from null to something.
125
+ <div key="keyFour" />,
126
+ ];
127
var callback = jasmine.createSpy().andCallFake(function(kid, index) {
127
- return index === 0 ? zeroMapped :
128
- index === 1 ? oneMapped :
129
- index === 2 ? twoMapped :
130
- index === 3 ? threeMapped : fourMapped;
128
+ return mapped[index];
129
});
130
131
var instance = (
@@ -159,7 +157,7 @@ describe('ReactChildren', function() {
157
mappedChildren[2].key,
158
mappedChildren[3].key,
159
]).toEqual(
162
- ['giraffe/.$keyZero', '/.$keyTwo', '/.3', 'keyFour/.$keyFour']
160
+ ['giraffe/.$keyZero', '.$keyTwo', '.3', '.$keyFour']
161
);
162
163
expect(callback).toHaveBeenCalledWith(zero, 0);
@@ -169,9 +167,9 @@ describe('ReactChildren', function() {
167
expect(callback).toHaveBeenCalledWith(four, 4);
168
169
expect(mappedChildren[0]).toEqual(<div key="giraffe/.$keyZero" />);
172
- expect(mappedChildren[1]).toEqual(<div key="/.$keyTwo" />);
173
- expect(mappedChildren[2]).toEqual(<span key="/.3" />);
174
- expect(mappedChildren[3]).toEqual(<div key="keyFour/.$keyFour" />);
170
+ expect(mappedChildren[1]).toEqual(<div key=".$keyTwo" />);
171
+ expect(mappedChildren[2]).toEqual(<span key=".3" />);
172
+ expect(mappedChildren[3]).toEqual(<div key=".$keyFour" />);
173
});
174
175
it('should be called for each child in nested structure', function() {
@@ -241,15 +239,15 @@ describe('ReactChildren', function() {
239
mappedChildren[3].key,
240
]).toEqual([
241
'giraffe/.0:$firstHalfKey/.$keyZero',
244
- '/.0:$firstHalfKey/.$keyTwo',
242
+ '.0:$firstHalfKey/.$keyTwo',
243
'keyFour/.0:$secondHalfKey/.$keyFour',
246
- '/.0:$keyFive/.$keyFiveInner',
244
+ '.0:$keyFive/.$keyFiveInner',
245
]);
246
247
expect(mappedChildren[0]).toEqual(<div key="giraffe/.0:$firstHalfKey/.$keyZero" />);
250
- expect(mappedChildren[1]).toEqual(<div key="/.0:$firstHalfKey/.$keyTwo" />);
248
+ expect(mappedChildren[1]).toEqual(<div key=".0:$firstHalfKey/.$keyTwo" />);
249
expect(mappedChildren[2]).toEqual(<div key="keyFour/.0:$secondHalfKey/.$keyFour" />);
252
- expect(mappedChildren[3]).toEqual(<div key="/.0:$keyFive/.$keyFiveInner" />);
250
+ expect(mappedChildren[3]).toEqual(<div key=".0:$keyFive/.$keyFiveInner" />);
251
});
252
253
it('should retain key across two mappings', function() {
@@ -272,7 +270,7 @@ describe('ReactChildren', function() {
270
</div>
271
);
272
275
- var expectedForcedKeys = ['giraffe/.$keyZero', '/.$keyOne'];
273
+ var expectedForcedKeys = ['giraffe/.$keyZero', '.$keyOne'];
274
var mappedChildrenForcedKeys =
275
ReactChildren.map(forcedKeys.props.children, mapFn);
276
var mappedForcedKeys = mappedChildrenForcedKeys.map((c) => c.key);
@@ -280,7 +278,7 @@ describe('ReactChildren', function() {
278
279
var expectedRemappedForcedKeys = [
280
'giraffe/.$giraffe/.$keyZero',
283
- '/.$/.$keyOne',
281
+ '.$.$keyOne',
282
];
283
var remappedChildrenForcedKeys =
284
ReactChildren.map(mappedChildrenForcedKeys, mapFn);
@@ -310,6 +308,46 @@ describe('ReactChildren', function() {
308
}).not.toThrow();
309
});
310
311
+ it('should use the same key for a cloned element', function() {
312
+ var instance = (
313
+ <div>
314
+ <div />
315
+ </div>
316
+ );
317
+
318
+ var mapped = ReactChildren.map(
319
+ instance.props.children,
320
+ element => element,
321
+ );
322
+
323
+ var mappedWithClone = ReactChildren.map(
324
+ instance.props.children,
325
+ element => React.cloneElement(element),
326
+ );
327
+
328
+ expect(mapped[0].key).toBe(mappedWithClone[0].key);
329
+ });
330
+
331
+ it('should use the same key for a cloned element with key', function() {
332
+ var instance = (
333
+ <div>
334
+ <div key="unique" />
335
+ </div>
336
+ );
337
+
338
+ var mapped = ReactChildren.map(
339
+ instance.props.children,
340
+ element => element,
341
+ );
342
+
343
+ var mappedWithClone = ReactChildren.map(
344
+ instance.props.children,
345
+ element => React.cloneElement(element, {key: 'unique'}),
346
+ );
347
+
348
+ expect(mapped[0].key).toBe(mappedWithClone[0].key);
349
+ });
350
+
351
it('should return 0 for null children', function() {
352
var numberOfChildren = ReactChildren.count(null);
353
expect(numberOfChildren).toBe(0);