Fix cloneWithProps() to allow overriding props
This is a clear bug.
Pete Hunt committed
Feb 4, 2014 at 13:00 UTC
97307593228bd537c2db3a8be0807cad4fb27033
2 files changed
+21
-26
src/utils/__tests__/cloneWithProps-test.js
+15
-2
@@ -55,7 +55,7 @@ describe('cloneWithProps', function() {
55
});
56
var component = ReactTestUtils.renderIntoDocument(<Grandparent />);
57
expect(component.getDOMNode().childNodes[0].className)
58
- .toBe('child xyz');
58
+ .toBe('xyz child');
59
});
60
61
it('should clone a composite component with new props', function() {
@@ -82,7 +82,7 @@ describe('cloneWithProps', function() {
82
});
83
var component = ReactTestUtils.renderIntoDocument(<Grandparent />);
84
expect(component.getDOMNode().childNodes[0].className)
85
- .toBe('child xyz');
85
+ .toBe('xyz child');
86
});
87
88
it('should warn when cloning with refs', function() {
@@ -178,4 +178,17 @@ describe('cloneWithProps', function() {
178
179
ReactTestUtils.renderIntoDocument(<Grandparent />);
180
});
181
+
182
+ it('should overwrite props', function() {
183
+ var Component = React.createClass({
184
+ render: function() {
185
+ expect(this.props.myprop).toBe('xyz');
186
+ return <div />;
187
+ }
188
+ });
189
+
190
+ ReactTestUtils.renderIntoDocument(
191
+ cloneWithProps(<Component myprop="abc" />, {myprop: 'xyz'})
192
+ );
193
+ });
194
});
src/utils/cloneWithProps.js
+6
-24
@@ -21,13 +21,9 @@
21
22
var ReactPropTransferer = require('ReactPropTransferer');
23
24
-var keyMirror = require('keyMirror');
24
+var keyOf = require('keyOf');
25
26
-var SpecialPropsToTransfer = keyMirror({
27
- key: null,
28
- children: null,
29
- ref: null
30
-});
26
+var CHILDREN_PROP = keyOf({children: null});
27
28
/**
29
* Sometimes you want to change the props of a child passed to you. Usually
@@ -49,28 +45,14 @@ function cloneWithProps(child, props) {
45
}
46
}
47
52
- var newProps = ReactPropTransferer.mergeProps(child.props, props);
48
+ var newProps = ReactPropTransferer.mergeProps(props, child.props);
49
54
- // ReactPropTransferer does not transfer the `key` prop so do it manually. Do
55
- // not transfer it from the original component.
56
- if (props.hasOwnProperty(SpecialPropsToTransfer.key)) {
57
- newProps.key = props.key;
58
- }
59
-
60
- // ReactPropTransferer does not transfer the `children` prop. Transfer it
61
- // from `props` if it exists, otherwise use `child.props.children` if it is
62
- // provided.
63
- if (props.hasOwnProperty(SpecialPropsToTransfer.children)) {
64
- newProps.children = props.children;
65
- } else if (child.props.hasOwnProperty(SpecialPropsToTransfer.children)) {
50
+ // Use `child.props.children` if it is provided.
51
+ if (!newProps.hasOwnProperty(CHILDREN_PROP) &&
52
+ child.props.hasOwnProperty(CHILDREN_PROP)) {
53
newProps.children = child.props.children;
54
}
55
69
- // ReactPropTransferer does not transfer `ref` so do it manually.
70
- if (props.hasOwnProperty(SpecialPropsToTransfer.ref)) {
71
- newProps.ref = props.ref;
72
- }
73
-
56
return child.constructor.ConvenienceConstructor(newProps);
57
}
58