From 97307593228bd537c2db3a8be0807cad4fb27033 Mon Sep 17 00:00:00 2001 From: Pete Hunt Date: Tue, 4 Feb 2014 13:00:36 -0800 Subject: [PATCH] Fix cloneWithProps() to allow overriding props This is a clear bug. --- src/utils/__tests__/cloneWithProps-test.js | 17 ++++++++++-- src/utils/cloneWithProps.js | 30 +++++----------------- 2 files changed, 21 insertions(+), 26 deletions(-) diff --git a/src/utils/__tests__/cloneWithProps-test.js b/src/utils/__tests__/cloneWithProps-test.js index b5aba846ae..1d1748d194 100644 --- a/src/utils/__tests__/cloneWithProps-test.js +++ b/src/utils/__tests__/cloneWithProps-test.js @@ -55,7 +55,7 @@ describe('cloneWithProps', function() { }); var component = ReactTestUtils.renderIntoDocument(); expect(component.getDOMNode().childNodes[0].className) - .toBe('child xyz'); + .toBe('xyz child'); }); it('should clone a composite component with new props', function() { @@ -82,7 +82,7 @@ describe('cloneWithProps', function() { }); var component = ReactTestUtils.renderIntoDocument(); expect(component.getDOMNode().childNodes[0].className) - .toBe('child xyz'); + .toBe('xyz child'); }); it('should warn when cloning with refs', function() { @@ -178,4 +178,17 @@ describe('cloneWithProps', function() { ReactTestUtils.renderIntoDocument(); }); + + it('should overwrite props', function() { + var Component = React.createClass({ + render: function() { + expect(this.props.myprop).toBe('xyz'); + return
; + } + }); + + ReactTestUtils.renderIntoDocument( + cloneWithProps(, {myprop: 'xyz'}) + ); + }); }); diff --git a/src/utils/cloneWithProps.js b/src/utils/cloneWithProps.js index 310dd08672..95fee5ffa8 100644 --- a/src/utils/cloneWithProps.js +++ b/src/utils/cloneWithProps.js @@ -21,13 +21,9 @@ var ReactPropTransferer = require('ReactPropTransferer'); -var keyMirror = require('keyMirror'); +var keyOf = require('keyOf'); -var SpecialPropsToTransfer = keyMirror({ - key: null, - children: null, - ref: null -}); +var CHILDREN_PROP = keyOf({children: null}); /** * Sometimes you want to change the props of a child passed to you. Usually @@ -49,28 +45,14 @@ function cloneWithProps(child, props) { } } - var newProps = ReactPropTransferer.mergeProps(child.props, props); + var newProps = ReactPropTransferer.mergeProps(props, child.props); - // ReactPropTransferer does not transfer the `key` prop so do it manually. Do - // not transfer it from the original component. - if (props.hasOwnProperty(SpecialPropsToTransfer.key)) { - newProps.key = props.key; - } - - // ReactPropTransferer does not transfer the `children` prop. Transfer it - // from `props` if it exists, otherwise use `child.props.children` if it is - // provided. - if (props.hasOwnProperty(SpecialPropsToTransfer.children)) { - newProps.children = props.children; - } else if (child.props.hasOwnProperty(SpecialPropsToTransfer.children)) { + // Use `child.props.children` if it is provided. + if (!newProps.hasOwnProperty(CHILDREN_PROP) && + child.props.hasOwnProperty(CHILDREN_PROP)) { newProps.children = child.props.children; } - // ReactPropTransferer does not transfer `ref` so do it manually. - if (props.hasOwnProperty(SpecialPropsToTransfer.ref)) { - newProps.ref = props.ref; - } - return child.constructor.ConvenienceConstructor(newProps); }