Fix bug with double updates in a single batch (#6650)
Fixes #2410. Fixes #6371. Fixes #6538. I also manually tested the codepen in #3762 and verified it now works.
Ben Alpert committed
Apr 29, 2016 at 14:02 UTC
c1e3f7ec14971dbf678a731408d38959545793eb
4 files changed
+133
-5
src/renderers/shared/reconciler/ReactCompositeComponent.js
+5
-3
@@ -153,6 +153,7 @@ var ReactCompositeComponentMixin = {
153
this._nativeContainerInfo = null;
154
155
// See ReactUpdateQueue
156
+ this._updateBatchNumber = null;
157
this._pendingElement = null;
158
this._pendingStateQueue = null;
159
this._pendingReplaceState = false;
@@ -765,9 +766,7 @@ var ReactCompositeComponentMixin = {
766
transaction,
767
this._context
768
);
768
- }
769
-
770
- if (this._pendingStateQueue !== null || this._pendingForceUpdate) {
769
+ } else if (this._pendingStateQueue !== null || this._pendingForceUpdate) {
770
this.updateComponent(
771
transaction,
772
this._currentElement,
@@ -775,6 +774,8 @@ var ReactCompositeComponentMixin = {
774
this._context,
775
this._context
776
);
777
+ } else {
778
+ this._updateBatchNumber = null;
779
}
780
},
781
@@ -878,6 +879,7 @@ var ReactCompositeComponentMixin = {
879
);
880
}
881
882
+ this._updateBatchNumber = null;
883
if (shouldUpdate) {
884
this._pendingForceUpdate = false;
885
// Will set `this.props`, `this.state` and `this.context`.
src/renderers/shared/reconciler/ReactReconciler.js
+17
-1
@@ -14,6 +14,8 @@
14
var ReactRef = require('ReactRef');
15
var ReactInstrumentation = require('ReactInstrumentation');
16
17
+var invariant = require('invariant');
18
+
19
/**
20
* Helper to call ReactRef.attachRefs with this composite component, split out
21
* to avoid allocations in the transaction mount-ready queue.
@@ -190,8 +192,22 @@ var ReactReconciler = {
192
*/
193
performUpdateIfNecessary: function(
194
internalInstance,
193
- transaction
195
+ transaction,
196
+ updateBatchNumber
197
) {
198
+ if (internalInstance._updateBatchNumber !== updateBatchNumber) {
199
+ // The component's enqueued batch number should always be the current
200
+ // batch or the following one.
201
+ invariant(
202
+ internalInstance._updateBatchNumber == null ||
203
+ internalInstance._updateBatchNumber === updateBatchNumber + 1,
204
+ 'performUpdateIfNecessary: Unexpected batch number (current %s, ' +
205
+ 'pending %s)',
206
+ updateBatchNumber,
207
+ internalInstance._updateBatchNumber
208
+ );
209
+ return;
210
+ }
211
if (__DEV__) {
212
if (internalInstance._debugID !== 0) {
213
ReactInstrumentation.debugTool.onBeginReconcilerTimer(
src/renderers/shared/reconciler/ReactUpdates.js
+13
-1
@@ -22,6 +22,7 @@ var Transaction = require('Transaction');
22
var invariant = require('invariant');
23
24
var dirtyComponents = [];
25
+var updateBatchNumber = 0;
26
var asapCallbackQueue = CallbackQueue.getPooled();
27
var asapEnqueued = false;
28
@@ -138,6 +139,13 @@ function runBatchedUpdates(transaction) {
139
// them before their children by sorting the array.
140
dirtyComponents.sort(mountOrderComparator);
141
142
+ // Any updates enqueued while reconciling must be performed after this entire
143
+ // batch. Otherwise, if dirtyComponents is [A, B] where A has children B and
144
+ // C, B could update twice in a single batch if C's render enqueues an update
145
+ // to B (since B would have already updated, we should skip it, and the only
146
+ // way we can know to do so is by checking the batch counter).
147
+ updateBatchNumber++;
148
+
149
for (var i = 0; i < len; i++) {
150
// If a component is unmounted before pending changes apply, it will still
151
// be here, but we assume that it has cleared its _pendingCallbacks and
@@ -166,7 +174,8 @@ function runBatchedUpdates(transaction) {
174
175
ReactReconciler.performUpdateIfNecessary(
176
component,
169
- transaction.reconcileTransaction
177
+ transaction.reconcileTransaction,
178
+ updateBatchNumber
179
);
180
181
if (markerName) {
@@ -238,6 +247,9 @@ function enqueueUpdate(component) {
247
}
248
249
dirtyComponents.push(component);
250
+ if (component._updateBatchNumber == null) {
251
+ component._updateBatchNumber = updateBatchNumber + 1;
252
+ }
253
}
254
255
/**
src/renderers/shared/reconciler/__tests__/ReactUpdates-test.js
+98
@@ -1024,4 +1024,102 @@ describe('ReactUpdates', function() {
1024
'to be a function. Instead received: Foo (keys: a, b).'
1025
);
1026
});
1027
+
1028
+ it('does not update one component twice in a batch (#2410)', function() {
1029
+ var Parent = React.createClass({
1030
+ getChild: function() {
1031
+ return this.refs.child;
1032
+ },
1033
+ render: function() {
1034
+ return <Child ref="child" />;
1035
+ },
1036
+ });
1037
+
1038
+ var renderCount = 0;
1039
+ var postRenderCount = 0;
1040
+ var once = false;
1041
+ var Child = React.createClass({
1042
+ getInitialState: function() {
1043
+ return {updated: false};
1044
+ },
1045
+ componentWillUpdate: function() {
1046
+ if (!once) {
1047
+ once = true;
1048
+ this.setState({updated: true});
1049
+ }
1050
+ },
1051
+ componentDidMount: function() {
1052
+ expect(renderCount).toBe(postRenderCount + 1);
1053
+ postRenderCount++;
1054
+ },
1055
+ componentDidUpdate: function() {
1056
+ expect(renderCount).toBe(postRenderCount + 1);
1057
+ postRenderCount++;
1058
+ },
1059
+ render: function() {
1060
+ expect(renderCount).toBe(postRenderCount);
1061
+ renderCount++;
1062
+ return <div />;
1063
+ },
1064
+ });
1065
+
1066
+ var parent = ReactTestUtils.renderIntoDocument(<Parent />);
1067
+ var child = parent.getChild();
1068
+ ReactDOM.unstable_batchedUpdates(function() {
1069
+ parent.forceUpdate();
1070
+ child.forceUpdate();
1071
+ });
1072
+ });
1073
+
1074
+ it('does not update one component twice in a batch (#6371)', function() {
1075
+ var callbacks = [];
1076
+ function emitChange() {
1077
+ callbacks.forEach(c => c());
1078
+ }
1079
+
1080
+ class App extends React.Component {
1081
+ constructor(props) {
1082
+ super(props);
1083
+ this.state = { showChild: true };
1084
+ }
1085
+ componentDidMount() {
1086
+ console.log('about to remove child via set state');
1087
+ this.setState({ showChild: false });
1088
+ }
1089
+ render() {
1090
+ return (
1091
+ <div>
1092
+ <ForceUpdatesOnChange />
1093
+ {this.state.showChild && <EmitsChangeOnUnmount />}
1094
+ </div>
1095
+ );
1096
+ }
1097
+ }
1098
+
1099
+ class EmitsChangeOnUnmount extends React.Component {
1100
+ componentWillUnmount() {
1101
+ emitChange();
1102
+ }
1103
+ render() {
1104
+ return null;
1105
+ }
1106
+ }
1107
+
1108
+ class ForceUpdatesOnChange extends React.Component {
1109
+ componentDidMount() {
1110
+ this.onChange = () => this.forceUpdate();
1111
+ this.onChange();
1112
+ callbacks.push(this.onChange);
1113
+ }
1114
+ componentWillUnmount() {
1115
+ callbacks = callbacks.filter((c) => c !== this.onChange);
1116
+ }
1117
+ render() {
1118
+ return <div key={Math.random()} onClick={function() {}} />;
1119
+ }
1120
+ }
1121
+
1122
+ ReactDOM.render(<App />, document.createElement('div'));
1123
+ });
1124
+
1125
});