From 4bb966a7f08953083316ebacdefcd622c5bde99d Mon Sep 17 00:00:00 2001 From: CommitSyncScript Date: Fri, 7 Jun 2013 22:04:18 -0700 Subject: [PATCH] Bugfixes to key assignment Type coersion bug and ID breaking assumption. Names need to be wrapped in something unique since otherwise two unique siblings can end up having IDs that are subsets of eachother. --- src/core/ReactComponent.js | 2 +- src/core/__tests__/ReactIdentity-test.js | 20 +++++++++---------- .../ReactMultiChildReconcile-test.js | 5 +++-- src/utils/__tests__/mapChildren-test.js | 8 ++++---- src/utils/flattenChildren.js | 4 ++-- 5 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/core/ReactComponent.js b/src/core/ReactComponent.js index e59c37fd69..85bf82115f 100644 --- a/src/core/ReactComponent.js +++ b/src/core/ReactComponent.js @@ -296,7 +296,7 @@ var ReactComponent = { if (child.length === 0) continue; if (targetArray === null) targetArray = []; - appendNestedChildren(i - 1, child, targetArray); + appendNestedChildren('' + (i - 1), child, targetArray); } else if (!isEmptyChild(child)) { diff --git a/src/core/__tests__/ReactIdentity-test.js b/src/core/__tests__/ReactIdentity-test.js index 8e00f1ebb0..71614082b9 100644 --- a/src/core/__tests__/ReactIdentity-test.js +++ b/src/core/__tests__/ReactIdentity-test.js @@ -53,8 +53,8 @@ describe('ReactIdentity', function() { React.renderComponent(instance, document.createElement('div')); var node = instance.getDOMNode(); reactComponentExpect(instance).toBeDOMComponentWithChildCount(2); - checkId(node.childNodes[0], '.reactRoot[0].:0:first'); - checkId(node.childNodes[1], '.reactRoot[0].:0:second'); + checkId(node.childNodes[0], '.reactRoot[0].[0]{first}'); + checkId(node.childNodes[1], '.reactRoot[0].[0]{second}'); }); it('should allow key property to express identity', function() { @@ -67,8 +67,8 @@ describe('ReactIdentity', function() { React.renderComponent(instance, document.createElement('div')); var node = instance.getDOMNode(); reactComponentExpect(instance).toBeDOMComponentWithChildCount(2); - checkId(node.childNodes[0], '.reactRoot[0].:apple'); - checkId(node.childNodes[1], '.reactRoot[0].:banana'); + checkId(node.childNodes[0], '.reactRoot[0].[apple]'); + checkId(node.childNodes[1], '.reactRoot[0].[banana]'); }); it('should use instance identity', function() { @@ -89,12 +89,12 @@ describe('ReactIdentity', function() { React.renderComponent(instance, document.createElement('div')); var node = instance.getDOMNode(); reactComponentExpect(instance).toBeDOMComponentWithChildCount(3); - checkId(node.childNodes[0], '.reactRoot[0].:wrap1'); - checkId(node.childNodes[0].firstChild, '.reactRoot[0].:wrap1.:squirrel'); - checkId(node.childNodes[1], '.reactRoot[0].:wrap2'); - checkId(node.childNodes[1].firstChild, '.reactRoot[0].:wrap2.:bunny'); - checkId(node.childNodes[2], '.reactRoot[0].:2'); - checkId(node.childNodes[2].firstChild, '.reactRoot[0].:2.:chipmunk'); + checkId(node.childNodes[0], '.reactRoot[0].[wrap1]'); + checkId(node.childNodes[0].firstChild, '.reactRoot[0].[wrap1].[squirrel]'); + checkId(node.childNodes[1], '.reactRoot[0].[wrap2]'); + checkId(node.childNodes[1].firstChild, '.reactRoot[0].[wrap2].[bunny]'); + checkId(node.childNodes[2], '.reactRoot[0].[2]'); + checkId(node.childNodes[2].firstChild, '.reactRoot[0].[2].[chipmunk]'); }); it('should let restructured components retain their uniqueness', function() { diff --git a/src/core/__tests__/ReactMultiChildReconcile-test.js b/src/core/__tests__/ReactMultiChildReconcile-test.js index bead6ae660..597ca8f182 100644 --- a/src/core/__tests__/ReactMultiChildReconcile-test.js +++ b/src/core/__tests__/ReactMultiChildReconcile-test.js @@ -42,10 +42,11 @@ var stripEmptyValues = function(obj) { /** * These children are wrapped in an array and therefore their keys are prefixed. - * This relies on a tiny implementation detail of the rendering system. + * Their name is also wrapped in an prefix and suffix character. We strip those + * out. This relies on a tiny implementation detail of the rendering system. */ var getOriginalKey = function(childName) { - return childName.substr(3); + return childName.substr(4, childName.length - 5); }; /** diff --git a/src/utils/__tests__/mapChildren-test.js b/src/utils/__tests__/mapChildren-test.js index 7933e0c211..32ac55bdb6 100644 --- a/src/utils/__tests__/mapChildren-test.js +++ b/src/utils/__tests__/mapChildren-test.js @@ -122,10 +122,10 @@ describe('mapChildren', function() { .instance(); expect(mapFn.calls.length).toBe(3); - expect(mapFn).toHaveBeenCalledWith(kidOne, 'one', 0); - expect(mapFn).toHaveBeenCalledWith(kidTwo, 'two', 1); - expect(mapFn).toHaveBeenCalledWith(kidThree, 'three', 2); + expect(mapFn).toHaveBeenCalledWith(kidOne, '0:one', 0); + expect(mapFn).toHaveBeenCalledWith(kidTwo, '0:two', 1); + expect(mapFn).toHaveBeenCalledWith(kidThree, '0:three', 2); expect(rendered.props.children).not.toEqual(instance.props.children); - expect(rendered.props.children).toHaveKeys(['one', 'two', 'three']); + expect(rendered.props.children).toHaveKeys(['0:one', '0:two', '0:three']); }); }); diff --git a/src/utils/flattenChildren.js b/src/utils/flattenChildren.js index 031b86e588..2728220b19 100644 --- a/src/utils/flattenChildren.js +++ b/src/utils/flattenChildren.js @@ -56,7 +56,7 @@ var flattenChildrenImpl = function(res, children, nameSoFar) { flattenChildrenImpl( res, child, - nameSoFar + ':' + escapedKey + nameSoFar + '[' + escapedKey + ']' ); } } else { @@ -80,7 +80,7 @@ var flattenChildrenImpl = function(res, children, nameSoFar) { flattenChildrenImpl( res, children[key], - nameSoFar + ':' + escapedKey + nameSoFar + '{' + escapedKey + '}' ); } }