Skip to content

Commit 4a63578

Browse files
gaearongaearon
authored
Fix User Timing oddities with Suspense, pure, and lazy (facebook#13833)
* Show pure components in fiber timings with name * Fix Suspense and lazy user timings * Tweak message and type name * Fix Flow
1 parent d270db1 commit 4a63578

File tree

5 files changed

+126
-8
lines changed

5 files changed

+126
-8
lines changed

‎packages/react-reconciler/src/ReactDebugFiberPerf.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
ContextProvider,
2121
ContextConsumer,
2222
Mode,
23+
SuspenseComponent,
2324
} from 'shared/ReactWorkTags';
2425

2526
type MeasurementPhase =
@@ -315,7 +316,10 @@ export function stopFailedWorkTimer(fiber: Fiber): void {
315316
return;
316317
}
317318
fiber._debugIsCurrentlyTiming = false;
318-
const warning = 'An error was thrown inside this error boundary';
319+
const warning =
320+
fiber.tag === SuspenseComponent
321+
? 'Rendering was suspended'
322+
: 'An error was thrown inside this error boundary';
319323
endFiberMark(fiber, null, warning);
320324
}
321325
}

‎packages/react-reconciler/src/ReactFiberBeginWork.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ import ReactStrictModeWarnings from './ReactStrictModeWarnings';
5656
import warning from 'shared/warning';
5757
import warningWithoutStack from 'shared/warningWithoutStack';
5858
import * as ReactCurrentFiber from './ReactCurrentFiber';
59-
import {cancelWorkTimer} from './ReactDebugFiberPerf';
59+
import {startWorkTimer, cancelWorkTimer} from './ReactDebugFiberPerf';
6060

