Skip to content

Commit 431bb0b

Browse files
authored
[DevTools] Mark Unknown Reasons for Suspending with a Note (facebook#34200)
We currently only track the reason something might suspend in development mode through debug info but this excludes some cases. As a result we can end up with boundary that suspends but has no cause. This tries to detect that and show a notice for why that might be. I'm also trying to make it work with old React versions to cover everything. In production we don't track any of this meta data like `_debugInfo`, `_debugThenable` etc. so after resolution there's no information to take from. Except suspensey images / css which we can track in prod too. We could track lazy component types already. We'd have to add something that tracks after the fact if something used a lazy child, child as a promise, hooks, etc. which doesn't exist today. So that's not backwards compatible and might add some perf/memory cost. However, another strategy is also to try to replay the components after the fact which could be backwards compatible. That's tricky for child position since there's so many rules for how to do that which would have to be replicated. If you're in development you get a different error. Given that we've added instrumentation very recently. If you're on an older development version of React, then you get a different error. Unfortunately I think my feature test is not quite perfect because it's tricky to test for the instrumentation I just added. facebook#34146 So I think for some prereleases that has `_debugOwner` but doesn't have that you'll get a misleading error. Finally, if you're in a modern development environment, the only reason we should have any gaps is because of throw-a-Promise. This will highlight it as missing. We can detect that something threw if a Suspense boundary commits with a RetryCache but since it's a WeakSet we can't look into it to see anything about what it might have been. I don't plan on doing anything to improve this since it would only apply to new versions of React anyway and it's just inherently flawed. So just deprecate it facebook#34032. Note that nothing in here can detect that we suspended Transition. So throwing at the root or in an update won't show that anywhere.
1 parent 5063b32 commit 431bb0b

File tree

8 files changed

+144
-2
lines changed

8 files changed

+144
-2
lines changed

packages/react-devtools-shared/src/backend/fiber/renderer.js

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import type {
1515
ReactIOInfo,
1616
ReactStackTrace,
1717
ReactCallSite,
18+
Wakeable,
1819
} from 'shared/ReactTypes';
1920

2021
import type {HooksTree} from 'react-debug-tools/src/ReactDebugHooks';
@@ -87,6 +88,10 @@ import {
8788
SUSPENSE_TREE_OPERATION_REMOVE,
8889
SUSPENSE_TREE_OPERATION_REORDER_CHILDREN,
8990
SUSPENSE_TREE_OPERATION_RESIZE,
91+
UNKNOWN_SUSPENDERS_NONE,
92+
UNKNOWN_SUSPENDERS_REASON_PRODUCTION,
93+
UNKNOWN_SUSPENDERS_REASON_OLD_VERSION,
94+
UNKNOWN_SUSPENDERS_REASON_THROWN_PROMISE,
9095
} from '../../constants';
9196
import {inspectHooksOfFiber} from 'react-debug-tools';
9297
import {
@@ -296,6 +301,9 @@ type SuspenseNode = {
296301
// Track whether any of the items in suspendedBy are unique this this Suspense boundaries or if they're all
297302
// also in the parent sets. This determine whether this could contribute in the loading sequence.
298303
hasUniqueSuspenders: boolean,
304+
// Track whether anything suspended in this boundary that we can't track either because it was using throw
305+
// a promise, an older version of React or because we're inspecting prod.
306+
hasUnknownSuspenders: boolean,
299307
};
300308

301309
function createSuspenseNode(
@@ -309,6 +317,7 @@ function createSuspenseNode(
309317
rects: null,
310318
suspendedBy: new Map(),
311319
hasUniqueSuspenders: false,
320+
hasUnknownSuspenders: false,
312321
});
313322
}
314323

@@ -2745,6 +2754,8 @@ export function attach(
27452754
parentSuspenseNode.hasUniqueSuspenders = true;
27462755
}
27472756
}
2757+
// We have observed at least one known reason this might have been suspended.
2758+
parentSuspenseNode.hasUnknownSuspenders = false;
27482759
// Suspending right below the root is not attributed to any particular component in UI
27492760
// other than the SuspenseNode and the HostRoot's FiberInstance.
27502761
const suspendedBy = parentInstance.suspendedBy;
@@ -2783,6 +2794,7 @@ export function attach(
27832794
// It can now be marked as having unique suspenders. We can skip its children
27842795
// since they'll still be blocked by this one.
27852796
node.hasUniqueSuspenders = true;
2797+
node.hasUnknownSuspenders = false;
27862798
} else if (node.firstChild !== null) {
27872799
node = node.firstChild;
27882800
continue;
@@ -3458,6 +3470,25 @@ export function attach(
34583470
insertSuspendedBy(asyncInfo);
34593471
}
34603472
3473+
function trackThrownPromisesFromRetryCache(
3474+
suspenseNode: SuspenseNode,
3475+
retryCache: ?WeakSet<Wakeable>,
3476+
): void {
3477+
if (retryCache != null) {
3478+
// If a Suspense boundary ever committed in fallback state with a retryCache, that
3479+
// suggests that something unique to that boundary was suspensey since otherwise
3480+
// it wouldn't have thrown and so never created the retryCache.
3481+
// Unfortunately if we don't have any DEV time debug info or debug thenables then
3482+
// we have no meta data to show. However, we still mark this Suspense boundary as
3483+
// participating in the loading sequence since apparently it can suspend.
3484+
suspenseNode.hasUniqueSuspenders = true;
3485+
// We have not seen any reason yet for why this suspense node might have been
3486+
// suspended but it clearly has been at some point. If we later discover a reason
3487+
// we'll clear this flag again.
3488+
suspenseNode.hasUnknownSuspenders = true;
3489+
}
3490+
}
3491+
34613492
function mountVirtualChildrenRecursively(
34623493
firstChild: Fiber,
34633494
lastChild: null | Fiber, // non-inclusive
@@ -3749,6 +3780,9 @@ export function attach(
37493780
} else if (fiber.tag === SuspenseComponent && OffscreenComponent === -1) {
37503781
// Legacy Suspense without the Offscreen wrapper. For the modern Suspense we just handle the
37513782
// Offscreen wrapper itself specially.
3783+
if (newSuspenseNode !== null) {
3784+
trackThrownPromisesFromRetryCache(newSuspenseNode, fiber.stateNode);
3785+
}
37523786
const isTimedOut = fiber.memoizedState !== null;
37533787
if (isTimedOut) {
37543788
// Special case: if Suspense mounts in a timed-out state,
@@ -3791,6 +3825,9 @@ export function attach(
37913825
'There should always be an Offscreen Fiber child in a Suspense boundary.',
37923826
);
37933827
}
3828+
3829+
trackThrownPromisesFromRetryCache(newSuspenseNode, fiber.stateNode);
3830+
37943831
const fallbackFiber = contentFiber.sibling;
37953832
37963833
// First update only the Offscreen boundary. I.e. the main content.
@@ -4600,6 +4637,18 @@ export function attach(
46004637
const prevWasHidden = isOffscreen && prevFiber.memoizedState !== null;
46014638
const nextIsHidden = isOffscreen && nextFiber.memoizedState !== null;
46024639
4640+
if (isLegacySuspense) {
4641+
if (
4642+
fiberInstance !== null &&
4643+
fiberInstance.suspenseNode !== null &&
4644+
(prevFiber.stateNode === null) !== (nextFiber.stateNode === null)
4645+
) {
4646+
trackThrownPromisesFromRetryCache(
4647+
fiberInstance.suspenseNode,
4648+
nextFiber.stateNode,
4649+
);
4650+
}
4651+
}
46034652
// The logic below is inspired by the code paths in updateSuspenseComponent()
46044653
// inside ReactFiberBeginWork in the React source code.
46054654
if (prevDidTimeout && nextDidTimeOut) {
@@ -4726,6 +4775,13 @@ export function attach(
47264775
const prevFallbackFiber = prevContentFiber.sibling;
47274776
const nextFallbackFiber = nextContentFiber.sibling;
47284777
4778+
if ((prevFiber.stateNode === null) !== (nextFiber.stateNode === null)) {
4779+
trackThrownPromisesFromRetryCache(
4780+
fiberInstance.suspenseNode,
4781+
nextFiber.stateNode,
4782+
);
4783+
}
4784+
47294785
// First update only the Offscreen boundary. I.e. the main content.
47304786
updateFlags |= updateVirtualChildrenRecursively(
47314787
nextContentFiber,
@@ -6100,6 +6156,23 @@ export function attach(
61006156
getNearestSuspenseNode(fiberInstance),
61016157
);
61026158
6159+
let unknownSuspenders = UNKNOWN_SUSPENDERS_NONE;
6160+
if (
6161+
fiberInstance.suspenseNode !== null &&
6162+
fiberInstance.suspenseNode.hasUnknownSuspenders &&
6163+
!isTimedOutSuspense
6164+
) {
6165+
// Something unknown threw to suspended this boundary. Let's figure out why that might be.
6166+
if (renderer.bundleType === 0) {
6167+
unknownSuspenders = UNKNOWN_SUSPENDERS_REASON_PRODUCTION;
6168+
} else if (!('_debugInfo' in fiber)) {
6169+
// TODO: We really should detect _debugThenable and the auto-instrumentation for lazy/thenables too.
6170+
unknownSuspenders = UNKNOWN_SUSPENDERS_REASON_OLD_VERSION;
6171+
} else {
6172+
unknownSuspenders = UNKNOWN_SUSPENDERS_REASON_THROWN_PROMISE;
6173+
}
6174+
}
6175+
61036176
return {
61046177
id: fiberInstance.id,
61056178
@@ -6164,6 +6237,7 @@ export function attach(
61646237
61656238
suspendedBy: suspendedBy,
61666239
suspendedByRange: suspendedByRange,
6240+
unknownSuspenders: unknownSuspenders,
61676241
61686242
// List of owners
61696243
owners,
@@ -6280,6 +6354,7 @@ export function attach(
62806354
serializeAsyncInfo(info, virtualInstance, null),
62816355
),
62826356
suspendedByRange: suspendedByRange,
6357+
unknownSuspenders: UNKNOWN_SUSPENDERS_NONE,
62836358
62846359
// List of owners
62856360
owners,

packages/react-devtools-shared/src/backend/legacy/renderer.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import {
3434
TREE_OPERATION_ADD,
3535
TREE_OPERATION_REMOVE,
3636
TREE_OPERATION_REORDER_CHILDREN,
37+
UNKNOWN_SUSPENDERS_NONE,
3738
} from '../../constants';
3839
import {decorateMany, forceUpdate, restoreMany} from './utils';
3940

@@ -860,6 +861,7 @@ export function attach(
860861
// Not supported in legacy renderers.
861862
suspendedBy: [],
862863
suspendedByRange: null,
864+
unknownSuspenders: UNKNOWN_SUSPENDERS_NONE,
863865

864866
// List of owners
865867
owners,

packages/react-devtools-shared/src/backend/types.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import type {TimelineDataExport} from 'react-devtools-timeline/src/types';
3434
import type {BackendBridge} from 'react-devtools-shared/src/bridge';
3535
import type {ReactFunctionLocation, ReactStackTrace} from 'shared/ReactTypes';
3636
import type Agent from './agent';
37+
import type {UnknownSuspendersReason} from '../constants';
3738

3839
type BundleType =
3940
| 0 // PROD
@@ -303,6 +304,7 @@ export type InspectedElement = {
303304
// Things that suspended this Instances
304305
suspendedBy: Object, // DehydratedData or Array<SerializedAsyncInfo>
305306
suspendedByRange: null | [number, number],
307+
unknownSuspenders: UnknownSuspendersReason,
306308

307309
// List of owners
308310
owners: Array<SerializedElement> | null,

packages/react-devtools-shared/src/backendAPI.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,7 @@ export function convertInspectedElementBackendToFrontend(
272272
warnings,
273273
suspendedBy,
274274
suspendedByRange,
275+
unknownSuspenders,
275276
nativeTag,
276277
} = inspectedElementBackend;
277278

@@ -317,6 +318,7 @@ export function convertInspectedElementBackendToFrontend(
317318
? []
318319
: hydratedSuspendedBy.map(backendToFrontendSerializedAsyncInfo),
319320
suspendedByRange,
321+
unknownSuspenders,
320322
nativeTag,
321323
};
322324

packages/react-devtools-shared/src/constants.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,13 @@ export const SUSPENSE_TREE_OPERATION_RESIZE = 11;
3232
export const PROFILING_FLAG_BASIC_SUPPORT = 0b01;
3333
export const PROFILING_FLAG_TIMELINE_SUPPORT = 0b10;
3434

35+
export const UNKNOWN_SUSPENDERS_NONE: UnknownSuspendersReason = 0; // If we had at least one debugInfo, then that might have been the reason.
36+
export const UNKNOWN_SUSPENDERS_REASON_PRODUCTION: UnknownSuspendersReason = 1; // We're running in prod. That might be why we had unknown suspenders.
37+
export const UNKNOWN_SUSPENDERS_REASON_OLD_VERSION: UnknownSuspendersReason = 2; // We're running an old version of React that doesn't have full coverage. That might be the reason.
38+
export const UNKNOWN_SUSPENDERS_REASON_THROWN_PROMISE: UnknownSuspendersReason = 3; // If we're in dev, didn't detect and debug info and still suspended (other than CSS/image) the only reason is thrown promise.
39+
40+
export opaque type UnknownSuspendersReason = 0 | 1 | 2 | 3;
41+
3542
export const LOCAL_STORAGE_DEFAULT_TAB_KEY = 'React::DevTools::defaultTab';
3643
export const LOCAL_STORAGE_COMPONENT_FILTER_PREFERENCES_KEY =
3744
'React::DevTools::componentFilters';

packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSharedStyles.css

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,15 @@
5252
min-width: 1rem;
5353
}
5454

55+
.InfoRow {
56+
border-top: 1px solid var(--color-border);
57+
padding: 0.5rem 1rem;
58+
}
59+
60+
.InfoRow:last-child {
61+
margin-bottom: -0.25rem;
62+
}
63+
5564
.CollapsableRow {
5665
border-top: 1px solid var(--color-border);
5766
}

packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspendedBy.js

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,13 @@ import type {
2727
} from 'react-devtools-shared/src/frontend/types';
2828
import type {FrontendBridge} from 'react-devtools-shared/src/bridge';
2929

30+
import {
31+
UNKNOWN_SUSPENDERS_NONE,
32+
UNKNOWN_SUSPENDERS_REASON_PRODUCTION,
33+
UNKNOWN_SUSPENDERS_REASON_OLD_VERSION,
34+
UNKNOWN_SUSPENDERS_REASON_THROWN_PROMISE,
35+
} from '../../../constants';
36+
3037
type RowProps = {
3138
bridge: FrontendBridge,
3239
element: Element,
@@ -295,7 +302,10 @@ export default function InspectedElementSuspendedBy({
295302
const {suspendedBy, suspendedByRange} = inspectedElement;
296303

297304
// Skip the section if nothing suspended this component.
298-
if (suspendedBy == null || suspendedBy.length === 0) {
305+
if (
306+
(suspendedBy == null || suspendedBy.length === 0) &&
307+
inspectedElement.unknownSuspenders === UNKNOWN_SUSPENDERS_NONE
308+
) {
299309
return null;
300310
}
301311

@@ -327,9 +337,41 @@ export default function InspectedElementSuspendedBy({
327337
minTime = maxTime - 25;
328338
}
329339

330-
const sortedSuspendedBy = suspendedBy.slice(0);
340+
const sortedSuspendedBy = suspendedBy === null ? [] : suspendedBy.slice(0);
331341
sortedSuspendedBy.sort(compareTime);
332342

343+
let unknownSuspenders = null;
344+
switch (inspectedElement.unknownSuspenders) {
345+
case UNKNOWN_SUSPENDERS_REASON_PRODUCTION:
346+
unknownSuspenders = (
347+
<div className={styles.InfoRow}>
348+
Something suspended but we don't know the exact reason in production
349+
builds of React. Test this in development mode to see exactly what
350+
might suspend.
351+
</div>
352+
);
353+
break;
354+
case UNKNOWN_SUSPENDERS_REASON_OLD_VERSION:
355+
unknownSuspenders = (
356+
<div className={styles.InfoRow}>
357+
Something suspended but we don't track all the necessary information
358+
in older versions of React. Upgrade to the latest version of React to
359+
see exactly what might suspend.
360+
</div>
361+
);
362+
break;
363+
case UNKNOWN_SUSPENDERS_REASON_THROWN_PROMISE:
364+
unknownSuspenders = (
365+
<div className={styles.InfoRow}>
366+
Something threw a Promise to suspend this boundary. It's likely an
367+
outdated version of a library that doesn't yet fully take advantage of
368+
use(). Upgrade your data fetching library to see exactly what might
369+
suspend.
370+
</div>
371+
);
372+
break;
373+
}
374+
333375
return (
334376
<div>
335377
<div className={styles.HeaderRow}>
@@ -351,6 +393,7 @@ export default function InspectedElementSuspendedBy({
351393
maxTime={maxTime}
352394
/>
353395
))}
396+
{unknownSuspenders}
354397
</div>
355398
);
356399
}

packages/react-devtools-shared/src/frontend/types.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import type {
1919
Unserializable,
2020
} from 'react-devtools-shared/src/hydration';
2121
import type {ReactFunctionLocation, ReactStackTrace} from 'shared/ReactTypes';
22+
import type {UnknownSuspendersReason} from '../constants';
2223

2324
export type BrowserTheme = 'dark' | 'light';
2425

@@ -283,6 +284,7 @@ export type InspectedElement = {
283284
suspendedBy: Object,
284285
// Minimum start time to maximum end time + a potential (not actual) throttle, within the nearest boundary.
285286
suspendedByRange: null | [number, number],
287+
unknownSuspenders: UnknownSuspendersReason,
286288

287289
// List of owners
288290
owners: Array<SerializedElement> | null,

0 commit comments

Comments
 (0)