Merge pull request #1188 from developit/bugfix/prevstate-batching

Fix prevState pointing to interim state value when batching (fixes #1186)

src/component.js

@@ -53,9 +53,11 @@ extend(Component.prototype, {
 	 * 	updated
 	 */
 	setState(state, callback) {
-		const prev = this.prevState = this.state;
-		if (typeof state === 'function') state = state(prev, this.props);
-		this.state = extend(extend({}, prev), state);
+		if (!this.prevState) this.prevState = this.state;
+		this.state = extend(
+			extend({}, this.state),
+			typeof state === 'function' ? state(this.state, this.props) : state
+		);
 		if (callback) this._renderCallbacks.push(callback);
 		enqueueRender(this);
 	},

test/browser/lifecycle.js

@@ -1347,8 +1347,67 @@ describe('Lifecycle methods', () => {
 				value: 4
 			});
 		});
-	});
 
+		it("should be passed correct this.state for batched setState", () => {
+			/** @type {() => void} */
+			let updateState;
+
+			let curState;
+			let nextStateArg;
+			let shouldComponentUpdateCount = 0;
+
+			class Foo extends Component {
+				constructor(props) {
+					super(props);
+					this.state = {
+						value: 0
+					};
+					updateState = (value) => this.setState({
+						value
+					});
+				}
+				shouldComponentUpdate(nextProps, nextState) {
+					shouldComponentUpdateCount++;
+					nextStateArg = {...nextState};
+					curState = {...this.state};
+					return this.state.value !== nextState.value;
+				}
+				render() {
+					return <div>{this.state.value}</div>;
+				}
+			}
+
+			// Initial render
+			// state.value: initialized to 0 in constructor, 0 -> 1 in gDSFP
+			let element = render(<Foo foo="foo" />, scratch);
+
+			expect(element.textContent).to.be.equal('0');
+			expect(curState).to.be.undefined;
+			expect(nextStateArg).to.be.undefined;
+			expect(shouldComponentUpdateCount).to.be.equal(0);
+
+			// New state
+			// state.value: 'bar2'
+
+			// batch 2 setState calls with same value
+			updateState('bar');
+			updateState('bar2');
+
+			// Expectation:
+			// `this.state` in shouldComponentUpdate should be
+			// the state from last render, before apply batched setState
+
+			rerender();
+			expect(nextStateArg).to.deep.equal({
+				value: 'bar2'
+			});
+			expect(curState).to.deep.equal({
+				value: 0
+			});
+			expect(shouldComponentUpdateCount).to.be.equal(1);
+			expect(element.textContent).to.be.equal('bar2');
+		});
+	});
 
 	describe('#setState', () => {
 		it('should NOT mutate state, only create new versions', () => {