Skip to content

Commit

Permalink
Merge pull request #5570 from jimfb/remove-setprops-replaceprops
Browse files Browse the repository at this point in the history
Remove setProps and replaceProps.
  • Loading branch information
jimfb committed Dec 3, 2015
2 parents 0be7786 + fbf81a8 commit 50c7b17
Show file tree
Hide file tree
Showing 8 changed files with 20 additions and 214 deletions.
50 changes: 0 additions & 50 deletions src/isomorphic/classic/class/ReactClass.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,18 +54,6 @@ var SpecPolicy = keyMirror({

var injectedMixins = [];

var warnedSetProps = false;
function warnSetProps() {
if (!warnedSetProps) {
warnedSetProps = true;
warning(
false,
'setProps(...) and replaceProps(...) are deprecated. ' +
'Instead, call render again at the top level.'
);
}
}

/**
* Composite components are higher-level components that compose other composite
* or native components.
Expand Down Expand Up @@ -737,44 +725,6 @@ var ReactClassMixin = {
isMounted: function() {
return this.updater.isMounted(this);
},

/**
* Sets a subset of the props.
*
* @param {object} partialProps Subset of the next props.
* @param {?function} callback Called after props are updated.
* @final
* @public
* @deprecated
*/
setProps: function(partialProps, callback) {
if (__DEV__) {
warnSetProps();
}
this.updater.enqueueSetProps(this, partialProps);
if (callback) {
this.updater.enqueueCallback(this, callback);
}
},

/**
* Replace all the props.
*
* @param {object} newProps Subset of the next props.
* @param {?function} callback Called after props are updated.
* @final
* @public
* @deprecated
*/
replaceProps: function(newProps, callback) {
if (__DEV__) {
warnSetProps();
}
this.updater.enqueueReplaceProps(this, newProps);
if (callback) {
this.updater.enqueueCallback(this, callback);
}
},
};

var ReactClassComponent = function() {};
Expand Down
8 changes: 0 additions & 8 deletions src/isomorphic/modern/class/ReactComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,19 +115,11 @@ if (__DEV__) {
'Instead, make sure to clean up subscriptions and pending requests in ' +
'componentWillUnmount to prevent memory leaks.',
],
replaceProps: [
'replaceProps',
'Instead, call render again at the top level.',
],
replaceState: [
'replaceState',
'Refactor your code to use setState instead (see ' +
'https://github.com/facebook/react/issues/3236).',
],
setProps: [
'setProps',
'Instead, call render again at the top level.',
],
};
var defineDeprecationWarning = function(methodName, info) {
if (canDefineProperty) {
Expand Down
23 changes: 0 additions & 23 deletions src/isomorphic/modern/class/ReactNoopUpdateQueue.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,29 +98,6 @@ var ReactNoopUpdateQueue = {
enqueueSetState: function(publicInstance, partialState) {
warnTDZ(publicInstance, 'setState');
},

/**
* Sets a subset of the props.
*
* @param {ReactClass} publicInstance The instance that should rerender.
* @param {object} partialProps Subset of the next props.
* @internal
*/
enqueueSetProps: function(publicInstance, partialProps) {
warnTDZ(publicInstance, 'setProps');
},

/**
* Replaces all of the props.
*
* @param {ReactClass} publicInstance The instance that should rerender.
* @param {object} props New props.
* @internal
*/
enqueueReplaceProps: function(publicInstance, props) {
warnTDZ(publicInstance, 'replaceProps');
},

};

module.exports = ReactNoopUpdateQueue;
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ describe 'ReactCoffeeScriptClass', ->
expect(-> instance.isMounted()).toThrow()
expect(-> instance.setProps name: 'bar').toThrow()
expect(-> instance.replaceProps name: 'bar').toThrow()
expect(console.error.calls.length).toBe 5
expect(console.error.calls.length).toBe 3
expect(console.error.argsForCall[0][0]).toContain(
'getDOMNode(...) is deprecated in plain JavaScript React classes'
)
Expand All @@ -360,12 +360,6 @@ describe 'ReactCoffeeScriptClass', ->
expect(console.error.argsForCall[2][0]).toContain(
'isMounted(...) is deprecated in plain JavaScript React classes'
)
expect(console.error.argsForCall[3][0]).toContain(
'setProps(...) is deprecated in plain JavaScript React classes'
)
expect(console.error.argsForCall[4][0]).toContain(
'replaceProps(...) is deprecated in plain JavaScript React classes'
)

it 'supports this.context passed via getChildContext', ->
class Bar extends React.Component
Expand Down
8 changes: 1 addition & 7 deletions src/isomorphic/modern/class/__tests__/ReactES6Class-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ describe('ReactES6Class', function() {
expect(() => instance.isMounted()).toThrow();
expect(() => instance.setProps({name: 'bar'})).toThrow();
expect(() => instance.replaceProps({name: 'bar'})).toThrow();
expect(console.error.calls.length).toBe(5);
expect(console.error.calls.length).toBe(3);
expect(console.error.argsForCall[0][0]).toContain(
'getDOMNode(...) is deprecated in plain JavaScript React classes. ' +
'Use ReactDOM.findDOMNode(component) instead.'
Expand All @@ -418,12 +418,6 @@ describe('ReactES6Class', function() {
expect(console.error.argsForCall[2][0]).toContain(
'isMounted(...) is deprecated in plain JavaScript React classes'
);
expect(console.error.argsForCall[3][0]).toContain(
'setProps(...) is deprecated in plain JavaScript React classes'
);
expect(console.error.argsForCall[4][0]).toContain(
'replaceProps(...) is deprecated in plain JavaScript React classes'
);
});

it('supports this.context passed via getChildContext', function() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ describe('ReactTypeScriptClass', function() {
expect(() => instance.isMounted()).toThrow();
expect(() => instance.setProps({ name: 'bar' })).toThrow();
expect(() => instance.replaceProps({ name: 'bar' })).toThrow();
expect((<any>console.error).argsForCall.length).toBe(5);
expect((<any>console.error).argsForCall.length).toBe(3);
expect((<any>console.error).argsForCall[0][0]).toContain(
'getDOMNode(...) is deprecated in plain JavaScript React classes'
);
Expand All @@ -501,12 +501,6 @@ describe('ReactTypeScriptClass', function() {
expect((<any>console.error).argsForCall[2][0]).toContain(
'isMounted(...) is deprecated in plain JavaScript React classes'
);
expect((<any>console.error).argsForCall[3][0]).toContain(
'setProps(...) is deprecated in plain JavaScript React classes'
);
expect((<any>console.error).argsForCall[4][0]).toContain(
'replaceProps(...) is deprecated in plain JavaScript React classes'
);
});

it('supports this.context passed via getChildContext', function() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -444,47 +444,6 @@ describe('ReactComponentLifeCycle', function() {
expect(instance.state).toEqual(POST_WILL_UNMOUNT_STATE);
});

it('should throw when calling setProps() on an owned component', function() {
/**
* calls setProps in an componentDidMount.
*/
var Inner = React.createClass({
render: function() {
return <div />;
},
});
var PropsUpdaterInOnDOMReady = React.createClass({
componentDidMount: function() {
this.refs.theSimpleComponent.setProps({
className: this.props.valueToUseInOnDOMReady,
});
},
render: function() {
return (
<Inner
className={this.props.valueToUseInitially}
ref="theSimpleComponent"
/>
);
},
});
var instance =
<PropsUpdaterInOnDOMReady
valueToUseInitially="hello"
valueToUseInOnDOMReady="goodbye"
/>;
spyOn(console, 'error');
expect(function() {
instance = ReactTestUtils.renderIntoDocument(instance);
}).toThrow(
'setProps(...): You called `setProps` on a component with a parent. ' +
'This is an anti-pattern since props will get reactively updated ' +
'when rendered. Instead, change the owner\'s `render` method to pass ' +
'the correct value as props to the component where it is created.'
);
expect(console.error.calls.length).toBe(1); // setProps deprecated
});

it('should not throw when updating an auxiliary component', function() {
var Tooltip = React.createClass({
render: function() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,12 +137,23 @@ describe('ReactCompositeComponent', function() {
});

it('should not cache old DOM nodes when switching constructors', function() {
var instance = <ChildUpdates renderAnchor={true} anchorClassOn={false}/>;
instance = ReactTestUtils.renderIntoDocument(instance);
instance.setProps({anchorClassOn: true}); // Warm any cache
instance.setProps({renderAnchor: false}); // Clear out the anchor
// rerender
instance.setProps({renderAnchor: true, anchorClassOn: false});
var container = document.createElement('div');
var instance = ReactDOM.render(
<ChildUpdates renderAnchor={true} anchorClassOn={false}/>,
container
);
ReactDOM.render( // Warm any cache
<ChildUpdates renderAnchor={true} anchorClassOn={true}/>,
container
);
ReactDOM.render( // Clear out the anchor
<ChildUpdates renderAnchor={false} anchorClassOn={true}/>,
container
);
ReactDOM.render( // rerender
<ChildUpdates renderAnchor={true} anchorClassOn={false}/>,
container
);
expect(instance.getAnchor().className).toBe('');
});

Expand Down Expand Up @@ -398,71 +409,6 @@ describe('ReactCompositeComponent', function() {
expect(instance2.state.value).toBe(1);
});

it('should not allow `setProps` on unmounted components', function() {
var container = document.createElement('div');
document.body.appendChild(container);

var Component = React.createClass({
render: function() {
return <div />;
},
});

var instance = <Component />;
expect(instance.setProps).not.toBeDefined();

instance = ReactDOM.render(instance, container);
expect(function() {
instance.setProps({value: 1});
}).not.toThrow();
expect(console.error.calls.length).toBe(1); // setProps deprecated

ReactDOM.unmountComponentAtNode(container);
expect(function() {
instance.setProps({value: 2});
}).not.toThrow();

expect(console.error.calls.length).toBe(2);
expect(console.error.argsForCall[1][0]).toBe(
'Warning: setProps(...): Can only update a mounted or ' +
'mounting component. This usually means you called setProps() on an ' +
'unmounted component. This is a no-op. Please check the code for the ' +
'Component component.'
);
});

it('should only allow `setProps` on top-level components', function() {
var container = document.createElement('div');
document.body.appendChild(container);

var innerInstance;

var Inner = React.createClass({
render: function() {
return <div />;
},
});
var Component = React.createClass({
render: function() {
return <div><Inner ref="inner" /></div>;
},
componentDidMount: function() {
innerInstance = this.refs.inner;
},
});
ReactDOM.render(<Component />, container);

expect(innerInstance).not.toBe(undefined);
expect(function() {
innerInstance.setProps({value: 1});
}).toThrow(
'setProps(...): You called `setProps` on a component with a parent. ' +
'This is an anti-pattern since props will get reactively updated when ' +
'rendered. Instead, change the owner\'s `render` method to pass the ' +
'correct value as props to the component where it is created.'
);
});

it('should cleanup even if render() fatals', function() {
var BadComponent = React.createClass({
render: function() {
Expand Down

0 comments on commit 50c7b17

Please sign in to comment.