From 216fcdeb42cd4d783a89f618ed989010c10506fc Mon Sep 17 00:00:00 2001 From: Ben Alpert Date: Sun, 6 Apr 2014 20:47:09 -0700 Subject: [PATCH] Ignore children with clashing keys Fixes #566. --- src/utils/ReactChildren.js | 21 +++++++++++++-------- src/utils/__tests__/ReactChildren-test.js | 14 ++++++++------ src/utils/flattenChildren.js | 14 ++++++++------ 3 files changed, 29 insertions(+), 20 deletions(-) diff --git a/src/utils/ReactChildren.js b/src/utils/ReactChildren.js index 4786b4dd4fa..49ca0ea2e08 100644 --- a/src/utils/ReactChildren.js +++ b/src/utils/ReactChildren.js @@ -20,8 +20,8 @@ var PooledClass = require('PooledClass'); -var invariant = require('invariant'); var traverseAllChildren = require('traverseAllChildren'); +var warning = require('warning'); var twoArgumentPooler = PooledClass.twoArgumentPooler; var threeArgumentPooler = PooledClass.threeArgumentPooler; @@ -86,16 +86,21 @@ PooledClass.addPoolingTo(MapBookKeeping, threeArgumentPooler); function mapSingleChildIntoContext(traverseContext, child, name, i) { var mapBookKeeping = traverseContext; var mapResult = mapBookKeeping.mapResult; - var mappedChild = - mapBookKeeping.mapFunction.call(mapBookKeeping.mapContext, child, i); - // We found a component instance - invariant( - !mapResult.hasOwnProperty(name), + + var keyUnique = !mapResult.hasOwnProperty(name); + warning( + keyUnique, 'ReactChildren.map(...): Encountered two children with the same key, ' + - '`%s`. Children keys must be unique.', + '`%s`. Child keys must be unique; when two children share a key, only ' + + 'the first child will be used.', name ); - mapResult[name] = mappedChild; + + if (keyUnique) { + var mappedChild = + mapBookKeeping.mapFunction.call(mapBookKeeping.mapContext, child, i); + mapResult[name] = mappedChild; + } } /** diff --git a/src/utils/__tests__/ReactChildren-test.js b/src/utils/__tests__/ReactChildren-test.js index b256d3fdf5f..3fb2cb2fd44 100644 --- a/src/utils/__tests__/ReactChildren-test.js +++ b/src/utils/__tests__/ReactChildren-test.js @@ -320,17 +320,19 @@ describe('ReactChildren', function() { }).not.toThrow(); }); - it('should throw if key provided is a dupe with explicit key', function() { + it('should warn if key provided is a dupe with explicit key', function() { var zero =
; - var one =
; + var one = ; - var mapFn = function() {return null;}; + var mapFn = function(component) { return component; }; var instance = (
{zero}{one}
); - expect(function() { - ReactChildren.map(instance.props.children, mapFn); - }).toThrow(); + spyOn(console, 'warn'); + var mapped = ReactChildren.map(instance.props.children, mapFn); + + expect(console.warn.calls.length).toEqual(1); + expect(mapped).toEqual({'.$something': zero}); }); }); diff --git a/src/utils/flattenChildren.js b/src/utils/flattenChildren.js index 77650b67aa9..fe1a68cd6e0 100644 --- a/src/utils/flattenChildren.js +++ b/src/utils/flattenChildren.js @@ -18,8 +18,8 @@ "use strict"; -var invariant = require('invariant'); var traverseAllChildren = require('traverseAllChildren'); +var warning = require('warning'); /** * @param {function} traverseContext Context passed through traversal. @@ -29,13 +29,15 @@ var traverseAllChildren = require('traverseAllChildren'); function flattenSingleChildIntoContext(traverseContext, child, name) { // We found a component instance. var result = traverseContext; - invariant( - !result.hasOwnProperty(name), - 'flattenChildren(...): Encountered two children with the same key, `%s`. ' + - 'Children keys must be unique.', + var keyUnique = !result.hasOwnProperty(name); + warning( + 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.', name ); - if (child != null) { + if (keyUnique && child != null) { result[name] = child; } }