From 4cbc4b58f6c10a2557b0da4dc519c932f167efe3 Mon Sep 17 00:00:00 2001 From: Pete Hunt Date: Thu, 30 Jan 2014 14:50:34 -0800 Subject: [PATCH] Support `children` and `ref` for `cloneWithProps()` We're not handling these correctly. --- src/core/ReactPropTransferer.js | 2 + src/utils/__tests__/cloneWithProps-test.js | 52 ++++++++++++++++++++++ src/utils/cloneWithProps.js | 29 +++++++++++- 3 files changed, 81 insertions(+), 2 deletions(-) diff --git a/src/core/ReactPropTransferer.js b/src/core/ReactPropTransferer.js index ed721a3c9b..77d83da6a2 100644 --- a/src/core/ReactPropTransferer.js +++ b/src/core/ReactPropTransferer.js @@ -42,6 +42,8 @@ function createTransferStrategy(mergeStrategy) { /** * Transfer strategies dictate how props are transferred by `transferPropsTo`. + * NOTE: if you add any more exceptions to this list you should be sure to + * update `cloneWithProps()` accordingly. */ var TransferStrategies = { /** diff --git a/src/utils/__tests__/cloneWithProps-test.js b/src/utils/__tests__/cloneWithProps-test.js index 774b61de1e..b5aba846ae 100644 --- a/src/utils/__tests__/cloneWithProps-test.js +++ b/src/utils/__tests__/cloneWithProps-test.js @@ -126,4 +126,56 @@ describe('cloneWithProps', function() { cloneWithProps(, {key: 'xyz'}) ); }); + + it('should transfer children', function() { + var Component = React.createClass({ + render: function() { + expect(this.props.children).toBe('xyz'); + return
; + } + }); + + ReactTestUtils.renderIntoDocument( + cloneWithProps(, {children: 'xyz'}) + ); + }); + + it('should shallow clone children', function() { + var Component = React.createClass({ + render: function() { + expect(this.props.children).toBe('xyz'); + return
; + } + }); + + ReactTestUtils.renderIntoDocument( + cloneWithProps(xyz, {}) + ); + }); + + it('should support keys and refs', function() { + var Component = React.createClass({ + render: function() { + expect(this.props.key).toBe('xyz'); + expect(this.props.ref).toBe('xyz'); + return
; + } + }); + + var Parent = React.createClass({ + render: function() { + var clone = + cloneWithProps(this.props.children, {key: 'xyz', ref: 'xyz'}); + return
{clone}
; + } + }); + + var Grandparent = React.createClass({ + render: function() { + return ; + } + }); + + ReactTestUtils.renderIntoDocument(); + }); }); diff --git a/src/utils/cloneWithProps.js b/src/utils/cloneWithProps.js index 28c3b4540d..310dd08672 100644 --- a/src/utils/cloneWithProps.js +++ b/src/utils/cloneWithProps.js @@ -21,6 +21,14 @@ var ReactPropTransferer = require('ReactPropTransferer'); +var keyMirror = require('keyMirror'); + +var SpecialPropsToTransfer = keyMirror({ + key: null, + children: null, + ref: null +}); + /** * Sometimes you want to change the props of a child passed to you. Usually * this is to add a CSS class. @@ -42,10 +50,27 @@ function cloneWithProps(child, props) { } var newProps = ReactPropTransferer.mergeProps(child.props, props); - // ReactPropTransferer does not transfer the `key` prop so do it manually. - if (props.key) { + + // 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)) { + 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); }