@samitouri / QOS-React / commits / ad5273d348

Call getSnapshotBeforeUpdate before mutation (#12468)

* Call getSnapshotBeforeUpdate in separate traversal, before mutation (aka revert db84b9a) and add unit test. * Added a new timer to ReactDebugFiberPerf for Snapshot effects

Brian Vaughn committed Mar 27, 2018 at 15:37 UTC ad5273d3488c39a2127d05925d99006b4b5641b2
4 files changed +147 -5
packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js
+39
@@ -1150,6 +1150,45 @@ describe('ReactComponentLifeCycle', () => {
1150 expect(log).toEqual([]);
1151 });
1152
1153 + it('should call getSnapshotBeforeUpdate before mutations are committed', () => {
1154 + const log = [];
1155 +
1156 + class MyComponent extends React.Component {
1157 + divRef = React.createRef();
1158 + getSnapshotBeforeUpdate(prevProps, prevState) {
1159 + log.push('getSnapshotBeforeUpdate');
1160 + expect(this.divRef.current.textContent).toBe(
1161 + `value:${prevProps.value}`,
1162 + );
1163 + return 'foobar';
1164 + }
1165 + componentDidUpdate(prevProps, prevState, snapshot) {
1166 + log.push('componentDidUpdate');
1167 + expect(this.divRef.current.textContent).toBe(
1168 + `value:${this.props.value}`,
1169 + );
1170 + expect(snapshot).toBe('foobar');
1171 + }
1172 + render() {
1173 + log.push('render');
1174 + return <div ref={this.divRef}>{`value:${this.props.value}`}</div>;
1175 + }
1176 + }
1177 +
1178 + const div = document.createElement('div');
1179 + ReactDOM.render(<MyComponent value="foo" />, div);
1180 + expect(log).toEqual(['render']);
1181 + log.length = 0;
1182 +
1183 + ReactDOM.render(<MyComponent value="bar" />, div);
1184 + expect(log).toEqual([
1185 + 'render',
1186 + 'getSnapshotBeforeUpdate',
1187 + 'componentDidUpdate',
1188 + ]);
1189 + log.length = 0;
1190 + });
1191 +
1192 it('should warn if getSnapshotBeforeUpdate returns undefined', () => {
1193 class MyComponent extends React.Component {
1194 getSnapshotBeforeUpdate() {}
packages/react-reconciler/src/ReactDebugFiberPerf.js
+25
@@ -426,6 +426,31 @@ export function stopCommitTimer(): void {
426 }
427 }
428
429 +export function startCommitSnapshotEffectsTimer(): void {
430 + if (enableUserTimingAPI) {
431 + if (!supportsUserTiming) {
432 + return;
433 + }
434 + effectCountInCurrentCommit = 0;
435 + beginMark('(Committing Snapshot Effects)');
436 + }
437 +}
438 +
439 +export function stopCommitSnapshotEffectsTimer(): void {
440 + if (enableUserTimingAPI) {
441 + if (!supportsUserTiming) {
442 + return;
443 + }
444 + const count = effectCountInCurrentCommit;
445 + effectCountInCurrentCommit = 0;
446 + endMark(
447 + `(Committing Snapshot Effects: ${count} Total)`,
448 + '(Committing Snapshot Effects)',
449 + null,
450 + );
451 + }
452 +}
453 +
454 export function startCommitHostEffectsTimer(): void {
455 if (enableUserTimingAPI) {
456 if (!supportsUserTiming) {
packages/react-reconciler/src/ReactFiberScheduler.js
+53 -5
@@ -68,6 +68,8 @@ import {
68 stopWorkLoopTimer,
69 startCommitTimer,
70 stopCommitTimer,
71 + startCommitSnapshotEffectsTimer,
72 + stopCommitSnapshotEffectsTimer,
73 startCommitHostEffectsTimer,
74 stopCommitHostEffectsTimer,
75 startCommitLifeCyclesTimer,
@@ -322,11 +324,6 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
324
325 const effectTag = nextEffect.effectTag;
326
325 - if (effectTag & Snapshot) {
326 - const current = nextEffect.alternate;
327 - commitBeforeMutationLifeCycles(current, nextEffect);
328 - }
329 -
327 if (effectTag & ContentReset) {
328 commitResetTextContent(nextEffect);
329 }
@@ -384,6 +381,22 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
381 }
382 }
383
384 + function commitBeforeMutationLifecycles() {
385 + while (nextEffect !== null) {
386 + const effectTag = nextEffect.effectTag;
387 +
388 + if (effectTag & Snapshot) {
389 + recordEffect();
390 + const current = nextEffect.alternate;
391 + commitBeforeMutationLifeCycles(current, nextEffect);
392 + }
393 +
394 + // Don't cleanup effects yet;
395 + // This will be done by commitAllLifeCycles()
396 + nextEffect = nextEffect.nextEffect;
397 + }
398 + }
399 +
400 function commitAllLifeCycles(
401 finishedRoot: FiberRoot,
402 currentTime: ExpirationTime,
@@ -491,6 +504,41 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
504
505 prepareForCommit(root.containerInfo);
506
507 + // Invoke instances of getSnapshotBeforeUpdate before mutation.
508 + nextEffect = firstEffect;
509 + startCommitSnapshotEffectsTimer();
510 + while (nextEffect !== null) {
511 + let didError = false;
512 + let error;
513 + if (__DEV__) {
514 + invokeGuardedCallback(null, commitBeforeMutationLifecycles, null);
515 + if (hasCaughtError()) {
516 + didError = true;
517 + error = clearCaughtError();
518 + }
519 + } else {
520 + try {
521 + commitBeforeMutationLifecycles();
522 + } catch (e) {
523 + didError = true;
524 + error = e;
525 + }
526 + }
527 + if (didError) {
528 + invariant(
529 + nextEffect !== null,
530 + 'Should have next effect. This error is likely caused by a bug ' +
531 + 'in React. Please file an issue.',
532 + );
533 + onCommitPhaseError(nextEffect, error);
534 + // Clean-up
535 + if (nextEffect !== null) {
536 + nextEffect = nextEffect.nextEffect;
537 + }
538 + }
539 + }
540 + stopCommitSnapshotEffectsTimer();
541 +
542 // Commit all the side-effects within a tree. We'll do this in two passes.
543 // The first pass performs all the host insertions, updates, deletions and
544 // ref unmounts.
packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.internal.js.snap
+30
@@ -10,6 +10,7 @@ exports[`ReactDebugFiberPerf captures all lifecycles 1`] = `
10 ⚛ AllLifecycles.getChildContext
11
12 ⚛ (Committing Changes)
13 + ⚛ (Committing Snapshot Effects: 0 Total)
14 ⚛ (Committing Host Effects: 1 Total)
15 ⚛ (Calling Lifecycle Methods: 1 Total)
16 ⚛ AllLifecycles.componentDidMount
@@ -25,6 +26,7 @@ exports[`ReactDebugFiberPerf captures all lifecycles 1`] = `
26 ⚛ AllLifecycles.getChildContext
27
28 ⚛ (Committing Changes)
29 + ⚛ (Committing Snapshot Effects: 0 Total)
30 ⚛ (Committing Host Effects: 2 Total)
31 ⚛ (Calling Lifecycle Methods: 2 Total)
32 ⚛ AllLifecycles.componentDidUpdate
@@ -35,6 +37,7 @@ exports[`ReactDebugFiberPerf captures all lifecycles 1`] = `
37 ⚛ (React Tree Reconciliation)
38
39 ⚛ (Committing Changes)
40 + ⚛ (Committing Snapshot Effects: 0 Total)
41 ⚛ (Committing Host Effects: 1 Total)
42 ⚛ AllLifecycles.componentWillUnmount
43 ⚛ (Calling Lifecycle Methods: 0 Total)
@@ -53,6 +56,7 @@ exports[`ReactDebugFiberPerf deduplicates lifecycle names during commit to reduc
56 ⚛ B [update]
57
58 ⚛ (Committing Changes)
59 + ⚛ (Committing Snapshot Effects: 0 Total)
60 ⚛ (Committing Host Effects: 9 Total)
61 ⚛ (Calling Lifecycle Methods: 9 Total)
62 ⚛ A.componentDidUpdate
@@ -70,6 +74,7 @@ exports[`ReactDebugFiberPerf deduplicates lifecycle names during commit to reduc
74 ⚛ B [update]
75
76 ⛔ (Committing Changes) Warning: Lifecycle hook scheduled a cascading update
77 + ⚛ (Committing Snapshot Effects: 0 Total)
78 ⚛ (Committing Host Effects: 9 Total)
79 ⚛ (Calling Lifecycle Methods: 9 Total)
80 ⚛ A.componentDidUpdate
@@ -79,6 +84,7 @@ exports[`ReactDebugFiberPerf deduplicates lifecycle names during commit to reduc
84 ⚛ B [update]
85
86 ⚛ (Committing Changes)
87 + ⚛ (Committing Snapshot Effects: 0 Total)
88 ⚛ (Committing Host Effects: 3 Total)
89 ⚛ (Calling Lifecycle Methods: 3 Total)
90 ⚛ B.componentDidUpdate
@@ -90,6 +96,7 @@ exports[`ReactDebugFiberPerf does not schedule an extra callback if setState is
96 ⚛ Component [mount]
97
98 ⛔ (Committing Changes) Warning: Lifecycle hook scheduled a cascading update
99 + ⚛ (Committing Snapshot Effects: 0 Total)
100 ⚛ (Committing Host Effects: 1 Total)
101 ⚛ (Calling Lifecycle Methods: 1 Total)
102 ⛔ Component.componentDidMount Warning: Scheduled a cascading update
@@ -98,6 +105,7 @@ exports[`ReactDebugFiberPerf does not schedule an extra callback if setState is
105 ⚛ Component [update]
106
107 ⚛ (Committing Changes)
108 + ⚛ (Committing Snapshot Effects: 0 Total)
109 ⚛ (Committing Host Effects: 1 Total)
110 ⚛ (Calling Lifecycle Methods: 1 Total)
111 "
@@ -113,6 +121,7 @@ exports[`ReactDebugFiberPerf does not treat setState from cWM or cWRP as cascadi
121 ⚛ NotCascading.componentWillMount
122
123 ⚛ (Committing Changes)
124 + ⚛ (Committing Snapshot Effects: 0 Total)
125 ⚛ (Committing Host Effects: 1 Total)
126 ⚛ (Calling Lifecycle Methods: 0 Total)
127
@@ -125,6 +134,7 @@ exports[`ReactDebugFiberPerf does not treat setState from cWM or cWRP as cascadi
134 ⚛ NotCascading.componentWillReceiveProps
135
136 ⚛ (Committing Changes)
137 + ⚛ (Committing Snapshot Effects: 0 Total)
138 ⚛ (Committing Host Effects: 2 Total)
139 ⚛ (Calling Lifecycle Methods: 2 Total)
140 "
@@ -139,6 +149,7 @@ exports[`ReactDebugFiberPerf measures a simple reconciliation 1`] = `
149 ⚛ Child [mount]
150
151 ⚛ (Committing Changes)
152 + ⚛ (Committing Snapshot Effects: 0 Total)
153 ⚛ (Committing Host Effects: 1 Total)
154 ⚛ (Calling Lifecycle Methods: 0 Total)
155
@@ -150,6 +161,7 @@ exports[`ReactDebugFiberPerf measures a simple reconciliation 1`] = `
161 ⚛ Child [update]
162
163 ⚛ (Committing Changes)
164 + ⚛ (Committing Snapshot Effects: 0 Total)
165 ⚛ (Committing Host Effects: 2 Total)
166 ⚛ (Calling Lifecycle Methods: 2 Total)
167
@@ -159,6 +171,7 @@ exports[`ReactDebugFiberPerf measures a simple reconciliation 1`] = `
171 ⚛ (React Tree Reconciliation)
172
173 ⚛ (Committing Changes)
174 + ⚛ (Committing Snapshot Effects: 0 Total)
175 ⚛ (Committing Host Effects: 1 Total)
176 ⚛ (Calling Lifecycle Methods: 0 Total)
177 "
@@ -189,6 +202,7 @@ exports[`ReactDebugFiberPerf measures deferred work in chunks 1`] = `
202 ⚛ Child [mount]
203
204 ⚛ (Committing Changes)
205 + ⚛ (Committing Snapshot Effects: 0 Total)
206 ⚛ (Committing Host Effects: 1 Total)
207 ⚛ (Calling Lifecycle Methods: 0 Total)
208 "
@@ -200,6 +214,7 @@ exports[`ReactDebugFiberPerf measures deprioritized work 1`] = `
214 ⚛ Parent [mount]
215
216 ⚛ (Committing Changes)
217 + ⚛ (Committing Snapshot Effects: 0 Total)
218 ⚛ (Committing Host Effects: 1 Total)
219 ⚛ (Calling Lifecycle Methods: 0 Total)
220
@@ -210,6 +225,7 @@ exports[`ReactDebugFiberPerf measures deprioritized work 1`] = `
225 ⚛ Child [mount]
226
227 ⚛ (Committing Changes)
228 + ⚛ (Committing Snapshot Effects: 0 Total)
229 ⚛ (Committing Host Effects: 3 Total)
230 ⚛ (Calling Lifecycle Methods: 2 Total)
231 "
@@ -230,6 +246,7 @@ exports[`ReactDebugFiberPerf recovers from caught errors 1`] = `
246 ⚛ Boundary [mount]
247
248 ⛔ (Committing Changes) Warning: Lifecycle hook scheduled a cascading update
249 + ⚛ (Committing Snapshot Effects: 0 Total)
250 ⚛ (Committing Host Effects: 2 Total)
251 ⚛ (Calling Lifecycle Methods: 0 Total)
252
@@ -238,6 +255,7 @@ exports[`ReactDebugFiberPerf recovers from caught errors 1`] = `
255 ⚛ ErrorReport [mount]
256
257 ⚛ (Committing Changes)
258 + ⚛ (Committing Snapshot Effects: 0 Total)
259 ⚛ (Committing Host Effects: 2 Total)
260 ⚛ (Calling Lifecycle Methods: 1 Total)
261 "
@@ -254,6 +272,7 @@ exports[`ReactDebugFiberPerf recovers from fatal errors 1`] = `
272 ⚛ (React Tree Reconciliation)
273
274 ⚛ (Committing Changes)
275 + ⚛ (Committing Snapshot Effects: 0 Total)
276 ⚛ (Committing Host Effects: 1 Total)
277 ⚛ (Calling Lifecycle Methods: 0 Total)
278
@@ -265,6 +284,7 @@ exports[`ReactDebugFiberPerf recovers from fatal errors 1`] = `
284 ⚛ Child [mount]
285
286 ⚛ (Committing Changes)
287 + ⚛ (Committing Snapshot Effects: 0 Total)
288 ⚛ (Committing Host Effects: 1 Total)
289 ⚛ (Calling Lifecycle Methods: 0 Total)
290 "
@@ -279,6 +299,7 @@ exports[`ReactDebugFiberPerf skips parents during setState 1`] = `
299 ⚛ B [update]
300
301 ⚛ (Committing Changes)
302 + ⚛ (Committing Snapshot Effects: 0 Total)
303 ⚛ (Committing Host Effects: 6 Total)
304 ⚛ (Calling Lifecycle Methods: 6 Total)
305 "
@@ -292,6 +313,7 @@ exports[`ReactDebugFiberPerf supports portals 1`] = `
313 ⚛ Child [mount]
314
315 ⚛ (Committing Changes)
316 + ⚛ (Committing Snapshot Effects: 0 Total)
317 ⚛ (Committing Host Effects: 2 Total)
318 ⚛ (Calling Lifecycle Methods: 0 Total)
319 "
@@ -310,6 +332,7 @@ exports[`ReactDebugFiberPerf supports returns 1`] = `
332 ⚛ Continuation [mount]
333
334 ⚛ (Committing Changes)
335 + ⚛ (Committing Snapshot Effects: 0 Total)
336 ⚛ (Committing Host Effects: 3 Total)
337 ⚛ (Calling Lifecycle Methods: 0 Total)
338 "
@@ -325,12 +348,14 @@ exports[`ReactDebugFiberPerf warns if an in-progress update is interrupted 1`] =
348 ⛔ (React Tree Reconciliation) Warning: A top-level update interrupted the previous render
349 ⚛ Foo [mount]
350 ⚛ (Committing Changes)
351 + ⚛ (Committing Snapshot Effects: 0 Total)
352 ⚛ (Committing Host Effects: 1 Total)
353 ⚛ (Calling Lifecycle Methods: 0 Total)
354
355 ⚛ (React Tree Reconciliation)
356
357 ⚛ (Committing Changes)
358 + ⚛ (Committing Snapshot Effects: 0 Total)
359 ⚛ (Committing Host Effects: 1 Total)
360 ⚛ (Calling Lifecycle Methods: 1 Total)
361 "
@@ -343,6 +368,7 @@ exports[`ReactDebugFiberPerf warns if async work expires (starvation) 1`] = `
368 ⚛ Foo [mount]
369
370 ⚛ (Committing Changes)
371 + ⚛ (Committing Snapshot Effects: 0 Total)
372 ⚛ (Committing Host Effects: 1 Total)
373 ⚛ (Calling Lifecycle Methods: 0 Total)
374 "
@@ -357,6 +383,7 @@ exports[`ReactDebugFiberPerf warns on cascading renders from setState 1`] = `
383 ⚛ Cascading [mount]
384
385 ⛔ (Committing Changes) Warning: Lifecycle hook scheduled a cascading update
386 + ⚛ (Committing Snapshot Effects: 0 Total)
387 ⚛ (Committing Host Effects: 2 Total)
388 ⚛ (Calling Lifecycle Methods: 1 Total)
389 ⛔ Cascading.componentDidMount Warning: Scheduled a cascading update
@@ -365,6 +392,7 @@ exports[`ReactDebugFiberPerf warns on cascading renders from setState 1`] = `
392 ⚛ Cascading [update]
393
394 ⚛ (Committing Changes)
395 + ⚛ (Committing Snapshot Effects: 0 Total)
396 ⚛ (Committing Host Effects: 2 Total)
397 ⚛ (Calling Lifecycle Methods: 2 Total)
398 "
@@ -378,6 +406,7 @@ exports[`ReactDebugFiberPerf warns on cascading renders from top-level render 1`
406 ⚛ Cascading [mount]
407
408 ⛔ (Committing Changes) Warning: Lifecycle hook scheduled a cascading update
409 + ⚛ (Committing Snapshot Effects: 0 Total)
410 ⚛ (Committing Host Effects: 1 Total)
411 ⚛ (Calling Lifecycle Methods: 1 Total)
412 ⛔ Cascading.componentDidMount Warning: Scheduled a cascading update
@@ -387,6 +416,7 @@ exports[`ReactDebugFiberPerf warns on cascading renders from top-level render 1`
416 ⚛ Child [mount]
417
418 ⚛ (Committing Changes)
419 + ⚛ (Committing Snapshot Effects: 0 Total)
420 ⚛ (Committing Host Effects: 1 Total)
421 ⚛ (Calling Lifecycle Methods: 0 Total)
422 "