Skip to content

Store pending state updates directly in this.state #629

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Store pending state updates directly in this.state #629

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 14 additions & 19 deletions src/core/ReactCompositeComponent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -580,8 +580,8 @@ var ReactCompositeComponentMixin = {
// Children can be either an array or more than one argument
ReactComponent.Mixin.construct.apply(this, arguments);

this.currentState = null;
this.state = null;
this._pendingState = null;

this.context = this._processContext(ReactContext.current);
this._currentContext = ReactContext.current;
Expand Down Expand Up @@ -630,18 +630,15 @@ var ReactCompositeComponentMixin = {
this._bindAutoBindMethods();
}

this.state = this.getInitialState ? this.getInitialState() : null;
this._pendingState = null;
this.currentState = this.getInitialState ? this.getInitialState() : null;
this.state = this.currentState;
this._pendingForceUpdate = false;

if (this.componentWillMount) {
this.componentWillMount();
// When mounting, calls to `setState` by `componentWillMount` will set
// `this._pendingState` without triggering a re-render.
if (this._pendingState) {
this.state = this._pendingState;
this._pendingState = null;
}
// `this.state` without triggering a re-render.
this.currentState = this.state;
}

this._renderedComponent = this._renderValidatedComponent();
Expand Down Expand Up @@ -707,9 +704,8 @@ var ReactCompositeComponentMixin = {
* @protected
*/
setState: function(partialState, callback) {
// Merge with `_pendingState` if it exists, otherwise with existing state.
this.replaceState(
merge(this._pendingState || this.state, partialState),
merge(this.state, partialState),
callback
);
},
Expand All @@ -728,7 +724,7 @@ var ReactCompositeComponentMixin = {
*/
replaceState: function(completeState, callback) {
validateLifeCycleOnReplaceState(this);
this._pendingState = completeState;
this.state = completeState;
ReactUpdates.enqueueUpdate(this, callback);
},

Expand Down Expand Up @@ -848,15 +844,15 @@ var ReactCompositeComponentMixin = {
},

/**
* If any of `_pendingProps`, `_pendingState`, or `_pendingForceUpdate` is
* If any of `_pendingProps`, pending `state`, or `_pendingForceUpdate` is
* set, update the component.
*
* @param {ReactReconcileTransaction} transaction
* @internal
*/
_performUpdateIfNecessary: function(transaction) {
if (this._pendingProps == null &&
this._pendingState == null &&
this.state === this.currentState &&
this._pendingContext == null &&
!this._pendingForceUpdate) {
return;
Expand Down Expand Up @@ -885,14 +881,13 @@ var ReactCompositeComponentMixin = {
// mean that there's no owner change pending.
var nextOwner = this._pendingOwner;

var nextState = this._pendingState || this.state;
this._pendingState = null;
var nextState = this.state;

if (this._pendingForceUpdate ||
!this.shouldComponentUpdate ||
this.shouldComponentUpdate(nextProps, nextState, nextContext)) {
this._pendingForceUpdate = false;
// Will set `this.props`, `this.state` and `this.context`.
// Will set `this.props`, `this.currentState` and `this.context`.
this._performComponentUpdate(
nextProps,
nextOwner,
Expand All @@ -906,7 +901,7 @@ var ReactCompositeComponentMixin = {
// to set props and state.
this.props = nextProps;
this._owner = nextOwner;
this.state = nextState;
this.currentState = nextState;
this._currentContext = nextFullContext;
this.context = nextContext;
}
Expand Down Expand Up @@ -936,7 +931,7 @@ var ReactCompositeComponentMixin = {
) {
var prevProps = this.props;
var prevOwner = this._owner;
var prevState = this.state;
var prevState = this.currentState;
var prevContext = this.context;

if (this.componentWillUpdate) {
Expand All @@ -945,7 +940,7 @@ var ReactCompositeComponentMixin = {

this.props = nextProps;
this._owner = nextOwner;
this.state = nextState;
this.currentState = nextState;
this._currentContext = nextFullContext;
this.context = nextContext;

Expand Down
46 changes: 23 additions & 23 deletions src/core/__tests__/ReactCompositeComponentState-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ describe('ReactCompositeComponent-state', function() {
} else {
this.props.stateListener(
from,
this.state && this.state.color,
this._pendingState && this._pendingState.color
this.currentState && this.currentState.color,
this.state && this.state.color
);
}
},
Expand Down Expand Up @@ -118,52 +118,52 @@ describe('ReactCompositeComponent-state', function() {
expect(stateListener.mock.calls).toEqual([
// there is no state when getInitialState() is called
[ 'getInitialState', null, null ],
[ 'componentWillMount-start', 'red', null ],
[ 'componentWillMount-start', 'red', 'red' ],
// setState() only enqueues a pending state.
[ 'componentWillMount-after-sunrise', 'red', 'sunrise' ],
[ 'componentWillMount-end', 'red', 'orange' ],
// pending state has been applied
[ 'render', 'orange', null ],
[ 'componentDidMount-start', 'orange', null ],
[ 'render', 'orange', 'orange' ],
[ 'componentDidMount-start', 'orange', 'orange' ],
// componentDidMount() called setState({color:'yellow'}), currently this
// occurs inline.
// In a future where setState() is async, this test result will change.
[ 'shouldComponentUpdate-currentState', 'orange', null ],
[ 'shouldComponentUpdate-currentState', 'orange', 'yellow' ],
[ 'shouldComponentUpdate-nextState', 'yellow' ],
[ 'componentWillUpdate-currentState', 'orange', null ],
[ 'componentWillUpdate-currentState', 'orange', 'yellow' ],
[ 'componentWillUpdate-nextState', 'yellow' ],
[ 'render', 'yellow', null ],
[ 'componentDidUpdate-currentState', 'yellow', null ],
[ 'render', 'yellow', 'yellow' ],
[ 'componentDidUpdate-currentState', 'yellow', 'yellow' ],
[ 'componentDidUpdate-prevState', 'orange' ],
// componentDidMount() finally closes.
[ 'componentDidMount-end', 'yellow', null ],
[ 'componentWillReceiveProps-start', 'yellow', null ],
[ 'componentDidMount-end', 'yellow', 'yellow' ],
[ 'componentWillReceiveProps-start', 'yellow', 'yellow' ],
// setState({color:'green'}) only enqueues a pending state.
[ 'componentWillReceiveProps-end', 'yellow', 'green' ],
[ 'shouldComponentUpdate-currentState', 'yellow', null ],
[ 'shouldComponentUpdate-currentState', 'yellow', 'green' ],
[ 'shouldComponentUpdate-nextState', 'green' ],
[ 'componentWillUpdate-currentState', 'yellow', null ],
[ 'componentWillUpdate-currentState', 'yellow', 'green' ],
[ 'componentWillUpdate-nextState', 'green' ],
[ 'render', 'green', null ],
[ 'componentDidUpdate-currentState', 'green', null ],
[ 'render', 'green', 'green' ],
[ 'componentDidUpdate-currentState', 'green', 'green' ],
[ 'componentDidUpdate-prevState', 'yellow' ],
// setFavoriteColor('blue')
[ 'shouldComponentUpdate-currentState', 'green', null ],
[ 'shouldComponentUpdate-currentState', 'green', 'blue' ],
[ 'shouldComponentUpdate-nextState', 'blue' ],
[ 'componentWillUpdate-currentState', 'green', null ],
[ 'componentWillUpdate-currentState', 'green', 'blue' ],
[ 'componentWillUpdate-nextState', 'blue' ],
[ 'render', 'blue', null ],
[ 'componentDidUpdate-currentState', 'blue', null ],
[ 'render', 'blue', 'blue' ],
[ 'componentDidUpdate-currentState', 'blue', 'blue' ],
[ 'componentDidUpdate-prevState', 'green' ],
// forceUpdate()
[ 'componentWillUpdate-currentState', 'blue', null ],
[ 'componentWillUpdate-currentState', 'blue', 'blue' ],
[ 'componentWillUpdate-nextState', 'blue' ],
[ 'render', 'blue', null ],
[ 'componentDidUpdate-currentState', 'blue', null ],
[ 'render', 'blue', 'blue' ],
[ 'componentDidUpdate-currentState', 'blue', 'blue' ],
[ 'componentDidUpdate-prevState', 'blue' ],
// unmountComponent()
// state is available within `componentWillUnmount()`
[ 'componentWillUnmount', 'blue', null ]
[ 'componentWillUnmount', 'blue', 'blue' ]
]);
});
});
54 changes: 43 additions & 11 deletions src/core/__tests__/ReactUpdates-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,18 @@ describe('ReactUpdates', function() {

var instance = ReactTestUtils.renderIntoDocument(<Component />);
expect(instance.state.x).toBe(0);
expect(instance.currentState.x).toBe(0);

ReactUpdates.batchedUpdates(function() {
instance.setState({x: 1});
instance.setState({x: 2});
expect(instance.state.x).toBe(0);
expect(instance.state.x).toBe(2);
expect(instance.currentState.x).toBe(0);
expect(updateCount).toBe(0);
});

expect(instance.state.x).toBe(2);
expect(instance.currentState.x).toBe(2);
expect(updateCount).toBe(1);
});

Expand All @@ -75,17 +78,23 @@ describe('ReactUpdates', function() {
var instance = ReactTestUtils.renderIntoDocument(<Component />);
expect(instance.state.x).toBe(0);
expect(instance.state.y).toBe(0);
expect(instance.currentState.x).toBe(0);
expect(instance.currentState.y).toBe(0);

ReactUpdates.batchedUpdates(function() {
instance.setState({x: 1});
instance.setState({y: 2});
expect(instance.state.x).toBe(0);
expect(instance.state.y).toBe(0);
expect(instance.state.x).toBe(1);
expect(instance.state.y).toBe(2);
expect(instance.currentState.x).toBe(0);
expect(instance.currentState.y).toBe(0);
expect(updateCount).toBe(0);
});

expect(instance.state.x).toBe(1);
expect(instance.state.y).toBe(2);
expect(instance.currentState.x).toBe(1);
expect(instance.currentState.y).toBe(2);
expect(updateCount).toBe(1);
});

Expand All @@ -106,12 +115,14 @@ describe('ReactUpdates', function() {
var instance = ReactTestUtils.renderIntoDocument(<Component x={0} />);
expect(instance.props.x).toBe(0);
expect(instance.state.y).toBe(0);
expect(instance.currentState.y).toBe(0);

ReactUpdates.batchedUpdates(function() {
instance.setProps({x: 1});
instance.setState({y: 2});
expect(instance.props.x).toBe(0);
expect(instance.state.y).toBe(0);
expect(instance.state.y).toBe(2);
expect(instance.currentState.y).toBe(0);
expect(updateCount).toBe(0);
});

Expand Down Expand Up @@ -149,19 +160,25 @@ describe('ReactUpdates', function() {
var instance = ReactTestUtils.renderIntoDocument(<Parent />);
var child = instance.refs.child;
expect(instance.state.x).toBe(0);
expect(instance.currentState.x).toBe(0);
expect(child.state.y).toBe(0);
expect(child.currentState.y).toBe(0);

ReactUpdates.batchedUpdates(function() {
instance.setState({x: 1});
child.setState({y: 2});
expect(instance.state.x).toBe(0);
expect(child.state.y).toBe(0);
expect(instance.state.x).toBe(1);
expect(instance.currentState.x).toBe(0);
expect(child.state.y).toBe(2);
expect(child.currentState.y).toBe(0);
expect(parentUpdateCount).toBe(0);
expect(childUpdateCount).toBe(0);
});

expect(instance.state.x).toBe(1);
expect(instance.currentState.x).toBe(1);
expect(child.state.y).toBe(2);
expect(child.currentState.y).toBe(2);
expect(parentUpdateCount).toBe(1);
expect(childUpdateCount).toBe(1);
});
Expand Down Expand Up @@ -195,19 +212,25 @@ describe('ReactUpdates', function() {
var instance = ReactTestUtils.renderIntoDocument(<Parent />);
var child = instance.refs.child;
expect(instance.state.x).toBe(0);
expect(instance.currentState.x).toBe(0);
expect(child.state.y).toBe(0);
expect(child.currentState.y).toBe(0);

ReactUpdates.batchedUpdates(function() {
child.setState({y: 2});
instance.setState({x: 1});
expect(instance.state.x).toBe(0);
expect(child.state.y).toBe(0);
expect(instance.state.x).toBe(1);
expect(instance.currentState.x).toBe(0);
expect(child.state.y).toBe(2);
expect(child.currentState.y).toBe(0);
expect(parentUpdateCount).toBe(0);
expect(childUpdateCount).toBe(0);
});

expect(instance.state.x).toBe(1);
expect(instance.currentState.x).toBe(1);
expect(child.state.y).toBe(2);
expect(child.currentState.y).toBe(2);
expect(parentUpdateCount).toBe(1);

// Batching reduces the number of updates here to 1.
Expand All @@ -230,6 +253,7 @@ describe('ReactUpdates', function() {

var instance = ReactTestUtils.renderIntoDocument(<Component />);
expect(instance.state.x).toBe(0);
expect(instance.currentState.x).toBe(0);

var innerCallbackRun = false;
ReactUpdates.batchedUpdates(function() {
Expand All @@ -238,17 +262,21 @@ describe('ReactUpdates', function() {
expect(this).toBe(instance);
innerCallbackRun = true;
expect(instance.state.x).toBe(2);
expect(instance.currentState.x).toBe(2);
expect(updateCount).toBe(2);
});
expect(instance.state.x).toBe(1);
expect(instance.state.x).toBe(2);
expect(instance.currentState.x).toBe(1);
expect(updateCount).toBe(1);
});
expect(instance.state.x).toBe(0);
expect(instance.state.x).toBe(1);
expect(instance.currentState.x).toBe(0);
expect(updateCount).toBe(0);
});

expect(innerCallbackRun).toBeTruthy();
expect(instance.state.x).toBe(2);
expect(instance.currentState.x).toBe(2);
expect(updateCount).toBe(2);
});

Expand All @@ -272,6 +300,7 @@ describe('ReactUpdates', function() {

var instance = ReactTestUtils.renderIntoDocument(<Component />);
expect(instance.state.x).toBe(0);
expect(instance.currentState.x).toBe(0);

var callbacksRun = 0;
ReactUpdates.batchedUpdates(function() {
Expand All @@ -281,14 +310,17 @@ describe('ReactUpdates', function() {
instance.forceUpdate(function() {
callbacksRun++;
});
expect(instance.state.x).toBe(0);
expect(instance.state.x).toBe(1);
expect(instance.currentState.x).toBe(0);
expect(callbacksRun).toBe(0);
expect(updateCount).toBe(0);
});

expect(callbacksRun).toBe(2);
// shouldComponentUpdate shouldn't be called since we're forcing
expect(shouldUpdateCount).toBe(0);
expect(instance.state.x).toBe(1);
expect(instance.currentState.x).toBe(1);
expect(updateCount).toBe(1);
});

Expand Down