Fix queueing updates in cWM/cWRP when batching (#8398)
I tried to add a temporary check for the correct state in https://gist.github.com/spicyj/338147e989215b6eeaf48a6ce2d68d93 and run our test suites over it, but none of our existing test cases would have triggered that invariant. The new one I added does.
Ben Alpert committed
Nov 23, 2016 at 13:00 UTC
7ac2044bce8f1f7f761647d6fb1d1c187ce82a13
4 files changed
+41
-5
scripts/fiber/tests-failing.txt
-1
@@ -113,7 +113,6 @@ src/renderers/shared/stack/reconciler/__tests__/ReactStatelessComponent-test.js
113
114
src/renderers/shared/stack/reconciler/__tests__/ReactUpdates-test.js
115
* should queue mount-ready handlers across different roots
116
-* should queue updates from during mount
116
* marks top-level updates
117
* throws in setState if the update callback is not a function
118
* throws in replaceState if the update callback is not a function
scripts/fiber/tests-passing.txt
+2
@@ -1294,6 +1294,7 @@ src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponent-test.js
1294
* unmasked context propagates through updates
1295
* should trigger componentWillReceiveProps for context changes
1296
* only renders once if updated in componentWillReceiveProps
1297
+* only renders once if updated in componentWillReceiveProps when batching
1298
* should allow access to findDOMNode in componentWillUnmount
1299
* context should be passed down from the parent
1300
* should replace state
@@ -1449,6 +1450,7 @@ src/renderers/shared/stack/reconciler/__tests__/ReactUpdates-test.js
1450
* should flush updates in the correct order
1451
* should flush updates in the correct order across roots
1452
* should queue nested updates
1453
+* should queue updates from during mount
1454
* calls componentWillReceiveProps setState callback properly
1455
* does not call render after a component as been deleted
1456
* does not update one component twice in a batch (#2410)
src/renderers/shared/fiber/ReactFiberScheduler.js
+7
-4
@@ -784,10 +784,13 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
784
try {
785
return fn();
786
} finally {
787
- shouldBatchUpdates = prev;
788
- // If we've exited the batch, perform any scheduled task work
789
- if (!shouldBatchUpdates) {
790
- performTaskWork();
787
+ // If we're exiting the batch, perform any scheduled task work
788
+ try {
789
+ if (!prev) {
790
+ performTaskWork();
791
+ }
792
+ } finally {
793
+ shouldBatchUpdates = prev;
794
}
795
}
796
}
src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponent-test.js
+32
@@ -1045,7 +1045,9 @@ describe('ReactCompositeComponent', () => {
1045
1046
componentWillReceiveProps(props) {
1047
expect(props.update).toBe(1);
1048
+ expect(renders).toBe(1);
1049
this.setState({updated: true});
1050
+ expect(renders).toBe(1);
1051
}
1052
1053
render() {
@@ -1063,6 +1065,36 @@ describe('ReactCompositeComponent', () => {
1065
expect(instance.state.updated).toBe(true);
1066
});
1067
1068
+ it('only renders once if updated in componentWillReceiveProps when batching', () => {
1069
+ var renders = 0;
1070
+
1071
+ class Component extends React.Component {
1072
+ state = {updated: false};
1073
+
1074
+ componentWillReceiveProps(props) {
1075
+ expect(props.update).toBe(1);
1076
+ expect(renders).toBe(1);
1077
+ this.setState({updated: true});
1078
+ expect(renders).toBe(1);
1079
+ }
1080
+
1081
+ render() {
1082
+ renders++;
1083
+ return <div />;
1084
+ }
1085
+ }
1086
+
1087
+ var container = document.createElement('div');
1088
+ var instance = ReactDOM.render(<Component update={0} />, container);
1089
+ expect(renders).toBe(1);
1090
+ expect(instance.state.updated).toBe(false);
1091
+ ReactDOM.unstable_batchedUpdates(() => {
1092
+ ReactDOM.render(<Component update={1} />, container);
1093
+ });
1094
+ expect(renders).toBe(2);
1095
+ expect(instance.state.updated).toBe(true);
1096
+ });
1097
+
1098
it('should update refs if shouldComponentUpdate gives false', () => {
1099
class Static extends React.Component {
1100
shouldComponentUpdate() {