Don't drop updates from queue when calling replaceState
We should be able to support passing a function to replaceState, which receives the accumulation of all the previously applied state updates. Which means we shouldn't drop those updates from the queue. Technically, we could drop them only when an object is passed to replaceState, but that seems like more trouble than it's worth.
Andrew Clark committed
Dec 15, 2016 at 18:26 UTC
7b6c94c767d9825d37f71b9b0a3aa1d746ed3ada
4 files changed
+38
-39
scripts/fiber/tests-passing.txt
+1
@@ -1236,6 +1236,7 @@ src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js
1236
* only drops updates with equal or lesser priority when replaceState is called
1237
* can abort an update, schedule additional updates, and resume
1238
* can abort an update, schedule a replaceState, and resume
1239
+* passes accumulation of previous updates to replaceState updater function
1240
* does not call callbacks that are scheduled by another callback until a later commit
1241
* gives setState during reconciliation the same priority as whatever level is currently reconciling
1242
* enqueues setState inside an updater function as if the in-progress update is progressed (and warns)
src/renderers/shared/fiber/ReactFiberUpdateQueue.js
+1
-33
@@ -309,36 +309,6 @@ function addReplaceUpdate(
309
next: null,
310
};
311
312
- // Drop all updates with equal priority
313
- let queue = ensureUpdateQueue(fiber);
314
- for (let i = 0; queue && i < 2; i++) {
315
- let replaceAfter = null;
316
- let replaceBefore = queue.first;
317
- let comparison = 255;
318
- while (replaceBefore &&
319
- (comparison = comparePriority(replaceBefore.priorityLevel, priorityLevel)) <= 0) {
320
- if (comparison < 0) {
321
- replaceAfter = replaceBefore;
322
- }
323
- replaceBefore = replaceBefore.next;
324
- }
325
-
326
- if (replaceAfter) {
327
- replaceAfter.next = replaceBefore;
328
- } else {
329
- queue.first = replaceBefore;
330
- }
331
-
332
- if (!replaceBefore) {
333
- queue.last = replaceAfter;
334
- }
335
-
336
- if (fiber.alternate) {
337
- queue = ensureUpdateQueue(fiber.alternate);
338
- } else {
339
- queue = null;
340
- }
341
- }
312
if (__DEV__) {
313
insertUpdate(fiber, update, 'replaceState');
314
} else {
@@ -426,9 +396,7 @@ function beginUpdateQueue(
396
397
let partialState;
398
if (update.isReplace) {
429
- // A replace should drop all previous updates in the queue, so
430
- // use the original `prevState`, not the accumulated `state`
431
- state = getStateFromUpdate(update, instance, prevState, props);
399
+ state = getStateFromUpdate(update, instance, state, props);
400
dontMutatePrevState = true;
401
} else {
402
partialState = getStateFromUpdate(update, instance, state, props);
src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js
+23
-5
@@ -203,9 +203,7 @@ describe('ReactIncrementalUpdates', () => {
203
render() {
204
ops.push('render');
205
instance = this;
206
- return (
207
- <span prop={Object.keys(this.state).join('')} />
208
- );
206
+ return <span />;
207
},
208
});
209
@@ -244,11 +242,31 @@ describe('ReactIncrementalUpdates', () => {
242
instance.setState(createUpdate('g'));
243
244
ReactNoop.flush();
247
- // Ensure that updater function d is never called.
248
- expect(progressedUpdates).toEqual(['e', 'f', 'g']);
245
+ expect(progressedUpdates).toEqual(['d', 'e', 'f', 'g']);
246
expect(instance.state).toEqual({ e: 'e', f: 'f', g: 'g' });
247
});
248
249
+ it('passes accumulation of previous updates to replaceState updater function', () => {
250
+ let instance;
251
+ const Foo = React.createClass({
252
+ getInitialState() {
253
+ return {};
254
+ },
255
+ render() {
256
+ instance = this;
257
+ return <span />;
258
+ },
259
+ });
260
+ ReactNoop.render(<Foo />);
261
+ ReactNoop.flush();
262
+
263
+ instance.setState({ a: 'a' });
264
+ instance.setState({ b: 'b' });
265
+ instance.replaceState(previousState => ({ previousState }));
266
+ ReactNoop.flush();
267
+ expect(instance.state).toEqual({ previousState: { a: 'a', b : 'b' } });
268
+ });
269
+
270
it('does not call callbacks that are scheduled by another callback until a later commit', () => {
271
let ops = [];
272
class Foo extends React.Component {
src/renderers/shared/shared/__tests__/ReactCompositeComponentState-test.js
+13
-1
@@ -193,7 +193,19 @@ describe('ReactCompositeComponent-state', () => {
193
// setState({color:'green'}) only enqueues a pending state.
194
['componentWillReceiveProps-end', 'yellow'],
195
// pending state queue is processed
196
- // before-setState-receiveProps never called, due to replaceState.
196
+ );
197
+
198
+ if (ReactDOMFeatureFlags.useFiber) {
199
+ // In Stack, this is never called because replaceState drops all updates
200
+ // from the queue. In Fiber, we keep updates in the queue to support
201
+ // replaceState(prevState => newState).
202
+ // TODO: Fix Stack to match Fiber.
203
+ expected.push(
204
+ ['before-setState-receiveProps', 'yellow'],
205
+ );
206
+ }
207
+
208
+ expected.push(
209
['before-setState-again-receiveProps', undefined],
210
['after-setState-receiveProps', 'green'],
211
['shouldComponentUpdate-currentState', 'yellow'],