Call Profiler onRender after mutations (#13572)
Brian Vaughn committed
Sep 5, 2018 at 17:55 UTC
550dd1d2eccd4855f1ec5e4155ac89dd43c70eff
3 files changed
+61
-35
packages/react-reconciler/src/ReactFiberCommitWork.js
+25
-30
@@ -326,7 +326,30 @@ function commitLifeCycles(
326
return;
327
}
328
case Profiler: {
329
- // We have no life-cycles associated with Profiler.
329
+ if (enableProfilerTimer) {
330
+ const onRender = finishedWork.memoizedProps.onRender;
331
+
332
+ if (enableSchedulerTracking) {
333
+ onRender(
334
+ finishedWork.memoizedProps.id,
335
+ current === null ? 'mount' : 'update',
336
+ finishedWork.actualDuration,
337
+ finishedWork.treeBaseDuration,
338
+ finishedWork.actualStartTime,
339
+ getCommitTime(),
340
+ finishedRoot.memoizedInteractions,
341
+ );
342
+ } else {
343
+ onRender(
344
+ finishedWork.memoizedProps.id,
345
+ current === null ? 'mount' : 'update',
346
+ finishedWork.actualDuration,
347
+ finishedWork.treeBaseDuration,
348
+ finishedWork.actualStartTime,
349
+ getCommitTime(),
350
+ );
351
+ }
352
+ }
353
return;
354
}
355
case PlaceholderComponent: {
@@ -781,11 +804,7 @@ function commitDeletion(current: Fiber): void {
804
detachFiber(current);
805
}
806
784
-function commitWork(
785
- root: FiberRoot,
786
- current: Fiber | null,
787
- finishedWork: Fiber,
788
-): void {
807
+function commitWork(current: Fiber | null, finishedWork: Fiber): void {
808
if (!supportsMutation) {
809
commitContainer(finishedWork);
810
return;
@@ -842,30 +861,6 @@ function commitWork(
861
return;
862
}
863
case Profiler: {
845
- if (enableProfilerTimer) {
846
- const onRender = finishedWork.memoizedProps.onRender;
847
-
848
- if (enableSchedulerTracking) {
849
- onRender(
850
- finishedWork.memoizedProps.id,
851
- current === null ? 'mount' : 'update',
852
- finishedWork.actualDuration,
853
- finishedWork.treeBaseDuration,
854
- finishedWork.actualStartTime,
855
- getCommitTime(),
856
- root.memoizedInteractions,
857
- );
858
- } else {
859
- onRender(
860
- finishedWork.memoizedProps.id,
861
- current === null ? 'mount' : 'update',
862
- finishedWork.actualDuration,
863
- finishedWork.treeBaseDuration,
864
- finishedWork.actualStartTime,
865
- getCommitTime(),
866
- );
867
- }
868
- }
864
return;
865
}
866
case PlaceholderComponent: {
packages/react-reconciler/src/ReactFiberScheduler.js
+5
-5
@@ -370,7 +370,7 @@ function resetStack() {
370
nextUnitOfWork = null;
371
}
372
373
-function commitAllHostEffects(root: FiberRoot) {
373
+function commitAllHostEffects() {
374
while (nextEffect !== null) {
375
if (__DEV__) {
376
ReactCurrentFiber.setCurrentFiber(nextEffect);
@@ -415,12 +415,12 @@ function commitAllHostEffects(root: FiberRoot) {
415
416
// Update
417
const current = nextEffect.alternate;
418
- commitWork(root, current, nextEffect);
418
+ commitWork(current, nextEffect);
419
break;
420
}
421
case Update: {
422
const current = nextEffect.alternate;
423
- commitWork(root, current, nextEffect);
423
+ commitWork(current, nextEffect);
424
break;
425
}
426
case Deletion: {
@@ -652,14 +652,14 @@ function commitRoot(root: FiberRoot, finishedWork: Fiber): void {
652
let didError = false;
653
let error;
654
if (__DEV__) {
655
- invokeGuardedCallback(null, commitAllHostEffects, null, root);
655
+ invokeGuardedCallback(null, commitAllHostEffects, null);
656
if (hasCaughtError()) {
657
didError = true;
658
error = clearCaughtError();
659
}
660
} else {
661
try {
662
- commitAllHostEffects(root);
662
+ commitAllHostEffects();
663
} catch (e) {
664
didError = true;
665
error = e;
packages/react/src/__tests__/ReactProfiler-test.internal.js
+31
@@ -1092,6 +1092,37 @@ describe('Profiler', () => {
1092
expect(updateCall[3]).toBe(1); // base time
1093
expect(updateCall[4]).toBe(27); // start time
1094
});
1095
+
1096
+ it('should not be called until after mutations', () => {
1097
+ let classComponentMounted = false;
1098
+ const callback = jest.fn(
1099
+ (id, phase, actualDuration, baseDuration, startTime, commitTime) => {
1100
+ // Don't call this hook until after mutations
1101
+ expect(classComponentMounted).toBe(true);
1102
+ // But the commit time should reflect pre-mutation
1103
+ expect(commitTime).toBe(2);
1104
+ },
1105
+ );
1106
+
1107
+ class ClassComponent extends React.Component {
1108
+ componentDidMount() {
1109
+ advanceTimeBy(5);
1110
+ classComponentMounted = true;
1111
+ }
1112
+ render() {
1113
+ advanceTimeBy(2);
1114
+ return null;
1115
+ }
1116
+ }
1117
+
1118
+ ReactTestRenderer.create(
1119
+ <React.unstable_Profiler id="test" onRender={callback}>
1120
+ <ClassComponent />
1121
+ </React.unstable_Profiler>,
1122
+ );
1123
+
1124
+ expect(callback).toHaveBeenCalledTimes(1);
1125
+ });
1126
});
1127
});
1128