6161
import {
6262
mountChildFibers,
@@ -720,11 +720,15 @@ function mountIndeterminateComponent(
720720
Component !== null &&
721721
typeof Component.then === 'function'
722722
) {
723+
// We can't start a User Timing measurement with correct label yet.
724+
// Cancel and resume right after we know the tag.
725+
cancelWorkTimer(workInProgress);
723726
Component = readLazyComponentType(Component);
724727
const resolvedTag = (workInProgress.tag = resolveLazyComponentTag(
725728
workInProgress,
726729
Component,
727730
));
731+
startWorkTimer(workInProgress);
728732
const resolvedProps = resolveDefaultProps(Component, props);
729733
let child;
730734
switch (resolvedTag) {

‎packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -555,6 +555,58 @@ describe('ReactDebugFiberPerf', () => {
555555
expect(getFlameChart()).toMatchSnapshot();
556556
});
557557

558+
it('supports pure', () => {
559+
const PureFoo = React.pure(function Foo() {
560+
return <div />;
561+
});
562+
ReactNoop.render(
563+
<Parent>
564+
<PureFoo />
565+
</Parent>,
566+
);
567+
ReactNoop.flush();
568+
expect(getFlameChart()).toMatchSnapshot();
569+
});
570+
571+
it('supports Suspense and lazy', async () => {
572+
function Spinner() {
573+
return <span />;
574+
}
575+
576+
let resolve;
577+
const LazyFoo = React.lazy(
578+
() =>
579+
new Promise(r => {
580+
resolve = r;
581+
}),
582+
);
583+
584+
ReactNoop.render(
585+
<Parent>
586+
<React.unstable_Suspense fallback={<Spinner />}>
587+
<LazyFoo />
588+
</React.unstable_Suspense>
589+
</Parent>,
590+
);
591+
ReactNoop.flush();
592+
expect(getFlameChart()).toMatchSnapshot();
593+
594+
resolve(function Foo() {
595+
return <div />;
596+
});
597+
await LazyFoo;
598+
599+
ReactNoop.render(
600+
<Parent>
601+
<React.unstable_Suspense>
602+
<LazyFoo />
603+
</React.unstable_Suspense>
604+
</Parent>,
605+
);
606+
ReactNoop.flush();
607+
expect(getFlameChart()).toMatchSnapshot();
608+
});
609+
558610
it('does not schedule an extra callback if setState is called during a synchronous commit phase', () => {
559611
class Component extends React.Component {
560612
state = {step: 1};

‎packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.internal.js.snap

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,40 @@ exports[`ReactDebugFiberPerf skips parents during setState 1`] = `
362362
"
363363
`;
364364

365+
exports[`ReactDebugFiberPerf supports Suspense and lazy 1`] = `
366+
"⚛ (Waiting for async callback... will force flush in 5250 ms)
367+
368+
⚛ (React Tree Reconciliation: Completed Root)
369+
⚛ Parent [mount]
370+
⛔ Suspense [mount] Warning: Rendering was suspended
371+
⚛ Suspense [mount]
372+
⚛ Spinner [mount]
373+
"
374+
`;
375+
376+
exports[`ReactDebugFiberPerf supports Suspense and lazy 2`] = `
377+
"⚛ (Waiting for async callback... will force flush in 5250 ms)
378+
379+
⚛ (React Tree Reconciliation: Completed Root)
380+
⚛ Parent [mount]
381+
⛔ Suspense [mount] Warning: Rendering was suspended
382+
⚛ Suspense [mount]
383+
⚛ Spinner [mount]
384+
385+
⚛ (Waiting for async callback... will force flush in 5250 ms)
386+
387+
⚛ (React Tree Reconciliation: Completed Root)
388+
⚛ Parent [mount]
389+
⚛ Suspense [mount]
390+
⚛ Foo [mount]
391+
392+
⚛ (Committing Changes)
393+
⚛ (Committing Snapshot Effects: 0 Total)
394+
⚛ (Committing Host Effects: 1 Total)
395+
⚛ (Calling Lifecycle Methods: 0 Total)
396+
"
397+
`;
398+
365399
exports[`ReactDebugFiberPerf supports portals 1`] = `
366400
"⚛ (Waiting for async callback... will force flush in 5250 ms)
367401
@@ -376,6 +410,20 @@ exports[`ReactDebugFiberPerf supports portals 1`] = `
376410
"
377411
`;
378412

413+
exports[`ReactDebugFiberPerf supports pure 1`] = `
414+
"⚛ (Waiting for async callback... will force flush in 5250 ms)
415+
416+
⚛ (React Tree Reconciliation: Completed Root)
417+
⚛ Parent [mount]
418+
⚛ Pure(Foo) [mount]
419+
420+
⚛ (Committing Changes)
421+
⚛ (Committing Snapshot Effects: 0 Total)
422+
⚛ (Committing Host Effects: 1 Total)
423+
⚛ (Calling Lifecycle Methods: 0 Total)
424+
"
425+
`;
426+
379427
exports[`ReactDebugFiberPerf warns if an in-progress update is interrupted 1`] = `
380428
"⚛ (Waiting for async callback... will force flush in 5250 ms)
381429

‎packages/shared/getComponentName.js

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,26 @@ import {
1616
REACT_FORWARD_REF_TYPE,
1717
REACT_FRAGMENT_TYPE,
1818
REACT_PORTAL_TYPE,
19+
REACT_PURE_TYPE,
1920
REACT_PROFILER_TYPE,
2021
REACT_PROVIDER_TYPE,
2122
REACT_STRICT_MODE_TYPE,
2223
REACT_SUSPENSE_TYPE,
2324
} from 'shared/ReactSymbols';
2425
import {refineResolvedThenable} from 'shared/ReactLazyComponent';
2526

27+
function getWrappedName(
28+
outerType: mixed,
29+
innerType: any,
30+
wrapperName: string,
31+
): string {
32+
const functionName = innerType.displayName || innerType.name || '';
33+
return (
34+
(outerType: any).displayName ||
35+
(functionName !== '' ? `${wrapperName}(${functionName})` : wrapperName)
36+
);
37+
}
38+
2639
function getComponentName(type: mixed): string | null {
2740
if (type == null) {
2841
// Host root, text node or just invalid type.
@@ -64,12 +77,9 @@ function getComponentName(type: mixed): string | null {
6477
case REACT_PROVIDER_TYPE:
6578
return 'Context.Provider';
6679
case REACT_FORWARD_REF_TYPE:
67-
const renderFn = (type.render: any);
68-
const functionName = renderFn.displayName || renderFn.name || '';
69-
return (
70-
(type: any).displayName ||
71-
(functionName !== '' ? `ForwardRef(${functionName})` : 'ForwardRef')
72-
);
80+
return getWrappedName(type, type.render, 'ForwardRef');
81+
case REACT_PURE_TYPE:
82+
return getWrappedName(type, type.render, 'Pure');
7383
}
7484
if (typeof type.then === 'function') {
7585
const thenable: Thenable<mixed> = (type: any);

0 commit comments

Comments
 (0)