From 552a5dadcfaee529b3ad8c8011c5d8f8f000ebf1 Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin <28902667+hoxyq@users.noreply.github.com> Date: Thu, 7 Aug 2025 14:05:56 +0100 Subject: [PATCH 1/4] [DevTools] fix: handle store mutations synchronously in TreeContext (#34119) If there is a commit that removes the currently inspected (selected) elements in the Components tree, we are going to kick off the transition to re-render the Tree. The elements will be re-rendered with the previous inspectedElementID, which was just removed and all consecutive calls to store object with this id would produce errors, since this element was just removed. We should handle store mutations synchronously. Doesn't make sense to start a transition in this case, because Elements depend on the TreeState and could make calls to store in render function. Before: Screenshot 2025-08-06 at 17 41 14 After: https://github.com/user-attachments/assets/3da36aff-6987-4b76-b741-ca59f829f8e6 --- .../src/devtools/views/Components/TreeContext.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-devtools-shared/src/devtools/views/Components/TreeContext.js b/packages/react-devtools-shared/src/devtools/views/Components/TreeContext.js index 797ffcd0f6e99..f43ced82447ef 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/TreeContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/TreeContext.js @@ -954,7 +954,7 @@ function TreeContextController({ Array, Map, ]) => { - transitionDispatch({ + dispatch({ type: 'HANDLE_STORE_MUTATION', payload: [addedElementIDs, removedElementIDs], }); @@ -965,7 +965,7 @@ function TreeContextController({ // At the moment, we can treat this as a mutation. // We don't know which Elements were newly added/removed, but that should be okay in this case. // It would only impact the search state, which is unlikely to exist yet at this point. - transitionDispatch({ + dispatch({ type: 'HANDLE_STORE_MUTATION', payload: [[], new Map()], }); From 4c9c109cea9be3622d9d70f81f96e72528bdad16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Thu, 7 Aug 2025 10:26:30 -0400 Subject: [PATCH 2/4] [Fiber] Try to give a stack trace to every entry in the Scheduler Performance Track (#34123) For "render" and "commit" phases we don't give any specific stack atm. This tries to always provide something useful to say the cause of the render. For normal renders this will now show the same thing as the "Event" and "Update" entries already showed. We stash the task that was used for those and use them throughout the render and commit phases. For Suspense (Retry lane) and Idle (Offscreen lane), we don't have any updates. Instead for those there's a component that left work behind in previous passes. For those I use the debugTask of the `` or `` boundary to indicate that this was the root of the render. Similarly when an Action is invoked on a `
` component using the built-in submit handler, there's no actionable stack in user space that called it. So we use the stack of the JSX for the form instead. --- .../src/ReactFiberBeginWork.js | 37 ++ .../react-reconciler/src/ReactFiberHooks.js | 7 +- .../src/ReactFiberPerformanceTrack.js | 477 +++++++++++++----- .../src/ReactFiberReconciler.js | 1 + .../src/ReactFiberWorkLoop.js | 85 +++- .../src/ReactProfilerTimer.js | 83 ++- 6 files changed, 546 insertions(+), 144 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 33b9e79edfaf3..7a3bb4ef814cc 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -158,6 +158,7 @@ import { DefaultHydrationLane, SomeRetryLane, includesSomeLane, + includesOnlyRetries, laneToLanes, removeLanes, mergeLanes, @@ -269,6 +270,7 @@ import { scheduleUpdateOnFiber, renderDidSuspendDelayIfPossible, markSkippedUpdateLanes, + markRenderDerivedCause, getWorkInProgressRoot, peekDeferredLane, } from './ReactFiberWorkLoop'; @@ -946,6 +948,13 @@ function updateDehydratedActivityComponent( // but after we've already committed once. warnIfHydrating(); + if (includesSomeLane(renderLanes, (OffscreenLane: Lane))) { + // If we're rendering Offscreen and we're entering the activity then it's possible + // that the only reason we rendered was because this boundary left work. Provide + // it as a cause if another one doesn't already exist. + markRenderDerivedCause(workInProgress); + } + if ( // TODO: Factoring is a little weird, since we check this right below, too. !didReceiveUpdate @@ -1132,6 +1141,16 @@ function updateActivityComponent( children: nextChildren, }; + if ( + includesSomeLane(renderLanes, (OffscreenLane: Lane)) && + includesSomeLane(renderLanes, current.lanes) + ) { + // If we're rendering Offscreen and we're entering the activity then it's possible + // that the only reason we rendered was because this boundary left work. Provide + // it as a cause if another one doesn't already exist. + markRenderDerivedCause(workInProgress); + } + const primaryChildFragment = updateWorkInProgressOffscreenFiber( currentChild, offscreenChildProps, @@ -2515,6 +2534,17 @@ function updateSuspenseComponent( workInProgress.memoizedState = SUSPENDED_MARKER; return fallbackChildFragment; } else { + if ( + prevState !== null && + includesOnlyRetries(renderLanes) && + includesSomeLane(renderLanes, current.lanes) + ) { + // If we're rendering Retry lanes and we're entering the primary content then it's possible + // that the only reason we rendered was because we left this boundary to be warmed up but + // nothing else scheduled an update. If so, use it as the cause of the render. + markRenderDerivedCause(workInProgress); + } + pushPrimaryTreeSuspenseHandler(workInProgress); const nextPrimaryChildren = nextProps.children; @@ -2873,6 +2903,13 @@ function updateDehydratedSuspenseComponent( // but after we've already committed once. warnIfHydrating(); + if (includesSomeLane(renderLanes, (OffscreenLane: Lane))) { + // If we're rendering Offscreen and we're entering the activity then it's possible + // that the only reason we rendered was because this boundary left work. Provide + // it as a cause if another one doesn't already exist. + markRenderDerivedCause(workInProgress); + } + if (isSuspenseInstanceFallback(suspenseInstance)) { // This boundary is in a permanent fallback state. In this case, we'll never // get an update and we'll never be able to hydrate the final content. Let's just try the diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 63332124e2f0b..940dff5e69a06 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -122,7 +122,10 @@ import { markStateUpdateScheduled, setIsStrictModeForDevtools, } from './ReactFiberDevToolsHook'; -import {startUpdateTimerByLane} from './ReactProfilerTimer'; +import { + startUpdateTimerByLane, + startHostActionTimer, +} from './ReactProfilerTimer'; import {createCache} from './ReactFiberCacheComponent'; import { createUpdate as createLegacyQueueUpdate, @@ -3239,6 +3242,8 @@ export function startHostTransition( BasicStateAction | TransitionStatus>, > = stateHook.queue; + startHostActionTimer(formFiber); + startTransition( formFiber, queue, diff --git a/packages/react-reconciler/src/ReactFiberPerformanceTrack.js b/packages/react-reconciler/src/ReactFiberPerformanceTrack.js index 4f70a589cb08d..f7415dbf46dcc 100644 --- a/packages/react-reconciler/src/ReactFiberPerformanceTrack.js +++ b/packages/react-reconciler/src/ReactFiberPerformanceTrack.js @@ -609,6 +609,7 @@ export function logBlockingStart( eventType: null | string, eventIsRepeat: boolean, isSpawnedUpdate: boolean, + isPingedUpdate: boolean, renderStartTime: number, lanes: Lanes, debugTask: null | ConsoleTask, // DEV-only @@ -658,11 +659,13 @@ export function logBlockingStart( // $FlowFixMe[method-unbinding] console.timeStamp.bind( console, - isSpawnedUpdate - ? 'Cascading Update' - : renderStartTime - updateTime > 5 - ? 'Update Blocked' - : 'Update', + isPingedUpdate + ? 'Promise Resolved' + : isSpawnedUpdate + ? 'Cascading Update' + : renderStartTime - updateTime > 5 + ? 'Update Blocked' + : 'Update', updateTime, renderStartTime, currentTrack, @@ -672,11 +675,13 @@ export function logBlockingStart( ); } else { console.timeStamp( - isSpawnedUpdate - ? 'Cascading Update' - : renderStartTime - updateTime > 5 - ? 'Update Blocked' - : 'Update', + isPingedUpdate + ? 'Promise Resolved' + : isSpawnedUpdate + ? 'Cascading Update' + : renderStartTime - updateTime > 5 + ? 'Update Blocked' + : 'Update', updateTime, renderStartTime, currentTrack, @@ -694,6 +699,7 @@ export function logTransitionStart( eventTime: number, eventType: null | string, eventIsRepeat: boolean, + isPingedUpdate: boolean, renderStartTime: number, debugTask: null | ConsoleTask, // DEV-only ): void { @@ -763,7 +769,11 @@ export function logTransitionStart( // $FlowFixMe[method-unbinding] console.timeStamp.bind( console, - renderStartTime - updateTime > 5 ? 'Update Blocked' : 'Update', + isPingedUpdate + ? 'Promise Resolved' + : renderStartTime - updateTime > 5 + ? 'Update Blocked' + : 'Update', updateTime, renderStartTime, currentTrack, @@ -773,7 +783,11 @@ export function logTransitionStart( ); } else { console.timeStamp( - renderStartTime - updateTime > 5 ? 'Update Blocked' : 'Update', + isPingedUpdate + ? 'Promise Resolved' + : renderStartTime - updateTime > 5 + ? 'Update Blocked' + : 'Update', updateTime, renderStartTime, currentTrack, @@ -789,23 +803,43 @@ export function logRenderPhase( startTime: number, endTime: number, lanes: Lanes, + debugTask: null | ConsoleTask, ): void { if (supportsUserTiming) { + if (endTime <= startTime) { + return; + } const color = includesOnlyHydrationOrOffscreenLanes(lanes) ? 'tertiary-dark' : 'primary-dark'; - console.timeStamp( - includesOnlyOffscreenLanes(lanes) - ? 'Prepared' - : includesOnlyHydrationLanes(lanes) - ? 'Hydrated' - : 'Render', - startTime, - endTime, - currentTrack, - LANES_TRACK_GROUP, - color, - ); + const label = includesOnlyOffscreenLanes(lanes) + ? 'Prepared' + : includesOnlyHydrationLanes(lanes) + ? 'Hydrated' + : 'Render'; + if (__DEV__ && debugTask) { + debugTask.run( + // $FlowFixMe[method-unbinding] + console.timeStamp.bind( + console, + label, + startTime, + endTime, + currentTrack, + LANES_TRACK_GROUP, + color, + ), + ); + } else { + console.timeStamp( + label, + startTime, + endTime, + currentTrack, + LANES_TRACK_GROUP, + color, + ); + } } } @@ -813,23 +847,43 @@ export function logInterruptedRenderPhase( startTime: number, endTime: number, lanes: Lanes, + debugTask: null | ConsoleTask, ): void { if (supportsUserTiming) { + if (endTime <= startTime) { + return; + } const color = includesOnlyHydrationOrOffscreenLanes(lanes) ? 'tertiary-dark' : 'primary-dark'; - console.timeStamp( - includesOnlyOffscreenLanes(lanes) - ? 'Prewarm' - : includesOnlyHydrationLanes(lanes) - ? 'Interrupted Hydration' - : 'Interrupted Render', - startTime, - endTime, - currentTrack, - LANES_TRACK_GROUP, - color, - ); + const label = includesOnlyOffscreenLanes(lanes) + ? 'Prewarm' + : includesOnlyHydrationLanes(lanes) + ? 'Interrupted Hydration' + : 'Interrupted Render'; + if (__DEV__ && debugTask) { + debugTask.run( + // $FlowFixMe[method-unbinding] + console.timeStamp.bind( + console, + label, + startTime, + endTime, + currentTrack, + LANES_TRACK_GROUP, + color, + ), + ); + } else { + console.timeStamp( + label, + startTime, + endTime, + currentTrack, + LANES_TRACK_GROUP, + color, + ); + } } } @@ -837,19 +891,38 @@ export function logSuspendedRenderPhase( startTime: number, endTime: number, lanes: Lanes, + debugTask: null | ConsoleTask, ): void { if (supportsUserTiming) { + if (endTime <= startTime) { + return; + } const color = includesOnlyHydrationOrOffscreenLanes(lanes) ? 'tertiary-dark' : 'primary-dark'; - console.timeStamp( - 'Prewarm', - startTime, - endTime, - currentTrack, - LANES_TRACK_GROUP, - color, - ); + if (__DEV__ && debugTask) { + debugTask.run( + // $FlowFixMe[method-unbinding] + console.timeStamp.bind( + console, + 'Prewarm', + startTime, + endTime, + currentTrack, + LANES_TRACK_GROUP, + color, + ), + ); + } else { + console.timeStamp( + 'Prewarm', + startTime, + endTime, + currentTrack, + LANES_TRACK_GROUP, + color, + ); + } } } @@ -857,20 +930,39 @@ export function logSuspendedWithDelayPhase( startTime: number, endTime: number, lanes: Lanes, + debugTask: null | ConsoleTask, ): void { // This means the render was suspended and cannot commit until it gets unblocked. if (supportsUserTiming) { + if (endTime <= startTime) { + return; + } const color = includesOnlyHydrationOrOffscreenLanes(lanes) ? 'tertiary-dark' : 'primary-dark'; - console.timeStamp( - 'Suspended', - startTime, - endTime, - currentTrack, - LANES_TRACK_GROUP, - color, - ); + if (__DEV__ && debugTask) { + debugTask.run( + // $FlowFixMe[method-unbinding] + console.timeStamp.bind( + console, + 'Suspended', + startTime, + endTime, + currentTrack, + LANES_TRACK_GROUP, + color, + ), + ); + } else { + console.timeStamp( + 'Suspended', + startTime, + endTime, + currentTrack, + LANES_TRACK_GROUP, + color, + ); + } } } @@ -880,8 +972,12 @@ export function logRecoveredRenderPhase( lanes: Lanes, recoverableErrors: Array>, hydrationFailed: boolean, + debugTask: null | ConsoleTask, ): void { if (supportsUserTiming) { + if (endTime <= startTime) { + return; + } if (__DEV__) { const properties: Array<[string, string]> = []; for (let i = 0; i < recoverableErrors.length; i++) { @@ -897,7 +993,7 @@ export function logRecoveredRenderPhase( String(error); properties.push(['Recoverable Error', message]); } - performance.measure('Recovered', { + const options = { start: startTime, end: endTime, detail: { @@ -911,7 +1007,15 @@ export function logRecoveredRenderPhase( properties, }, }, - }); + }; + if (debugTask) { + debugTask.run( + // $FlowFixMe[method-unbinding] + performance.measure.bind(performance, 'Recovered', options), + ); + } else { + performance.measure('Recovered', options); + } } else { console.timeStamp( 'Recovered', @@ -929,68 +1033,144 @@ export function logErroredRenderPhase( startTime: number, endTime: number, lanes: Lanes, + debugTask: null | ConsoleTask, ): void { if (supportsUserTiming) { - console.timeStamp( - 'Errored', - startTime, - endTime, - currentTrack, - LANES_TRACK_GROUP, - 'error', - ); + if (endTime <= startTime) { + return; + } + if (__DEV__ && debugTask) { + debugTask.run( + // $FlowFixMe[method-unbinding] + console.timeStamp.bind( + console, + 'Errored', + startTime, + endTime, + currentTrack, + LANES_TRACK_GROUP, + 'error', + ), + ); + } else { + console.timeStamp( + 'Errored', + startTime, + endTime, + currentTrack, + LANES_TRACK_GROUP, + 'error', + ); + } } } export function logInconsistentRender( startTime: number, endTime: number, + debugTask: null | ConsoleTask, ): void { if (supportsUserTiming) { - console.timeStamp( - 'Teared Render', - startTime, - endTime, - currentTrack, - LANES_TRACK_GROUP, - 'error', - ); + if (endTime <= startTime) { + return; + } + if (__DEV__ && debugTask) { + debugTask.run( + // $FlowFixMe[method-unbinding] + console.timeStamp.bind( + console, + 'Teared Render', + startTime, + endTime, + currentTrack, + LANES_TRACK_GROUP, + 'error', + ), + ); + } else { + console.timeStamp( + 'Teared Render', + startTime, + endTime, + currentTrack, + LANES_TRACK_GROUP, + 'error', + ); + } } } export function logSuspenseThrottlePhase( startTime: number, endTime: number, + debugTask: null | ConsoleTask, ): void { // This was inside a throttled Suspense boundary commit. if (supportsUserTiming) { - console.timeStamp( - 'Throttled', - startTime, - endTime, - currentTrack, - LANES_TRACK_GROUP, - 'secondary-light', - ); + if (endTime <= startTime) { + return; + } + if (__DEV__ && debugTask) { + debugTask.run( + // $FlowFixMe[method-unbinding] + console.timeStamp.bind( + console, + 'Throttled', + startTime, + endTime, + currentTrack, + LANES_TRACK_GROUP, + 'secondary-light', + ), + ); + } else { + console.timeStamp( + 'Throttled', + startTime, + endTime, + currentTrack, + LANES_TRACK_GROUP, + 'secondary-light', + ); + } } } export function logSuspendedCommitPhase( startTime: number, endTime: number, + debugTask: null | ConsoleTask, ): void { // This means the commit was suspended on CSS or images. if (supportsUserTiming) { + if (endTime <= startTime) { + return; + } // TODO: Include the exact reason and URLs of what resources suspended. // TODO: This might also be Suspended while waiting on a View Transition. - console.timeStamp( - 'Suspended on CSS or Images', - startTime, - endTime, - currentTrack, - LANES_TRACK_GROUP, - 'secondary-light', - ); + if (__DEV__ && debugTask) { + debugTask.run( + // $FlowFixMe[method-unbinding] + console.timeStamp.bind( + console, + 'Suspended on CSS or Images', + startTime, + endTime, + currentTrack, + LANES_TRACK_GROUP, + 'secondary-light', + ), + ); + } else { + console.timeStamp( + 'Suspended on CSS or Images', + startTime, + endTime, + currentTrack, + LANES_TRACK_GROUP, + 'secondary-light', + ); + } } } @@ -999,8 +1179,12 @@ export function logCommitErrored( endTime: number, errors: Array>, passive: boolean, + debugTask: null | ConsoleTask, ): void { if (supportsUserTiming) { + if (endTime <= startTime) { + return; + } if (__DEV__) { const properties: Array<[string, string]> = []; for (let i = 0; i < errors.length; i++) { @@ -1016,7 +1200,7 @@ export function logCommitErrored( String(error); properties.push(['Error', message]); } - performance.measure('Errored', { + const options = { start: startTime, end: endTime, detail: { @@ -1030,7 +1214,15 @@ export function logCommitErrored( properties, }, }, - }); + }; + if (debugTask) { + debugTask.run( + // $FlowFixMe[method-unbinding] + performance.measure.bind(performance, 'Errored', options), + ); + } else { + performance.measure('Errored', options); + } } else { console.timeStamp( 'Errored', @@ -1048,20 +1240,39 @@ export function logCommitPhase( startTime: number, endTime: number, errors: null | Array>, + debugTask: null | ConsoleTask, ): void { if (errors !== null) { - logCommitErrored(startTime, endTime, errors, false); + logCommitErrored(startTime, endTime, errors, false, debugTask); return; } if (supportsUserTiming) { - console.timeStamp( - 'Commit', - startTime, - endTime, - currentTrack, - LANES_TRACK_GROUP, - 'secondary-dark', - ); + if (endTime <= startTime) { + return; + } + if (__DEV__ && debugTask) { + debugTask.run( + // $FlowFixMe[method-unbinding] + console.timeStamp.bind( + console, + 'Commit', + startTime, + endTime, + currentTrack, + LANES_TRACK_GROUP, + 'secondary-dark', + ), + ); + } else { + console.timeStamp( + 'Commit', + startTime, + endTime, + currentTrack, + LANES_TRACK_GROUP, + 'secondary-dark', + ); + } } } @@ -1069,16 +1280,35 @@ export function logPaintYieldPhase( startTime: number, endTime: number, delayedUntilPaint: boolean, + debugTask: null | ConsoleTask, ): void { if (supportsUserTiming) { - console.timeStamp( - delayedUntilPaint ? 'Waiting for Paint' : '', - startTime, - endTime, - currentTrack, - LANES_TRACK_GROUP, - 'secondary-light', - ); + if (endTime <= startTime) { + return; + } + if (__DEV__ && debugTask) { + debugTask.run( + // $FlowFixMe[method-unbinding] + console.timeStamp.bind( + console, + delayedUntilPaint ? 'Waiting for Paint' : '', + startTime, + endTime, + currentTrack, + LANES_TRACK_GROUP, + 'secondary-light', + ), + ); + } else { + console.timeStamp( + delayedUntilPaint ? 'Waiting for Paint' : '', + startTime, + endTime, + currentTrack, + LANES_TRACK_GROUP, + 'secondary-light', + ); + } } } @@ -1086,19 +1316,38 @@ export function logPassiveCommitPhase( startTime: number, endTime: number, errors: null | Array>, + debugTask: null | ConsoleTask, ): void { if (errors !== null) { - logCommitErrored(startTime, endTime, errors, true); + logCommitErrored(startTime, endTime, errors, true, debugTask); return; } if (supportsUserTiming) { - console.timeStamp( - 'Remaining Effects', - startTime, - endTime, - currentTrack, - LANES_TRACK_GROUP, - 'secondary-dark', - ); + if (endTime <= startTime) { + return; + } + if (__DEV__ && debugTask) { + debugTask.run( + // $FlowFixMe[method-unbinding] + console.timeStamp.bind( + console, + 'Remaining Effects', + startTime, + endTime, + currentTrack, + LANES_TRACK_GROUP, + 'secondary-dark', + ), + ); + } else { + console.timeStamp( + 'Remaining Effects', + startTime, + endTime, + currentTrack, + LANES_TRACK_GROUP, + 'secondary-dark', + ); + } } } diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index 778518116ff2a..8d0f71d16b391 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -346,6 +346,7 @@ export function createHydrationContainer( update.callback = callback !== undefined && callback !== null ? callback : null; enqueueUpdate(current, update, lane); + startUpdateTimerByLane(lane, 'hydrateRoot()'); scheduleInitialHydrationOnRoot(root, lane); return root; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 1107c5bd4624d..e76083eb0a9fc 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -266,15 +266,16 @@ import { blockingClampTime, blockingUpdateTime, blockingUpdateTask, + blockingUpdateType, blockingEventTime, blockingEventType, blockingEventIsRepeat, - blockingSpawnedUpdate, blockingSuspendedTime, transitionClampTime, transitionStartTime, transitionUpdateTime, transitionUpdateTask, + transitionUpdateType, transitionEventTime, transitionEventType, transitionEventIsRepeat, @@ -301,6 +302,8 @@ import { startPingTimerByLanes, recordEffectError, resetCommitErrors, + PINGED_UPDATE, + SPAWNED_UPDATE, } from './ReactProfilerTimer'; // DEV stuff @@ -482,6 +485,9 @@ export function getWorkInProgressTransitions(): null | Array { return workInProgressTransitions; } +// The first setState call that eventually caused the current render. +let workInProgressUpdateTask: null | ConsoleTask = null; + let currentPendingTransitionCallbacks: PendingTransitionCallbacks | null = null; let currentEndTime: number | null = null; @@ -1104,7 +1110,11 @@ export function performWorkOnRoot( ) { if (enableProfilerTimer && enableComponentPerformanceTrack) { setCurrentTrackFromLanes(lanes); - logInconsistentRender(renderStartTime, renderEndTime); + logInconsistentRender( + renderStartTime, + renderEndTime, + workInProgressUpdateTask, + ); finalizeRender(lanes, renderEndTime); } // A store was mutated in an interleaved event. Render again, @@ -1130,7 +1140,12 @@ export function performWorkOnRoot( if (errorRetryLanes !== NoLanes) { if (enableProfilerTimer && enableComponentPerformanceTrack) { setCurrentTrackFromLanes(lanes); - logErroredRenderPhase(renderStartTime, renderEndTime, lanes); + logErroredRenderPhase( + renderStartTime, + renderEndTime, + lanes, + workInProgressUpdateTask, + ); finalizeRender(lanes, renderEndTime); } lanes = errorRetryLanes; @@ -1161,7 +1176,12 @@ export function performWorkOnRoot( if (exitStatus === RootFatalErrored) { if (enableProfilerTimer && enableComponentPerformanceTrack) { setCurrentTrackFromLanes(lanes); - logErroredRenderPhase(renderStartTime, renderEndTime, lanes); + logErroredRenderPhase( + renderStartTime, + renderEndTime, + lanes, + workInProgressUpdateTask, + ); finalizeRender(lanes, renderEndTime); } prepareFreshStack(root, NoLanes); @@ -1302,7 +1322,12 @@ function finishConcurrentRender( // until we receive more data. if (enableProfilerTimer && enableComponentPerformanceTrack) { setCurrentTrackFromLanes(lanes); - logSuspendedRenderPhase(renderStartTime, renderEndTime, lanes); + logSuspendedRenderPhase( + renderStartTime, + renderEndTime, + lanes, + workInProgressUpdateTask, + ); finalizeRender(lanes, renderEndTime); trackSuspendedTime(lanes, renderEndTime); } @@ -1867,18 +1892,22 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber { previousRenderStartTime, renderStartTime, lanes, + workInProgressUpdateTask, ); } else { logInterruptedRenderPhase( previousRenderStartTime, renderStartTime, lanes, + workInProgressUpdateTask, ); } finalizeRender(workInProgressRootRenderLanes, renderStartTime); } + workInProgressUpdateTask = null; if (includesSyncLane(lanes) || includesBlockingLane(lanes)) { + workInProgressUpdateTask = blockingUpdateTask; const clampedUpdateTime = blockingUpdateTime >= 0 && blockingUpdateTime < blockingClampTime ? blockingClampTime @@ -1898,6 +1927,7 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber { ? clampedUpdateTime : renderStartTime, lanes, + workInProgressUpdateTask, ); } logBlockingStart( @@ -1905,7 +1935,8 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber { clampedEventTime, blockingEventType, blockingEventIsRepeat, - blockingSpawnedUpdate, + blockingUpdateType === SPAWNED_UPDATE, + blockingUpdateType === PINGED_UPDATE, renderStartTime, lanes, blockingUpdateTask, @@ -1913,6 +1944,7 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber { clearBlockingTimers(); } if (includesTransitionLane(lanes)) { + workInProgressUpdateTask = transitionUpdateTask; const clampedStartTime = transitionStartTime >= 0 && transitionStartTime < transitionClampTime ? transitionClampTime @@ -1936,6 +1968,7 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber { ? clampedUpdateTime : renderStartTime, lanes, + workInProgressUpdateTask, ); } logTransitionStart( @@ -1944,6 +1977,7 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber { clampedEventTime, transitionEventType, transitionEventIsRepeat, + transitionUpdateType === PINGED_UPDATE, renderStartTime, transitionUpdateTask, ); @@ -2227,6 +2261,21 @@ function popAsyncDispatcher(prevAsyncDispatcher: any) { ReactSharedInternals.A = prevAsyncDispatcher; } +export function markRenderDerivedCause(fiber: Fiber): void { + if (enableProfilerTimer && enableComponentPerformanceTrack) { + if (__DEV__) { + if (workInProgressUpdateTask === null) { + // If we don't have a cause associated with this render, it's likely because some + // other render left work behind on this Fiber. The real cause is this Fiber itself. + // We use its debugTask as the cause for this render. This might not be the only + // one when multiple siblings are rendered but they ideally shouldn't be. + workInProgressUpdateTask = + fiber._debugTask == null ? null : fiber._debugTask; + } + } + } +} + export function markCommitTimeOfFallback() { globalMostRecentFallbackTime = now(); } @@ -3239,6 +3288,7 @@ function commitRoot( completedRenderStartTime, completedRenderEndTime, lanes, + workInProgressUpdateTask, ); } else if (recoverableErrors !== null) { const hydrationFailed = @@ -3252,9 +3302,15 @@ function commitRoot( lanes, recoverableErrors, hydrationFailed, + workInProgressUpdateTask, ); } else { - logRenderPhase(completedRenderStartTime, completedRenderEndTime, lanes); + logRenderPhase( + completedRenderStartTime, + completedRenderEndTime, + lanes, + workInProgressUpdateTask, + ); } } @@ -3425,9 +3481,17 @@ function commitRoot( recordCommitTime(); if (enableComponentPerformanceTrack) { if (suspendedCommitReason === SUSPENDED_COMMIT) { - logSuspendedCommitPhase(completedRenderEndTime, commitStartTime); + logSuspendedCommitPhase( + completedRenderEndTime, + commitStartTime, + workInProgressUpdateTask, + ); } else if (suspendedCommitReason === THROTTLED_COMMIT) { - logSuspenseThrottlePhase(completedRenderEndTime, commitStartTime); + logSuspenseThrottlePhase( + completedRenderEndTime, + commitStartTime, + workInProgressUpdateTask, + ); } } } @@ -3672,6 +3736,7 @@ function flushSpawnedWork(): void { : commitStartTime, commitEndTime, commitErrors, + workInProgressUpdateTask, ); } @@ -4147,6 +4212,7 @@ function flushPassiveEffectsImpl(wasDelayedCommit: void | boolean) { commitEndTime, passiveEffectStartTime, !!wasDelayedCommit, + workInProgressUpdateTask, ); } @@ -4182,6 +4248,7 @@ function flushPassiveEffectsImpl(wasDelayedCommit: void | boolean) { passiveEffectStartTime, passiveEffectsEndTime, commitErrors, + workInProgressUpdateTask, ); finalizeRender(lanes, passiveEffectsEndTime); } diff --git a/packages/react-reconciler/src/ReactProfilerTimer.js b/packages/react-reconciler/src/ReactProfilerTimer.js index b46209a6e6ff4..4768ffffb5635 100644 --- a/packages/react-reconciler/src/ReactProfilerTimer.js +++ b/packages/react-reconciler/src/ReactProfilerTimer.js @@ -48,6 +48,11 @@ const createTask = console.createTask : (name: string) => null; +export const REGULAR_UPDATE: UpdateType = 0; +export const SPAWNED_UPDATE: UpdateType = 1; +export const PINGED_UPDATE: UpdateType = 2; +export opaque type UpdateType = 0 | 1 | 2; + export let renderStartTime: number = -0; export let commitStartTime: number = -0; export let commitEndTime: number = -0; @@ -62,15 +67,16 @@ export let componentEffectErrors: null | Array> = null; export let blockingClampTime: number = -0; export let blockingUpdateTime: number = -1.1; // First sync setState scheduled. export let blockingUpdateTask: null | ConsoleTask = null; // First sync setState's stack trace. +export let blockingUpdateType: UpdateType = 0; export let blockingEventTime: number = -1.1; // Event timeStamp of the first setState. export let blockingEventType: null | string = null; // Event type of the first setState. export let blockingEventIsRepeat: boolean = false; -export let blockingSpawnedUpdate: boolean = false; export let blockingSuspendedTime: number = -1.1; // TODO: This should really be one per Transition lane. export let transitionClampTime: number = -0; export let transitionStartTime: number = -1.1; // First startTransition call before setState. export let transitionUpdateTime: number = -1.1; // First transition setState scheduled. +export let transitionUpdateType: UpdateType = 0; export let transitionUpdateTask: null | ConsoleTask = null; // First transition setState's stack trace. export let transitionEventTime: number = -1.1; // Event timeStamp of the first transition. export let transitionEventType: null | string = null; // Event type of the first transition. @@ -97,7 +103,7 @@ export function startUpdateTimerByLane(lane: Lane, method: string): void { blockingUpdateTime = now(); blockingUpdateTask = createTask(method); if (isAlreadyRendering()) { - blockingSpawnedUpdate = true; + blockingUpdateType = SPAWNED_UPDATE; } const newEventTime = resolveEventTimeStamp(); const newEventType = resolveEventType(); @@ -110,7 +116,7 @@ export function startUpdateTimerByLane(lane: Lane, method: string): void { // If this is a second update in the same event, we treat it as a spawned update. // This might be a microtask spawned from useEffect, multiple flushSync or // a setState in a microtask spawned after the first setState. Regardless it's bad. - blockingSpawnedUpdate = true; + blockingUpdateType = SPAWNED_UPDATE; } blockingEventTime = newEventTime; blockingEventType = newEventType; @@ -135,6 +141,54 @@ export function startUpdateTimerByLane(lane: Lane, method: string): void { } } +export function startHostActionTimer(fiber: Fiber): void { + if (!enableProfilerTimer || !enableComponentPerformanceTrack) { + return; + } + // This schedules an update on both the blocking lane for the pending state and on the + // transition lane for the action update. Using the debug task from the host fiber. + if (blockingUpdateTime < 0) { + blockingUpdateTime = now(); + blockingUpdateTask = + __DEV__ && fiber._debugTask != null ? fiber._debugTask : null; + if (isAlreadyRendering()) { + blockingUpdateType = SPAWNED_UPDATE; + } + const newEventTime = resolveEventTimeStamp(); + const newEventType = resolveEventType(); + if ( + newEventTime !== blockingEventTime || + newEventType !== blockingEventType + ) { + blockingEventIsRepeat = false; + } else if (newEventType !== null) { + // If this is a second update in the same event, we treat it as a spawned update. + // This might be a microtask spawned from useEffect, multiple flushSync or + // a setState in a microtask spawned after the first setState. Regardless it's bad. + blockingUpdateType = SPAWNED_UPDATE; + } + blockingEventTime = newEventTime; + blockingEventType = newEventType; + } + if (transitionUpdateTime < 0) { + transitionUpdateTime = now(); + transitionUpdateTask = + __DEV__ && fiber._debugTask != null ? fiber._debugTask : null; + if (transitionStartTime < 0) { + const newEventTime = resolveEventTimeStamp(); + const newEventType = resolveEventType(); + if ( + newEventTime !== transitionEventTime || + newEventType !== transitionEventType + ) { + transitionEventIsRepeat = false; + } + transitionEventTime = newEventTime; + transitionEventType = newEventType; + } + } +} + export function startPingTimerByLanes(lanes: Lanes): void { if (!enableProfilerTimer || !enableComponentPerformanceTrack) { return; @@ -145,10 +199,14 @@ export function startPingTimerByLanes(lanes: Lanes): void { if (includesSyncLane(lanes) || includesBlockingLane(lanes)) { if (blockingUpdateTime < 0) { blockingClampTime = blockingUpdateTime = now(); + blockingUpdateTask = createTask('Promise Resolved'); + blockingUpdateType = PINGED_UPDATE; } } else if (includesTransitionLane(lanes)) { if (transitionUpdateTime < 0) { transitionClampTime = transitionUpdateTime = now(); + transitionUpdateTask = createTask('Promise Resolved'); + transitionUpdateType = PINGED_UPDATE; } } } @@ -166,10 +224,9 @@ export function trackSuspendedTime(lanes: Lanes, renderEndTime: number) { export function clearBlockingTimers(): void { blockingUpdateTime = -1.1; - blockingUpdateTask = null; + blockingUpdateType = 0; blockingSuspendedTime = -1.1; blockingEventIsRepeat = true; - blockingSpawnedUpdate = false; } export function startAsyncTransitionTimer(): void { @@ -196,20 +253,6 @@ export function hasScheduledTransitionWork(): boolean { return transitionUpdateTime > -1; } -// We use this marker to indicate that we have scheduled a render to be performed -// but it's not an explicit state update. -const ACTION_STATE_MARKER = -0.5; - -export function startActionStateUpdate(): void { - if (!enableProfilerTimer || !enableComponentPerformanceTrack) { - return; - } - if (transitionUpdateTime < 0) { - transitionUpdateTime = ACTION_STATE_MARKER; - transitionUpdateTask = null; - } -} - export function clearAsyncTransitionTimer(): void { transitionStartTime = -1.1; } @@ -217,7 +260,7 @@ export function clearAsyncTransitionTimer(): void { export function clearTransitionTimers(): void { transitionStartTime = -1.1; transitionUpdateTime = -1.1; - transitionUpdateTask = null; + transitionUpdateType = 0; transitionSuspendedTime = -1.1; transitionEventIsRepeat = true; } From 738aebdbacff9becd469b888209a08436f87c511 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Thu, 7 Aug 2025 10:39:08 -0400 Subject: [PATCH 3/4] [DevTools] Add Badge to Owners and sometimes stack traces (#34106) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stacked on #34101. This adds a badge to owners if they are different from the currently selected component's environment. Screenshot 2025-08-04 at 5 15 02 PM We also add one to the end of stack traces if the stack trace has a different environment than the owner which can happen when you call a function (without rendering a component) into a third party environment but the owner component was in the first party. One awkward thing is that Suspense boundaries are always in the client environment so their Server Components are always badged. --- .../src/__tests__/profilingCache-test.js | 3 ++ .../src/backend/fiber/renderer.js | 6 ++++ .../src/backend/legacy/renderer.js | 3 ++ .../src/backend/types.js | 5 +++ .../react-devtools-shared/src/backendAPI.js | 2 ++ .../views/Components/ElementBadges.js | 7 +++- .../Components/InspectedElementSuspendedBy.js | 32 +++++++++++++++++-- .../views/Components/InspectedElementView.js | 3 ++ .../devtools/views/Components/OwnerView.js | 3 ++ .../devtools/views/Components/OwnersStack.js | 2 ++ .../views/Components/StackTraceView.js | 26 +++++++++++++-- .../src/frontend/types.js | 4 +++ 12 files changed, 90 insertions(+), 6 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/profilingCache-test.js b/packages/react-devtools-shared/src/__tests__/profilingCache-test.js index c0f0804f416af..795f37183a81f 100644 --- a/packages/react-devtools-shared/src/__tests__/profilingCache-test.js +++ b/packages/react-devtools-shared/src/__tests__/profilingCache-test.js @@ -862,6 +862,7 @@ describe('ProfilingCache', () => { { "compiledWithForget": false, "displayName": "render()", + "env": null, "hocDisplayNames": null, "id": 1, "key": null, @@ -903,6 +904,7 @@ describe('ProfilingCache', () => { { "compiledWithForget": false, "displayName": "createRoot()", + "env": null, "hocDisplayNames": null, "id": 1, "key": null, @@ -943,6 +945,7 @@ describe('ProfilingCache', () => { { "compiledWithForget": false, "displayName": "createRoot()", + "env": null, "hocDisplayNames": null, "id": 1, "key": null, diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index aab9476db3074..b26da3530bf9b 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -4818,6 +4818,7 @@ export function attach( displayName: getDisplayNameForFiber(fiber) || 'Anonymous', id: instance.id, key: fiber.key, + env: null, type: getElementTypeForFiber(fiber), }; } else { @@ -4826,6 +4827,7 @@ export function attach( displayName: componentInfo.name || 'Anonymous', id: instance.id, key: componentInfo.key == null ? null : componentInfo.key, + env: componentInfo.env == null ? null : componentInfo.env, type: ElementTypeVirtual, }; } @@ -5451,6 +5453,8 @@ export function attach( // List of owners owners, + env: null, + rootType, rendererPackageName: renderer.rendererPackageName, rendererVersion: renderer.version, @@ -5554,6 +5558,8 @@ export function attach( // List of owners owners, + env: componentInfo.env == null ? null : componentInfo.env, + rootType, rendererPackageName: renderer.rendererPackageName, rendererVersion: renderer.version, diff --git a/packages/react-devtools-shared/src/backend/legacy/renderer.js b/packages/react-devtools-shared/src/backend/legacy/renderer.js index d2b846bee24be..6153e08832a11 100644 --- a/packages/react-devtools-shared/src/backend/legacy/renderer.js +++ b/packages/react-devtools-shared/src/backend/legacy/renderer.js @@ -795,6 +795,7 @@ export function attach( displayName: getData(owner).displayName || 'Unknown', id: getID(owner), key: element.key, + env: null, type: getElementType(owner), }); if (owner._currentElement) { @@ -857,6 +858,8 @@ export function attach( // List of owners owners, + env: null, + rootType: null, rendererPackageName: null, rendererVersion: null, diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js index 6324e63da1490..585654252da20 100644 --- a/packages/react-devtools-shared/src/backend/types.js +++ b/packages/react-devtools-shared/src/backend/types.js @@ -256,6 +256,7 @@ export type SerializedElement = { displayName: string | null, id: number, key: number | string | null, + env: null | string, type: ElementType, }; @@ -301,6 +302,10 @@ export type InspectedElement = { // List of owners owners: Array | null, + + // Environment name that this component executed in or null for the client + env: string | null, + source: ReactFunctionLocation | null, type: ElementType, diff --git a/packages/react-devtools-shared/src/backendAPI.js b/packages/react-devtools-shared/src/backendAPI.js index d6aa18cd31715..a27e70c26d008 100644 --- a/packages/react-devtools-shared/src/backendAPI.js +++ b/packages/react-devtools-shared/src/backendAPI.js @@ -255,6 +255,7 @@ export function convertInspectedElementBackendToFrontend( id, type, owners, + env, source, context, hooks, @@ -299,6 +300,7 @@ export function convertInspectedElementBackendToFrontend( owners === null ? null : owners.map(backendToFrontendSerializedElementMapper), + env, context: hydrateHelper(context), hooks: hydrateHelper(hooks), props: hydrateHelper(props), diff --git a/packages/react-devtools-shared/src/devtools/views/Components/ElementBadges.js b/packages/react-devtools-shared/src/devtools/views/Components/ElementBadges.js index a829ad0153aa7..5a3355c60c491 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/ElementBadges.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/ElementBadges.js @@ -16,18 +16,21 @@ import styles from './ElementBadges.css'; type Props = { hocDisplayNames: Array | null, + environmentName: string | null, compiledWithForget: boolean, className?: string, }; export default function ElementBadges({ compiledWithForget, + environmentName, hocDisplayNames, className = '', }: Props): React.Node { if ( !compiledWithForget && - (hocDisplayNames == null || hocDisplayNames.length === 0) + (hocDisplayNames == null || hocDisplayNames.length === 0) && + environmentName == null ) { return null; } @@ -36,6 +39,8 @@ export default function ElementBadges({
{compiledWithForget && } + {environmentName != null ? {environmentName} : null} + {hocDisplayNames != null && hocDisplayNames.length > 0 && ( {hocDisplayNames[0]} )} diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspendedBy.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspendedBy.js index a1b76e49b6add..c24dd881e9891 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspendedBy.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspendedBy.js @@ -150,13 +150,28 @@ function SuspendedByRow({ {isOpen && (
- {showIOStack && } + {showIOStack && ( + + )} {(showIOStack || !showAwaitStack) && ioOwner !== null && ioOwner.id !== inspectedElement.id ? (
awaited at:
{asyncInfo.stack !== null && asyncInfo.stack.length > 0 && ( - + )} {asyncOwner !== null && asyncOwner.id !== inspectedElement.id ? ( | null, + environmentName: string | null, compiledWithForget: boolean, id: number, isInStore: boolean, @@ -27,6 +28,7 @@ type OwnerViewProps = { export default function OwnerView({ displayName, + environmentName, hocDisplayNames, compiledWithForget, id, @@ -65,6 +67,7 @@ export default function OwnerView({ diff --git a/packages/react-devtools-shared/src/devtools/views/Components/OwnersStack.js b/packages/react-devtools-shared/src/devtools/views/Components/OwnersStack.js index 0fa5c0910bb6e..09bdb96af01cd 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/OwnersStack.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/OwnersStack.js @@ -220,6 +220,7 @@ function ElementsDropdown({owners, selectOwner}: ElementsDropdownProps) { @@ -268,6 +269,7 @@ function ElementView({isSelected, owner, selectOwner}: ElementViewProps) { diff --git a/packages/react-devtools-shared/src/devtools/views/Components/StackTraceView.js b/packages/react-devtools-shared/src/devtools/views/Components/StackTraceView.js index 4352ad6a827d9..fdbdba702dafb 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/StackTraceView.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/StackTraceView.js @@ -12,6 +12,8 @@ import {use, useContext} from 'react'; import useOpenResource from '../useOpenResource'; +import ElementBadges from './ElementBadges'; + import styles from './StackTraceView.css'; import type { @@ -28,9 +30,13 @@ import formatLocationForDisplay from './formatLocationForDisplay'; type CallSiteViewProps = { callSite: ReactCallSite, + environmentName: null | string, }; -export function CallSiteView({callSite}: CallSiteViewProps): React.Node { +export function CallSiteView({ + callSite, + environmentName, +}: CallSiteViewProps): React.Node { const fetchFileWithCaching = useContext(FetchFileWithCachingContext); const [virtualFunctionName, virtualURL, virtualLine, virtualColumn] = @@ -64,19 +70,33 @@ export function CallSiteView({callSite}: CallSiteViewProps): React.Node { title={url + ':' + line}> {formatLocationForDisplay(url, line, column)} +
); } type Props = { stack: ReactStackTrace, + environmentName: null | string, }; -export default function StackTraceView({stack}: Props): React.Node { +export default function StackTraceView({ + stack, + environmentName, +}: Props): React.Node { return (
{stack.map((callSite, index) => ( - + ))}
); diff --git a/packages/react-devtools-shared/src/frontend/types.js b/packages/react-devtools-shared/src/frontend/types.js index 622205dd672b7..e4a4c5400bfd5 100644 --- a/packages/react-devtools-shared/src/frontend/types.js +++ b/packages/react-devtools-shared/src/frontend/types.js @@ -208,6 +208,7 @@ export type SerializedElement = { displayName: string | null, id: number, key: number | string | null, + env: null | string, hocDisplayNames: Array | null, compiledWithForget: boolean, type: ElementType, @@ -265,6 +266,9 @@ export type InspectedElement = { // List of owners owners: Array | null, + // Environment name that this component executed in or null for the client + env: string | null, + // Location of component in source code. source: ReactFunctionLocation | null, From 3958d5d84b3d3e6ae5c1caef9c8cf0dc58e862e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Thu, 7 Aug 2025 10:55:01 -0400 Subject: [PATCH 4/4] [Flight] Copy the name field of a serialized function debug value (#34085) This ensures that if the name is set manually after the declaration, then we get that name when we log the value. For example Node.js `Response` is declared as `_Response` and then later assigned a new name. We should probably really serialize all static enumerable properties but "name" is non-enumerable so it's still a special case. --- .../react-client/src/ReactFlightClient.js | 94 +++++++++++++------ .../src/__tests__/ReactFlight-test.js | 3 + .../react-server/src/ReactFlightServer.js | 13 ++- 3 files changed, 78 insertions(+), 32 deletions(-) diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index fe88fe0357be7..c4d47bfecc55d 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -1969,6 +1969,44 @@ function createModel(response: Response, model: any): any { return model; } +const mightHaveStaticConstructor = /\bclass\b.*\bstatic\b/; + +function getInferredFunctionApproximate(code: string): () => void { + let slicedCode; + if (code.startsWith('Object.defineProperty(')) { + slicedCode = code.slice('Object.defineProperty('.length); + } else if (code.startsWith('(')) { + slicedCode = code.slice(1); + } else { + slicedCode = code; + } + if (slicedCode.startsWith('async function')) { + const idx = slicedCode.indexOf('(', 14); + if (idx !== -1) { + const name = slicedCode.slice(14, idx).trim(); + // eslint-disable-next-line no-eval + return (0, eval)('({' + JSON.stringify(name) + ':async function(){}})')[ + name + ]; + } + } else if (slicedCode.startsWith('function')) { + const idx = slicedCode.indexOf('(', 8); + if (idx !== -1) { + const name = slicedCode.slice(8, idx).trim(); + // eslint-disable-next-line no-eval + return (0, eval)('({' + JSON.stringify(name) + ':function(){}})')[name]; + } + } else if (slicedCode.startsWith('class')) { + const idx = slicedCode.indexOf('{', 5); + if (idx !== -1) { + const name = slicedCode.slice(5, idx).trim(); + // eslint-disable-next-line no-eval + return (0, eval)('({' + JSON.stringify(name) + ':class{}})')[name]; + } + } + return function () {}; +} + function parseModelString( response: Response, parentObject: Object, @@ -2158,41 +2196,37 @@ function parseModelString( // This should not compile to eval() because then it has local scope access. const code = value.slice(2); try { - // eslint-disable-next-line no-eval - return (0, eval)(code); + // If this might be a class constructor with a static initializer or + // static constructor then don't eval it. It might cause unexpected + // side-effects. Instead, fallback to parsing out the function type + // and name. + if (!mightHaveStaticConstructor.test(code)) { + // eslint-disable-next-line no-eval + return (0, eval)(code); + } } catch (x) { - // We currently use this to express functions so we fail parsing it, - // let's just return a blank function as a place holder. - if (code.startsWith('(async function')) { - const idx = code.indexOf('(', 15); - if (idx !== -1) { - const name = code.slice(15, idx).trim(); - // eslint-disable-next-line no-eval - return (0, eval)( - '({' + JSON.stringify(name) + ':async function(){}})', - )[name]; - } - } else if (code.startsWith('(function')) { - const idx = code.indexOf('(', 9); - if (idx !== -1) { - const name = code.slice(9, idx).trim(); - // eslint-disable-next-line no-eval - return (0, eval)( - '({' + JSON.stringify(name) + ':function(){}})', - )[name]; - } - } else if (code.startsWith('(class')) { - const idx = code.indexOf('{', 6); + // Fallthrough to fallback case. + } + // We currently use this to express functions so we fail parsing it, + // let's just return a blank function as a place holder. + let fn; + try { + fn = getInferredFunctionApproximate(code); + if (code.startsWith('Object.defineProperty(')) { + const DESCRIPTOR = ',"name",{value:"'; + const idx = code.lastIndexOf(DESCRIPTOR); if (idx !== -1) { - const name = code.slice(6, idx).trim(); - // eslint-disable-next-line no-eval - return (0, eval)('({' + JSON.stringify(name) + ':class{}})')[ - name - ]; + const name = JSON.parse( + code.slice(idx + DESCRIPTOR.length - 1, code.length - 2), + ); + // $FlowFixMe[cannot-write] + Object.defineProperty(fn, 'name', {value: name}); } } - return function () {}; + } catch (_) { + fn = function () {}; } + return fn; } // Fallthrough } diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index 150997c1c3746..9a60c3bd66b29 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/packages/react-client/src/__tests__/ReactFlight-test.js @@ -3239,6 +3239,8 @@ describe('ReactFlight', () => { } Object.defineProperty(MyClass.prototype, 'y', {enumerable: true}); + Object.defineProperty(MyClass, 'name', {value: 'MyClassName'}); + function ServerComponent() { console.log('hi', { prop: 123, @@ -3341,6 +3343,7 @@ describe('ReactFlight', () => { const instance = mockConsoleLog.mock.calls[0][1].instance; expect(typeof Class).toBe('function'); expect(Class.prototype.constructor).toBe(Class); + expect(Class.name).toBe('MyClassName'); expect(instance instanceof Class).toBe(true); expect(Object.getPrototypeOf(instance)).toBe(Class.prototype); expect(instance.x).toBe(1); diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 1a5cee7ba7245..19a49dcd32b98 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -4848,9 +4848,18 @@ function renderDebugModel( return existingReference; } + // $FlowFixMe[method-unbinding] + const functionBody: string = Function.prototype.toString.call(value); + + const name = value.name; const serializedValue = serializeEval( - // $FlowFixMe[method-unbinding] - '(' + Function.prototype.toString.call(value) + ')', + typeof name === 'string' + ? 'Object.defineProperty(' + + functionBody + + ',"name",{value:' + + JSON.stringify(name) + + '})' + : '(' + functionBody + ')', ); request.pendingDebugChunks++; const id = request.nextChunkId++;