Skip to content

Commit

Permalink
Don't mutate passed-in props, take 2
Browse files Browse the repository at this point in the history
  • Loading branch information
sophiebits committed Jan 7, 2014
1 parent 4f57515 commit 6576021
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 6 deletions.
15 changes: 9 additions & 6 deletions src/core/ReactCompositeComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,7 @@ var ReactCompositeComponentMixin = {
this._compositeLifeCycleState = CompositeLifeCycle.MOUNTING;

this._defaultProps = this.getDefaultProps ? this.getDefaultProps() : null;
this._processProps(this.props);
this.props = this._processProps(this.props);

if (this.__reactAutoBindMap) {
this._bindAutoBindMethods();
Expand Down Expand Up @@ -853,12 +853,15 @@ var ReactCompositeComponentMixin = {

/**
* Processes props by setting default values for unspecified props and
* asserting that the props are valid.
* asserting that the props are valid. Does not mutate its argument; returns
* a new props object with defaults merged in.
*
* @param {object} props
* @param {object} newProps
* @return {object}
* @private
*/
_processProps: function(props) {
_processProps: function(newProps) {
var props = merge(newProps);
var defaultProps = this._defaultProps;
for (var propName in defaultProps) {
if (typeof props[propName] === 'undefined') {
Expand All @@ -869,6 +872,7 @@ var ReactCompositeComponentMixin = {
if (propTypes) {
this._checkPropTypes(propTypes, props, ReactPropTypeLocations.prop);
}
return props;
},

/**
Expand Down Expand Up @@ -920,8 +924,7 @@ var ReactCompositeComponentMixin = {

var nextProps = this.props;
if (this._pendingProps != null) {
nextProps = this._pendingProps;
this._processProps(nextProps);
nextProps = this._processProps(this._pendingProps);
this._pendingProps = null;

this._compositeLifeCycleState = CompositeLifeCycle.RECEIVING_PROPS;
Expand Down
41 changes: 41 additions & 0 deletions src/core/__tests__/ReactCompositeComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,47 @@ describe('ReactCompositeComponent', function() {
reactComponentExpect(instance3).scalarPropsEqual({key: null});
});

it('should not mutate passed-in props object', function() {
var Component = React.createClass({
getDefaultProps: function() {
return {key: 'testKey'};
},
render: function() {
return <span />;
}
});

var inputProps = {};
var instance1 = Component(inputProps);
ReactTestUtils.renderIntoDocument(instance1);
expect(instance1.props.key).toBe('testKey');

// We don't mutate the input, just in case the caller wants to do something
// with it after using it to instantiate a component
expect(inputProps.key).not.toBeDefined();
});

it('should use default prop value when removing a key', function() {
var Component = React.createClass({
getDefaultProps: function() {
return {fruit: 'persimmon'};
},
render: function() {
return <span />;
}
});

var container = document.createElement('div');
var instance = React.renderComponent(
<Component fruit="mango" />,
container
);
expect(instance.props.fruit).toBe('mango');

React.renderComponent(<Component />, container);
expect(instance.props.fruit).toBe('persimmon');
});

it('should normalize props with default values', function() {
var Component = React.createClass({
propTypes: {key: ReactPropTypes.string.isRequired},
Expand Down

0 comments on commit 6576021

Please sign in to comment.