From 4a58b63865c5c732012fe4746e2b54fdc165990e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 28 Jul 2025 12:05:56 -0400 Subject: [PATCH 01/10] [DevTools] Add "suspended by" Section to Component Inspector Sidebar (#34012) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This collects the ReactAsyncInfo between instances. It associates it with the parent. Typically this would be a Server Component's Promise return value but it can also be Promises in a fragment. It can also be associated with a client component when you pass a Promise into the child position e.g. `
{promise}
` then it's associated with the div. If an instance is filtered, then it gets associated with the parent of that's unfiltered. The stack trace currently isn't source mapped. I'll do that in a follow up. We also need to add a "short name" from the Promise for the description (e.g. url). I'll also add a little marker showing the relative time span of each entry. Screenshot 2025-07-26 at 7 56 00 PM Screenshot 2025-07-26 at 7 55 23 PM --- .../src/backend/fiber/renderer.js | 141 +++++++++++++++- .../src/backend/legacy/renderer.js | 7 + .../src/backend/types.js | 32 +++- .../react-devtools-shared/src/backendAPI.js | 36 +++++ .../InspectedElementSharedStyles.css | 38 +++++ .../Components/InspectedElementSourcePanel.js | 48 ++---- .../Components/InspectedElementSuspendedBy.js | 153 ++++++++++++++++++ .../views/Components/InspectedElementView.css | 45 ------ .../views/Components/InspectedElementView.js | 72 ++------- .../devtools/views/Components/OwnerView.css | 41 +++++ .../devtools/views/Components/OwnerView.js | 72 +++++++++ .../views/Components/StackTraceView.css | 24 +++ .../views/Components/StackTraceView.js | 58 +++++++ .../Components/formatLocationForDisplay.js | 44 +++++ .../src/devtools/views/utils.js | 2 +- .../src/frontend/types.js | 24 ++- 16 files changed, 678 insertions(+), 159 deletions(-) create mode 100644 packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspendedBy.js create mode 100644 packages/react-devtools-shared/src/devtools/views/Components/OwnerView.css create mode 100644 packages/react-devtools-shared/src/devtools/views/Components/OwnerView.js create mode 100644 packages/react-devtools-shared/src/devtools/views/Components/StackTraceView.css create mode 100644 packages/react-devtools-shared/src/devtools/views/Components/StackTraceView.js create mode 100644 packages/react-devtools-shared/src/devtools/views/Components/formatLocationForDisplay.js diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 96c7a38863b2e..31db8a7433448 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -7,7 +7,11 @@ * @flow */ -import type {ReactComponentInfo, ReactDebugInfo} from 'shared/ReactTypes'; +import type { + ReactComponentInfo, + ReactDebugInfo, + ReactAsyncInfo, +} from 'shared/ReactTypes'; import { ComponentFilterDisplayName, @@ -135,6 +139,7 @@ import type { ReactRenderer, RendererInterface, SerializedElement, + SerializedAsyncInfo, WorkTagMap, CurrentDispatcherRef, LegacyDispatcherRef, @@ -165,6 +170,7 @@ type FiberInstance = { source: null | string | Error | ReactFunctionLocation, // source location of this component function, or owned child stack logCount: number, // total number of errors/warnings last seen treeBaseDuration: number, // the profiled time of the last render of this subtree + suspendedBy: null | Array, // things that suspended in the children position of this component data: Fiber, // one of a Fiber pair }; @@ -178,6 +184,7 @@ function createFiberInstance(fiber: Fiber): FiberInstance { source: null, logCount: 0, treeBaseDuration: 0, + suspendedBy: null, data: fiber, }; } @@ -193,6 +200,7 @@ type FilteredFiberInstance = { source: null | string | Error | ReactFunctionLocation, // always null here. logCount: number, // total number of errors/warnings last seen treeBaseDuration: number, // the profiled time of the last render of this subtree + suspendedBy: null | Array, // not used data: Fiber, // one of a Fiber pair }; @@ -207,6 +215,7 @@ function createFilteredFiberInstance(fiber: Fiber): FilteredFiberInstance { source: null, logCount: 0, treeBaseDuration: 0, + suspendedBy: null, data: fiber, }: any); } @@ -225,6 +234,7 @@ type VirtualInstance = { source: null | string | Error | ReactFunctionLocation, // source location of this server component, or owned child stack logCount: number, // total number of errors/warnings last seen treeBaseDuration: number, // the profiled time of the last render of this subtree + suspendedBy: null | Array, // things that blocked the server component's child from rendering // The latest info for this instance. This can be updated over time and the // same info can appear in more than once ServerComponentInstance. data: ReactComponentInfo, @@ -242,6 +252,7 @@ function createVirtualInstance( source: null, logCount: 0, treeBaseDuration: 0, + suspendedBy: null, data: debugEntry, }; } @@ -2354,6 +2365,21 @@ export function attach( // the current parent here as well. let reconcilingParent: null | DevToolsInstance = null; + function insertSuspendedBy(asyncInfo: ReactAsyncInfo): void { + const parentInstance = reconcilingParent; + if (parentInstance === null) { + // Suspending at the root is not attributed to any particular component + // TODO: It should be attributed to the shell. + return; + } + const suspendedBy = parentInstance.suspendedBy; + if (suspendedBy === null) { + parentInstance.suspendedBy = [asyncInfo]; + } else if (suspendedBy.indexOf(asyncInfo) === -1) { + suspendedBy.push(asyncInfo); + } + } + function insertChild(instance: DevToolsInstance): void { const parentInstance = reconcilingParent; if (parentInstance === null) { @@ -2515,6 +2541,17 @@ export function attach( if (fiber._debugInfo) { for (let i = 0; i < fiber._debugInfo.length; i++) { const debugEntry = fiber._debugInfo[i]; + if (debugEntry.awaited) { + // Async Info + const asyncInfo: ReactAsyncInfo = (debugEntry: any); + if (level === virtualLevel) { + // Track any async info between the previous virtual instance up until to this + // instance and add it to the parent. This can add the same set multiple times + // so we assume insertSuspendedBy dedupes. + insertSuspendedBy(asyncInfo); + } + if (previousVirtualInstance) continue; + } if (typeof debugEntry.name !== 'string') { // Not a Component. Some other Debug Info. continue; @@ -2768,6 +2805,7 @@ export function attach( // Move all the children of this instance to the remaining set. remainingReconcilingChildren = instance.firstChild; instance.firstChild = null; + instance.suspendedBy = null; try { // Unmount the remaining set. unmountRemainingChildren(); @@ -2968,6 +3006,7 @@ export function attach( // We'll move them back one by one, and anything that remains is deleted. remainingReconcilingChildren = virtualInstance.firstChild; virtualInstance.firstChild = null; + virtualInstance.suspendedBy = null; try { if ( updateVirtualChildrenRecursively( @@ -3019,6 +3058,17 @@ export function attach( if (nextChild._debugInfo) { for (let i = 0; i < nextChild._debugInfo.length; i++) { const debugEntry = nextChild._debugInfo[i]; + if (debugEntry.awaited) { + // Async Info + const asyncInfo: ReactAsyncInfo = (debugEntry: any); + if (level === virtualLevel) { + // Track any async info between the previous virtual instance up until to this + // instance and add it to the parent. This can add the same set multiple times + // so we assume insertSuspendedBy dedupes. + insertSuspendedBy(asyncInfo); + } + if (previousVirtualInstance) continue; + } if (typeof debugEntry.name !== 'string') { // Not a Component. Some other Debug Info. continue; @@ -3343,6 +3393,7 @@ export function attach( // We'll move them back one by one, and anything that remains is deleted. remainingReconcilingChildren = fiberInstance.firstChild; fiberInstance.firstChild = null; + fiberInstance.suspendedBy = null; } try { if ( @@ -4051,6 +4102,42 @@ export function attach( return null; } + function serializeAsyncInfo( + asyncInfo: ReactAsyncInfo, + index: number, + parentInstance: DevToolsInstance, + ): SerializedAsyncInfo { + const ioInfo = asyncInfo.awaited; + const ioOwnerInstance = findNearestOwnerInstance( + parentInstance, + ioInfo.owner, + ); + const awaitOwnerInstance = findNearestOwnerInstance( + parentInstance, + asyncInfo.owner, + ); + return { + awaited: { + name: ioInfo.name, + start: ioInfo.start, + end: ioInfo.end, + value: ioInfo.value == null ? null : ioInfo.value, + env: ioInfo.env == null ? null : ioInfo.env, + owner: + ioOwnerInstance === null + ? null + : instanceToSerializedElement(ioOwnerInstance), + stack: ioInfo.stack == null ? null : ioInfo.stack, + }, + env: asyncInfo.env == null ? null : asyncInfo.env, + owner: + awaitOwnerInstance === null + ? null + : instanceToSerializedElement(awaitOwnerInstance), + stack: asyncInfo.stack == null ? null : asyncInfo.stack, + }; + } + // Fast path props lookup for React Native style editor. // Could use inspectElementRaw() but that would require shallow rendering hooks components, // and could also mess with memoization. @@ -4342,6 +4429,13 @@ export function attach( nativeTag = getNativeTag(fiber.stateNode); } + // This set is an edge case where if you pass a promise to a Client Component into a children + // position without a Server Component as the direct parent. E.g.
{promise}
+ // In this case, this becomes associated with the Client/Host Component where as normally + // you'd expect these to be associated with the Server Component that awaited the data. + // TODO: Prepend other suspense sources like css, images and use(). + const suspendedBy = fiberInstance.suspendedBy; + return { id: fiberInstance.id, @@ -4398,6 +4492,13 @@ export function attach( ? [] : Array.from(componentLogsEntry.warnings.entries()), + suspendedBy: + suspendedBy === null + ? [] + : suspendedBy.map((info, index) => + serializeAsyncInfo(info, index, fiberInstance), + ), + // List of owners owners, @@ -4451,6 +4552,9 @@ export function attach( const componentLogsEntry = componentInfoToComponentLogsMap.get(componentInfo); + // Things that Suspended this Server Component (use(), awaits and direct child promises) + const suspendedBy = virtualInstance.suspendedBy; + return { id: virtualInstance.id, @@ -4490,6 +4594,14 @@ export function attach( componentLogsEntry === undefined ? [] : Array.from(componentLogsEntry.warnings.entries()), + + suspendedBy: + suspendedBy === null + ? [] + : suspendedBy.map((info, index) => + serializeAsyncInfo(info, index, virtualInstance), + ), + // List of owners owners, @@ -4534,7 +4646,7 @@ export function attach( function createIsPathAllowed( key: string | null, - secondaryCategory: 'hooks' | null, + secondaryCategory: 'suspendedBy' | 'hooks' | null, ) { // This function helps prevent previously-inspected paths from being dehydrated in updates. // This is important to avoid a bad user experience where expanded toggles collapse on update. @@ -4566,6 +4678,13 @@ export function attach( return true; } break; + case 'suspendedBy': + if (path.length < 5) { + // Never dehydrate anything above suspendedBy[index].awaited.value + // Those are part of the internal meta data. We only dehydrate inside the Promise. + return true; + } + break; default: break; } @@ -4789,36 +4908,42 @@ export function attach( type: 'not-found', }; } + const inspectedElement = mostRecentlyInspectedElement; // Any time an inspected element has an update, // we should update the selected $r value as wel. // Do this before dehydration (cleanForBridge). - updateSelectedElement(mostRecentlyInspectedElement); + updateSelectedElement(inspectedElement); // Clone before cleaning so that we preserve the full data. // This will enable us to send patches without re-inspecting if hydrated paths are requested. // (Reducing how often we shallow-render is a better DX for function components that use hooks.) - const cleanedInspectedElement = {...mostRecentlyInspectedElement}; + const cleanedInspectedElement = {...inspectedElement}; // $FlowFixMe[prop-missing] found when upgrading Flow cleanedInspectedElement.context = cleanForBridge( - cleanedInspectedElement.context, + inspectedElement.context, createIsPathAllowed('context', null), ); // $FlowFixMe[prop-missing] found when upgrading Flow cleanedInspectedElement.hooks = cleanForBridge( - cleanedInspectedElement.hooks, + inspectedElement.hooks, createIsPathAllowed('hooks', 'hooks'), ); // $FlowFixMe[prop-missing] found when upgrading Flow cleanedInspectedElement.props = cleanForBridge( - cleanedInspectedElement.props, + inspectedElement.props, createIsPathAllowed('props', null), ); // $FlowFixMe[prop-missing] found when upgrading Flow cleanedInspectedElement.state = cleanForBridge( - cleanedInspectedElement.state, + inspectedElement.state, createIsPathAllowed('state', null), ); + // $FlowFixMe[prop-missing] found when upgrading Flow + cleanedInspectedElement.suspendedBy = cleanForBridge( + inspectedElement.suspendedBy, + createIsPathAllowed('suspendedBy', 'suspendedBy'), + ); return { id, diff --git a/packages/react-devtools-shared/src/backend/legacy/renderer.js b/packages/react-devtools-shared/src/backend/legacy/renderer.js index cc097c8379090..d2b846bee24be 100644 --- a/packages/react-devtools-shared/src/backend/legacy/renderer.js +++ b/packages/react-devtools-shared/src/backend/legacy/renderer.js @@ -755,6 +755,10 @@ export function attach( inspectedElement.state, createIsPathAllowed('state'), ); + inspectedElement.suspendedBy = cleanForBridge( + inspectedElement.suspendedBy, + createIsPathAllowed('suspendedBy'), + ); return { id, @@ -847,6 +851,9 @@ export function attach( errors, warnings, + // Not supported in legacy renderers. + suspendedBy: [], + // List of owners owners, diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js index c9d6284b2f424..979baa3e0ae7b 100644 --- a/packages/react-devtools-shared/src/backend/types.js +++ b/packages/react-devtools-shared/src/backend/types.js @@ -32,7 +32,7 @@ import type { import type {InitBackend} from 'react-devtools-shared/src/backend'; import type {TimelineDataExport} from 'react-devtools-timeline/src/types'; import type {BackendBridge} from 'react-devtools-shared/src/bridge'; -import type {ReactFunctionLocation} from 'shared/ReactTypes'; +import type {ReactFunctionLocation, ReactStackTrace} from 'shared/ReactTypes'; import type Agent from './agent'; type BundleType = @@ -232,6 +232,25 @@ export type PathMatch = { isFullMatch: boolean, }; +// Serialized version of ReactIOInfo +export type SerializedIOInfo = { + name: string, + start: number, + end: number, + value: null | Promise, + env: null | string, + owner: null | SerializedElement, + stack: null | ReactStackTrace, +}; + +// Serialized version of ReactAsyncInfo +export type SerializedAsyncInfo = { + awaited: SerializedIOInfo, + env: null | string, + owner: null | SerializedElement, + stack: null | ReactStackTrace, +}; + export type SerializedElement = { displayName: string | null, id: number, @@ -268,14 +287,17 @@ export type InspectedElement = { hasLegacyContext: boolean, // Inspectable properties. - context: Object | null, - hooks: Object | null, - props: Object | null, - state: Object | null, + context: Object | null, // DehydratedData or {[string]: mixed} + hooks: Object | null, // DehydratedData or {[string]: mixed} + props: Object | null, // DehydratedData or {[string]: mixed} + state: Object | null, // DehydratedData or {[string]: mixed} key: number | string | null, errors: Array<[string, number]>, warnings: Array<[string, number]>, + // Things that suspended this Instances + suspendedBy: Object, // DehydratedData or Array + // List of owners owners: Array | null, source: ReactFunctionLocation | null, diff --git a/packages/react-devtools-shared/src/backendAPI.js b/packages/react-devtools-shared/src/backendAPI.js index 20b4e99a101e7..9f96215026a7d 100644 --- a/packages/react-devtools-shared/src/backendAPI.js +++ b/packages/react-devtools-shared/src/backendAPI.js @@ -16,6 +16,7 @@ import ElementPollingCancellationError from 'react-devtools-shared/src/errors/El import type { InspectedElement as InspectedElementBackend, InspectedElementPayload, + SerializedAsyncInfo as SerializedAsyncInfoBackend, } from 'react-devtools-shared/src/backend/types'; import type { BackendEvents, @@ -24,6 +25,7 @@ import type { import type { DehydratedData, InspectedElement as InspectedElementFrontend, + SerializedAsyncInfo as SerializedAsyncInfoFrontend, } from 'react-devtools-shared/src/frontend/types'; import type {InspectedElementPath} from 'react-devtools-shared/src/frontend/types'; @@ -209,6 +211,32 @@ export function cloneInspectedElementWithPath( return clonedInspectedElement; } +function backendToFrontendSerializedAsyncInfo( + asyncInfo: SerializedAsyncInfoBackend, +): SerializedAsyncInfoFrontend { + const ioInfo = asyncInfo.awaited; + return { + awaited: { + name: ioInfo.name, + start: ioInfo.start, + end: ioInfo.end, + value: ioInfo.value, + env: ioInfo.env, + owner: + ioInfo.owner === null + ? null + : backendToFrontendSerializedElementMapper(ioInfo.owner), + stack: ioInfo.stack, + }, + env: asyncInfo.env, + owner: + asyncInfo.owner === null + ? null + : backendToFrontendSerializedElementMapper(asyncInfo.owner), + stack: asyncInfo.stack, + }; +} + export function convertInspectedElementBackendToFrontend( inspectedElementBackend: InspectedElementBackend, ): InspectedElementFrontend { @@ -238,9 +266,13 @@ export function convertInspectedElementBackendToFrontend( key, errors, warnings, + suspendedBy, nativeTag, } = inspectedElementBackend; + const hydratedSuspendedBy: null | Array = + hydrateHelper(suspendedBy); + const inspectedElement: InspectedElementFrontend = { canEditFunctionProps, canEditFunctionPropsDeletePaths, @@ -272,6 +304,10 @@ export function convertInspectedElementBackendToFrontend( state: hydrateHelper(state), errors, warnings, + suspendedBy: + hydratedSuspendedBy == null // backwards compat + ? [] + : hydratedSuspendedBy.map(backendToFrontendSerializedAsyncInfo), nativeTag, }; diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSharedStyles.css b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSharedStyles.css index e9916d467cfa8..65dd6baf4a6fc 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSharedStyles.css +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSharedStyles.css @@ -51,3 +51,41 @@ .EditableValue { min-width: 1rem; } + +.CollapsableRow { + border-top: 1px solid var(--color-border); +} + +.CollapsableRow:last-child { + margin-bottom: -0.25rem; +} + +.CollapsableHeader { + width: 100%; + padding: 0.25rem; + display: flex; +} + +.CollapsableHeaderIcon { + flex: 0 0 1rem; + margin-left: -0.25rem; + width: 1rem; + height: 1rem; + padding: 0; + color: var(--color-expand-collapse-toggle); +} + +.CollapsableHeaderTitle { + flex: 1 1 auto; + font-family: var(--font-family-monospace); + font-size: var(--font-size-monospace-normal); + text-align: left; +} + +.CollapsableContent { + padding: 0.25rem 0; +} + +.PreviewContainer { + padding: 0 0.25rem 0.25rem 0.25rem; +} diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSourcePanel.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSourcePanel.js index 585361cedcf43..604c3784b75c7 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSourcePanel.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSourcePanel.js @@ -9,7 +9,6 @@ import * as React from 'react'; import {copy} from 'clipboard-js'; -import {toNormalUrl} from 'jsc-safe-url'; import Button from '../Button'; import ButtonIcon from '../ButtonIcon'; @@ -21,6 +20,8 @@ import useOpenResource from '../useOpenResource'; import type {ReactFunctionLocation} from 'shared/ReactTypes'; import styles from './InspectedElementSourcePanel.css'; +import formatLocationForDisplay from './formatLocationForDisplay'; + type Props = { source: ReactFunctionLocation, symbolicatedSourcePromise: Promise, @@ -95,52 +96,21 @@ function FormattedSourceString({source, symbolicatedSourcePromise}: Props) { symbolicatedSource, ); - const [, sourceURL, line] = + const [, sourceURL, line, column] = symbolicatedSource == null ? source : symbolicatedSource; return (
- {linkIsEnabled ? ( - - {formatSourceForDisplay(sourceURL, line)} - - ) : ( - formatSourceForDisplay(sourceURL, line) - )} + + {formatLocationForDisplay(sourceURL, line, column)} +
); } -// This function is based on describeComponentFrame() in packages/shared/ReactComponentStackFrame -function formatSourceForDisplay(sourceURL: string, line: number) { - // Metro can return JSC-safe URLs, which have `//&` as a delimiter - // https://www.npmjs.com/package/jsc-safe-url - const sanitizedSourceURL = sourceURL.includes('//&') - ? toNormalUrl(sourceURL) - : sourceURL; - - // Note: this RegExp doesn't work well with URLs from Metro, - // which provides bundle URL with query parameters prefixed with /& - const BEFORE_SLASH_RE = /^(.*)[\\\/]/; - - let nameOnly = sanitizedSourceURL.replace(BEFORE_SLASH_RE, ''); - - // In DEV, include code for a common special case: - // prefer "folder/index.js" instead of just "index.js". - if (/^index\./.test(nameOnly)) { - const match = sanitizedSourceURL.match(BEFORE_SLASH_RE); - if (match) { - const pathBeforeSlash = match[1]; - if (pathBeforeSlash) { - const folderName = pathBeforeSlash.replace(BEFORE_SLASH_RE, ''); - nameOnly = folderName + '/' + nameOnly; - } - } - } - - return `${nameOnly}:${line}`; -} - export default InspectedElementSourcePanel; diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspendedBy.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspendedBy.js new file mode 100644 index 0000000000000..a0bc761c506b0 --- /dev/null +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspendedBy.js @@ -0,0 +1,153 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import {copy} from 'clipboard-js'; +import * as React from 'react'; +import {useState} from 'react'; +import Button from '../Button'; +import ButtonIcon from '../ButtonIcon'; +import KeyValue from './KeyValue'; +import {serializeDataForCopy} from '../utils'; +import Store from '../../store'; +import styles from './InspectedElementSharedStyles.css'; +import {withPermissionsCheck} from 'react-devtools-shared/src/frontend/utils/withPermissionsCheck'; +import StackTraceView from './StackTraceView'; +import OwnerView from './OwnerView'; + +import type {InspectedElement} from 'react-devtools-shared/src/frontend/types'; +import type {FrontendBridge} from 'react-devtools-shared/src/bridge'; +import type {SerializedAsyncInfo} from 'react-devtools-shared/src/frontend/types'; + +type RowProps = { + bridge: FrontendBridge, + element: Element, + inspectedElement: InspectedElement, + store: Store, + asyncInfo: SerializedAsyncInfo, + index: number, +}; + +function SuspendedByRow({ + bridge, + element, + inspectedElement, + store, + asyncInfo, + index, +}: RowProps) { + const [isOpen, setIsOpen] = useState(false); + const name = asyncInfo.awaited.name; + let stack; + let owner; + if (asyncInfo.stack === null || asyncInfo.stack.length === 0) { + stack = asyncInfo.awaited.stack; + owner = asyncInfo.awaited.owner; + } else { + stack = asyncInfo.stack; + owner = asyncInfo.owner; + } + return ( +
+ + {isOpen && ( +
+
+
+ {stack !== null && stack.length > 0 && ( + + )} + {owner !== null && owner.id !== inspectedElement.id ? ( + + ) : null} +
+ )} +
+ ); +} + +type Props = { + bridge: FrontendBridge, + element: Element, + inspectedElement: InspectedElement, + store: Store, +}; + +export default function InspectedElementSuspendedBy({ + bridge, + element, + inspectedElement, + store, +}: Props): React.Node { + const {suspendedBy} = inspectedElement; + + // Skip the section if nothing suspended this component. + if (suspendedBy == null || suspendedBy.length === 0) { + return null; + } + + const handleCopy = withPermissionsCheck( + {permissions: ['clipboardWrite']}, + () => copy(serializeDataForCopy(suspendedBy)), + ); + + return ( +
+
+
suspended by
+ +
+ {suspendedBy.map((asyncInfo, index) => ( + + ))} +
+ ); +} diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementView.css b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementView.css index 57dc15d8b3d9f..3450a745b9e45 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementView.css +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementView.css @@ -2,16 +2,6 @@ font-family: var(--font-family-sans); } -.Owner { - color: var(--color-component-name); - font-family: var(--font-family-monospace); - font-size: var(--font-size-monospace-normal); - white-space: nowrap; - overflow: hidden; - text-overflow: ellipsis; - max-width: 100%; -} - .InspectedElement { overflow-x: hidden; overflow-y: auto; @@ -28,41 +18,6 @@ } } -.Owner { - border-radius: 0.25rem; - padding: 0.125rem 0.25rem; - background: none; - border: none; - display: block; -} -.Owner:focus { - outline: none; - background-color: var(--color-button-background-focus); -} - -.NotInStore { - color: var(--color-dim); - cursor: default; -} - -.OwnerButton { - cursor: pointer; - width: 100%; - padding: 0; -} - -.OwnerContent { - display: flex; - align-items: center; - padding-left: 1rem; - width: 100%; - border-radius: 0.25rem; -} - -.OwnerContent:hover { - background-color: var(--color-background-hover); -} - .OwnersMetaField { padding-left: 1.25rem; white-space: nowrap; diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementView.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementView.js index 037ca36b89fdc..8bf373f685cae 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementView.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementView.js @@ -8,10 +8,8 @@ */ import * as React from 'react'; -import {Fragment, useCallback, useContext} from 'react'; -import {TreeDispatcherContext} from './TreeContext'; +import {Fragment, useContext} from 'react'; import {BridgeContext, StoreContext} from '../context'; -import Button from '../Button'; import InspectedElementBadges from './InspectedElementBadges'; import InspectedElementContextTree from './InspectedElementContextTree'; import InspectedElementErrorsAndWarningsTree from './InspectedElementErrorsAndWarningsTree'; @@ -20,12 +18,11 @@ import InspectedElementPropsTree from './InspectedElementPropsTree'; import InspectedElementStateTree from './InspectedElementStateTree'; import InspectedElementStyleXPlugin from './InspectedElementStyleXPlugin'; import InspectedElementSuspenseToggle from './InspectedElementSuspenseToggle'; +import InspectedElementSuspendedBy from './InspectedElementSuspendedBy'; import NativeStyleEditor from './NativeStyleEditor'; -import ElementBadges from './ElementBadges'; -import {useHighlightHostInstance} from '../hooks'; import {enableStyleXFeatures} from 'react-devtools-feature-flags'; -import {logEvent} from 'react-devtools-shared/src/Logger'; import InspectedElementSourcePanel from './InspectedElementSourcePanel'; +import OwnerView from './OwnerView'; import styles from './InspectedElementView.css'; @@ -156,6 +153,15 @@ export default function InspectedElementView({ +
+ +
+ {showRenderedBy && (
); } - -type OwnerViewProps = { - displayName: string, - hocDisplayNames: Array | null, - compiledWithForget: boolean, - id: number, - isInStore: boolean, -}; - -function OwnerView({ - displayName, - hocDisplayNames, - compiledWithForget, - id, - isInStore, -}: OwnerViewProps) { - const dispatch = useContext(TreeDispatcherContext); - const {highlightHostInstance, clearHighlightHostInstance} = - useHighlightHostInstance(); - - const handleClick = useCallback(() => { - logEvent({ - event_name: 'select-element', - metadata: {source: 'owner-view'}, - }); - dispatch({ - type: 'SELECT_ELEMENT_BY_ID', - payload: id, - }); - }, [dispatch, id]); - - return ( - - ); -} diff --git a/packages/react-devtools-shared/src/devtools/views/Components/OwnerView.css b/packages/react-devtools-shared/src/devtools/views/Components/OwnerView.css new file mode 100644 index 0000000000000..b4e5cd157f3c4 --- /dev/null +++ b/packages/react-devtools-shared/src/devtools/views/Components/OwnerView.css @@ -0,0 +1,41 @@ +.Owner { + color: var(--color-component-name); + font-family: var(--font-family-monospace); + font-size: var(--font-size-monospace-normal); + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; + max-width: 100%; + border-radius: 0.25rem; + padding: 0.125rem 0.25rem; + background: none; + border: none; + display: block; +} +.Owner:focus { + outline: none; + background-color: var(--color-button-background-focus); +} + +.OwnerButton { + cursor: pointer; + width: 100%; + padding: 0; +} + +.OwnerContent { + display: flex; + align-items: center; + padding-left: 1rem; + width: 100%; + border-radius: 0.25rem; +} + +.OwnerContent:hover { + background-color: var(--color-background-hover); +} + +.NotInStore { + color: var(--color-dim); + cursor: default; +} diff --git a/packages/react-devtools-shared/src/devtools/views/Components/OwnerView.js b/packages/react-devtools-shared/src/devtools/views/Components/OwnerView.js new file mode 100644 index 0000000000000..561e8a6651362 --- /dev/null +++ b/packages/react-devtools-shared/src/devtools/views/Components/OwnerView.js @@ -0,0 +1,72 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import * as React from 'react'; +import {useCallback, useContext} from 'react'; +import {TreeDispatcherContext} from './TreeContext'; +import Button from '../Button'; +import ElementBadges from './ElementBadges'; +import {useHighlightHostInstance} from '../hooks'; +import {logEvent} from 'react-devtools-shared/src/Logger'; + +import styles from './OwnerView.css'; + +type OwnerViewProps = { + displayName: string, + hocDisplayNames: Array | null, + compiledWithForget: boolean, + id: number, + isInStore: boolean, +}; + +export default function OwnerView({ + displayName, + hocDisplayNames, + compiledWithForget, + id, + isInStore, +}: OwnerViewProps): React.Node { + const dispatch = useContext(TreeDispatcherContext); + const {highlightHostInstance, clearHighlightHostInstance} = + useHighlightHostInstance(); + + const handleClick = useCallback(() => { + logEvent({ + event_name: 'select-element', + metadata: {source: 'owner-view'}, + }); + dispatch({ + type: 'SELECT_ELEMENT_BY_ID', + payload: id, + }); + }, [dispatch, id]); + + return ( + + ); +} diff --git a/packages/react-devtools-shared/src/devtools/views/Components/StackTraceView.css b/packages/react-devtools-shared/src/devtools/views/Components/StackTraceView.css new file mode 100644 index 0000000000000..2dd1410c8c7ad --- /dev/null +++ b/packages/react-devtools-shared/src/devtools/views/Components/StackTraceView.css @@ -0,0 +1,24 @@ +.StackTraceView { + padding: 0.25rem; +} + +.CallSite { + display: block; + padding-left: 1rem; +} + +.Link { + color: var(--color-link); + white-space: pre; + overflow: hidden; + text-overflow: ellipsis; + flex: 1; + cursor: pointer; + border-radius: 0.125rem; + padding: 0px 2px; +} + +.Link:hover { + background-color: var(--color-background-hover); +} + diff --git a/packages/react-devtools-shared/src/devtools/views/Components/StackTraceView.js b/packages/react-devtools-shared/src/devtools/views/Components/StackTraceView.js new file mode 100644 index 0000000000000..62cf911b9fe73 --- /dev/null +++ b/packages/react-devtools-shared/src/devtools/views/Components/StackTraceView.js @@ -0,0 +1,58 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import * as React from 'react'; + +import useOpenResource from '../useOpenResource'; + +import styles from './StackTraceView.css'; + +import type {ReactStackTrace, ReactCallSite} from 'shared/ReactTypes'; + +import formatLocationForDisplay from './formatLocationForDisplay'; + +type CallSiteViewProps = { + callSite: ReactCallSite, +}; + +export function CallSiteView({callSite}: CallSiteViewProps): React.Node { + const symbolicatedCallSite: null | ReactCallSite = null; // TODO + const [linkIsEnabled, viewSource] = useOpenResource( + callSite, + symbolicatedCallSite, + ); + const [functionName, url, line, column] = + symbolicatedCallSite !== null ? symbolicatedCallSite : callSite; + return ( +
+ {functionName} + {' @ '} + + {formatLocationForDisplay(url, line, column)} + +
+ ); +} + +type Props = { + stack: ReactStackTrace, +}; + +export default function StackTraceView({stack}: Props): React.Node { + return ( +
+ {stack.map((callSite, index) => ( + + ))} +
+ ); +} diff --git a/packages/react-devtools-shared/src/devtools/views/Components/formatLocationForDisplay.js b/packages/react-devtools-shared/src/devtools/views/Components/formatLocationForDisplay.js new file mode 100644 index 0000000000000..1c113e3883927 --- /dev/null +++ b/packages/react-devtools-shared/src/devtools/views/Components/formatLocationForDisplay.js @@ -0,0 +1,44 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import {toNormalUrl} from 'jsc-safe-url'; + +// This function is based on describeComponentFrame() in packages/shared/ReactComponentStackFrame +export default function formatLocationForDisplay( + sourceURL: string, + line: number, + column: number, +): string { + // Metro can return JSC-safe URLs, which have `//&` as a delimiter + // https://www.npmjs.com/package/jsc-safe-url + const sanitizedSourceURL = sourceURL.includes('//&') + ? toNormalUrl(sourceURL) + : sourceURL; + + // Note: this RegExp doesn't work well with URLs from Metro, + // which provides bundle URL with query parameters prefixed with /& + const BEFORE_SLASH_RE = /^(.*)[\\\/]/; + + let nameOnly = sanitizedSourceURL.replace(BEFORE_SLASH_RE, ''); + + // In DEV, include code for a common special case: + // prefer "folder/index.js" instead of just "index.js". + if (/^index\./.test(nameOnly)) { + const match = sanitizedSourceURL.match(BEFORE_SLASH_RE); + if (match) { + const pathBeforeSlash = match[1]; + if (pathBeforeSlash) { + const folderName = pathBeforeSlash.replace(BEFORE_SLASH_RE, ''); + nameOnly = folderName + '/' + nameOnly; + } + } + } + + return `${nameOnly}:${line}`; +} diff --git a/packages/react-devtools-shared/src/devtools/views/utils.js b/packages/react-devtools-shared/src/devtools/views/utils.js index 2759a3f452c86..ed14b2c236bd5 100644 --- a/packages/react-devtools-shared/src/devtools/views/utils.js +++ b/packages/react-devtools-shared/src/devtools/views/utils.js @@ -121,7 +121,7 @@ function sanitize(data: Object): void { } export function serializeDataForCopy(props: Object): string { - const cloned = Object.assign({}, props); + const cloned = isArray(props) ? props.slice(0) : Object.assign({}, props); sanitize(cloned); diff --git a/packages/react-devtools-shared/src/frontend/types.js b/packages/react-devtools-shared/src/frontend/types.js index 3f687a03da6c4..aa6b95a88ab3e 100644 --- a/packages/react-devtools-shared/src/frontend/types.js +++ b/packages/react-devtools-shared/src/frontend/types.js @@ -18,7 +18,7 @@ import type { Dehydrated, Unserializable, } from 'react-devtools-shared/src/hydration'; -import type {ReactFunctionLocation} from 'shared/ReactTypes'; +import type {ReactFunctionLocation, ReactStackTrace} from 'shared/ReactTypes'; export type BrowserTheme = 'dark' | 'light'; @@ -184,6 +184,25 @@ export type Element = { compiledWithForget: boolean, }; +// Serialized version of ReactIOInfo +export type SerializedIOInfo = { + name: string, + start: number, + end: number, + value: null | Promise, + env: null | string, + owner: null | SerializedElement, + stack: null | ReactStackTrace, +}; + +// Serialized version of ReactAsyncInfo +export type SerializedAsyncInfo = { + awaited: SerializedIOInfo, + env: null | string, + owner: null | SerializedElement, + stack: null | ReactStackTrace, +}; + export type SerializedElement = { displayName: string | null, id: number, @@ -239,6 +258,9 @@ export type InspectedElement = { errors: Array<[string, number]>, warnings: Array<[string, number]>, + // Things that suspended this Instances + suspendedBy: Object, + // List of owners owners: Array | null, From eaee5308cc68232c4380e62dc73f512b2c50ab96 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Mon, 28 Jul 2025 12:09:56 -0400 Subject: [PATCH 02/10] Add changelog entry for 19.1.1 (#34021) Add changelog details matching release notes: https://github.com/facebook/react/releases/tag/v19.1.1 --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a10cda01536b6..edc80475d3db1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +## 19.1.1 (July 28, 2025) + +### React +* Fixed Owner Stacks to work with ES2015 function.name semantics (#33680 by @hoxyq) +* Move the Fabric completeRoot call from `finalizeContainerChildren` to `replaceContainerChildren` to align how JS API and Fabric interpret `completeRoot` (#30513, #33064 by @kassens and @jackpope) +* Fix React retaining shadow nodes longer that it needs to (#33161, #33447 by @sammy-SC and @yungsters) + ## 19.1.0 (March 28, 2025) ### Owner Stack From 101b20b663cc866ad4c0102413bbf7cb111451b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 28 Jul 2025 12:22:33 -0400 Subject: [PATCH 03/10] [DevTools] Add a little bar indicating time span of an async entry relative to others (#34016) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stacked on #34012. This shows a time track for when some I/O started and when it finished relative to other I/O in the same component (or later in the same suspense boundary). This is not meant to be a precise visualization since the data might be misleading if you're running this in dev which has other perf characteristics anyway. It's just meant to be a general way to orient yourself in the data. We can also highlight rejected promises here. The color scheme is the same as Chrome's current Performance Track colors to add continuity but those could change. Screenshot 2025-07-27 at 11 48 03 PM --- .../src/devtools/constants.js | 6 ++ .../InspectedElementSharedStyles.css | 20 ++++++ .../Components/InspectedElementSuspendedBy.js | 63 ++++++++++++++++++- 3 files changed, 86 insertions(+), 3 deletions(-) diff --git a/packages/react-devtools-shared/src/devtools/constants.js b/packages/react-devtools-shared/src/devtools/constants.js index cfa3d5af1648b..ee13e5b1630a2 100644 --- a/packages/react-devtools-shared/src/devtools/constants.js +++ b/packages/react-devtools-shared/src/devtools/constants.js @@ -135,6 +135,9 @@ export const THEME_STYLES: {[style: Theme | DisplayDensity]: any, ...} = { '--color-timeline-text-color': '#000000', '--color-timeline-text-dim-color': '#ccc', '--color-timeline-react-work-border': '#eeeeee', + '--color-timebar-background': '#f6f6f6', + '--color-timespan-background': '#62bc6a', + '--color-timespan-background-errored': '#d57066', '--color-search-match': 'yellow', '--color-search-match-current': '#f7923b', '--color-selected-tree-highlight-active': 'rgba(0, 136, 250, 0.1)', @@ -283,6 +286,9 @@ export const THEME_STYLES: {[style: Theme | DisplayDensity]: any, ...} = { '--color-timeline-text-color': '#282c34', '--color-timeline-text-dim-color': '#555b66', '--color-timeline-react-work-border': '#3d424a', + '--color-timebar-background': '#1d2129', + '--color-timespan-background': '#62bc6a', + '--color-timespan-background-errored': '#d57066', '--color-search-match': 'yellow', '--color-search-match-current': '#f7923b', '--color-selected-tree-highlight-active': 'rgba(23, 143, 185, 0.15)', diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSharedStyles.css b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSharedStyles.css index 65dd6baf4a6fc..c0d3c95bec1a8 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSharedStyles.css +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSharedStyles.css @@ -89,3 +89,23 @@ .PreviewContainer { padding: 0 0.25rem 0.25rem 0.25rem; } + +.TimeBarContainer { + position: relative; + flex: 0 0 20%; + height: 0.25rem; + border-radius: 0.125rem; + background-color: var(--color-timebar-background); +} + +.TimeBarSpan, .TimeBarSpanErrored { + position: absolute; + border-radius: 0.125rem; + background-color: var(--color-timespan-background); + width: 100%; + height: 100%; +} + +.TimeBarSpanErrored { + background-color: var(--color-timespan-background-errored); +} \ No newline at end of file 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 a0bc761c506b0..93f4078a0ead3 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspendedBy.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspendedBy.js @@ -19,10 +19,13 @@ import styles from './InspectedElementSharedStyles.css'; import {withPermissionsCheck} from 'react-devtools-shared/src/frontend/utils/withPermissionsCheck'; import StackTraceView from './StackTraceView'; import OwnerView from './OwnerView'; +import {meta} from '../../../hydration'; -import type {InspectedElement} from 'react-devtools-shared/src/frontend/types'; +import type { + InspectedElement, + SerializedAsyncInfo, +} from 'react-devtools-shared/src/frontend/types'; import type {FrontendBridge} from 'react-devtools-shared/src/bridge'; -import type {SerializedAsyncInfo} from 'react-devtools-shared/src/frontend/types'; type RowProps = { bridge: FrontendBridge, @@ -31,6 +34,8 @@ type RowProps = { store: Store, asyncInfo: SerializedAsyncInfo, index: number, + minTime: number, + maxTime: number, }; function SuspendedByRow({ @@ -40,6 +45,8 @@ function SuspendedByRow({ store, asyncInfo, index, + minTime, + maxTime, }: RowProps) { const [isOpen, setIsOpen] = useState(false); const name = asyncInfo.awaited.name; @@ -52,17 +59,47 @@ function SuspendedByRow({ stack = asyncInfo.stack; owner = asyncInfo.owner; } + const start = asyncInfo.awaited.start; + const end = asyncInfo.awaited.end; + const timeScale = 100 / (maxTime - minTime); + let left = (start - minTime) * timeScale; + let width = (end - start) * timeScale; + if (width < 5) { + // Use at least a 5% width to avoid showing too small indicators. + width = 5; + if (left > 95) { + left = 95; + } + } + + const value: any = asyncInfo.awaited.value; + const isErrored = + value !== null && + typeof value === 'object' && + value[meta.name] === 'rejected Thenable'; + return (
{isOpen && (
@@ -129,6 +166,24 @@ export default function InspectedElementSuspendedBy({ () => copy(serializeDataForCopy(suspendedBy)), ); + let minTime = Infinity; + let maxTime = -Infinity; + for (let i = 0; i < suspendedBy.length; i++) { + const asyncInfo: SerializedAsyncInfo = suspendedBy[i]; + if (asyncInfo.awaited.start < minTime) { + minTime = asyncInfo.awaited.start; + } + if (asyncInfo.awaited.end > maxTime) { + maxTime = asyncInfo.awaited.end; + } + } + + if (maxTime - minTime < 25) { + // Stretch the time span a bit to ensure that we don't show + // large bars that represent very small timespans. + minTime = maxTime - 25; + } + return (
@@ -146,6 +201,8 @@ export default function InspectedElementSuspendedBy({ element={element} inspectedElement={inspectedElement} store={store} + minTime={minTime} + maxTime={maxTime} /> ))}
From ab2681af0305aa1f7451bd15524d4d783dd78f69 Mon Sep 17 00:00:00 2001 From: "Sebastian \"Sebbie\" Silbermann" Date: Mon, 28 Jul 2025 18:26:55 +0200 Subject: [PATCH 04/10] [DevTools] Skeleton for Suspense tab (#34020) --- .../src/main/index.js | 66 ++++++++++++++++++- packages/react-devtools-shared/src/Logger.js | 3 + .../src/backend/agent.js | 10 +++ packages/react-devtools-shared/src/bridge.js | 1 + .../src/devtools/store.js | 13 ++++ .../src/devtools/views/DevTools.js | 48 +++++++++++++- .../src/devtools/views/Icon.js | 13 +++- .../views/Settings/SettingsContext.js | 14 +++- .../devtools/views/SuspenseTab/SuspenseTab.js | 8 +++ 9 files changed, 169 insertions(+), 7 deletions(-) create mode 100644 packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTab.js diff --git a/packages/react-devtools-extensions/src/main/index.js b/packages/react-devtools-extensions/src/main/index.js index f21bdc7a9a9d8..362793e3eb430 100644 --- a/packages/react-devtools-extensions/src/main/index.js +++ b/packages/react-devtools-extensions/src/main/index.js @@ -151,6 +151,10 @@ function createBridgeAndStore() { supportsClickToInspect: true, }); + store.addListener('enableSuspenseTab', () => { + createSuspensePanel(); + }); + store.addListener('settingsUpdated', settings => { chrome.storage.local.set(settings); }); @@ -209,6 +213,7 @@ function createBridgeAndStore() { overrideTab, showTabBar: false, store, + suspensePortalContainer, warnIfUnsupportedVersionDetected: true, viewAttributeSourceFunction, // Firefox doesn't support chrome.devtools.panels.openResource yet @@ -354,6 +359,42 @@ function createSourcesEditorPanel() { }); } +function createSuspensePanel() { + if (suspensePortalContainer) { + // Panel is created and user opened it at least once + ensureInitialHTMLIsCleared(suspensePortalContainer); + render('suspense'); + + return; + } + + if (suspensePanel) { + // Panel is created, but wasn't opened yet, so no document is present for it + return; + } + + chrome.devtools.panels.create( + __IS_CHROME__ || __IS_EDGE__ ? 'Suspense ⚛' : 'Suspense', + __IS_EDGE__ ? 'icons/production.svg' : '', + 'panel.html', + createdPanel => { + suspensePanel = createdPanel; + + createdPanel.onShown.addListener(portal => { + suspensePortalContainer = portal.container; + if (suspensePortalContainer != null && render) { + ensureInitialHTMLIsCleared(suspensePortalContainer); + + render('suspense'); + portal.injectStyles(cloneStyleTags); + + logEvent({event_name: 'selected-suspense-tab'}); + } + }); + }, + ); +} + function performInTabNavigationCleanup() { // Potentially, if react hasn't loaded yet and user performs in-tab navigation clearReactPollingInstance(); @@ -365,7 +406,12 @@ function performInTabNavigationCleanup() { // If panels were already created, and we have already mounted React root to display // tabs (Components or Profiler), we should unmount root first and render them again - if ((componentsPortalContainer || profilerPortalContainer) && root) { + if ( + (componentsPortalContainer || + profilerPortalContainer || + suspensePortalContainer) && + root + ) { // It's easiest to recreate the DevTools panel (to clean up potential stale state). // We can revisit this in the future as a small optimization. // This should also emit bridge.shutdown, but only if this root was mounted @@ -395,7 +441,12 @@ function performFullCleanup() { // Potentially, if react hasn't loaded yet and user closed the browser DevTools clearReactPollingInstance(); - if ((componentsPortalContainer || profilerPortalContainer) && root) { + if ( + (componentsPortalContainer || + profilerPortalContainer || + suspensePortalContainer) && + root + ) { // This should also emit bridge.shutdown, but only if this root was mounted flushSync(() => root.unmount()); } else { @@ -404,6 +455,7 @@ function performFullCleanup() { componentsPortalContainer = null; profilerPortalContainer = null; + suspensePortalContainer = null; root = null; mostRecentOverrideTab = null; @@ -454,6 +506,8 @@ function mountReactDevTools() { createComponentsPanel(); createProfilerPanel(); createSourcesEditorPanel(); + // Suspense Tab is created via the hook + // TODO(enableSuspenseTab): Create eagerly once Suspense tab is stable } let reactPollingInstance = null; @@ -474,6 +528,12 @@ function showNoReactDisclaimer() { '

Looks like this page doesn\'t have React, or it hasn\'t been loaded yet.

'; delete profilerPortalContainer._hasInitialHTMLBeenCleared; } + + if (suspensePortalContainer) { + suspensePortalContainer.innerHTML = + '

Looks like this page doesn\'t have React, or it hasn\'t been loaded yet.

'; + delete suspensePortalContainer._hasInitialHTMLBeenCleared; + } } function mountReactDevToolsWhenReactHasLoaded() { @@ -492,9 +552,11 @@ let profilingData = null; let componentsPanel = null; let profilerPanel = null; +let suspensePanel = null; let editorPane = null; let componentsPortalContainer = null; let profilerPortalContainer = null; +let suspensePortalContainer = null; let editorPortalContainer = null; let mostRecentOverrideTab = null; diff --git a/packages/react-devtools-shared/src/Logger.js b/packages/react-devtools-shared/src/Logger.js index d37a33cf1c7eb..dd9dfb6202544 100644 --- a/packages/react-devtools-shared/src/Logger.js +++ b/packages/react-devtools-shared/src/Logger.js @@ -25,6 +25,9 @@ export type LoggerEvent = | { +event_name: 'selected-profiler-tab', } + | { + +event_name: 'selected-suspense-tab', + } | { +event_name: 'load-hook-names', +event_status: 'success' | 'error' | 'timeout' | 'unknown', diff --git a/packages/react-devtools-shared/src/backend/agent.js b/packages/react-devtools-shared/src/backend/agent.js index e883724f49765..1ae7f5dfb11b7 100644 --- a/packages/react-devtools-shared/src/backend/agent.js +++ b/packages/react-devtools-shared/src/backend/agent.js @@ -710,6 +710,16 @@ export default class Agent extends EventEmitter<{ rendererInterface.setTraceUpdatesEnabled(this._traceUpdatesEnabled); + const renderer = rendererInterface.renderer; + if (renderer !== null) { + const devRenderer = renderer.bundleType === 1; + const enableSuspenseTab = + devRenderer && renderer.version.includes('-experimental-'); + if (enableSuspenseTab) { + this._bridge.send('enableSuspenseTab'); + } + } + // When the renderer is attached, we need to tell it whether // we remember the previous selection that we'd like to restore. // It'll start tracking mounts for matches to the last selection path. diff --git a/packages/react-devtools-shared/src/bridge.js b/packages/react-devtools-shared/src/bridge.js index 3a12ae7415025..f0638ae896b60 100644 --- a/packages/react-devtools-shared/src/bridge.js +++ b/packages/react-devtools-shared/src/bridge.js @@ -178,6 +178,7 @@ export type BackendEvents = { backendInitialized: [], backendVersion: [string], bridgeProtocol: [BridgeProtocol], + enableSuspenseTab: [], extensionBackendInitialized: [], fastRefreshScheduled: [], getSavedPreferences: [], diff --git a/packages/react-devtools-shared/src/devtools/store.js b/packages/react-devtools-shared/src/devtools/store.js index 97c3adc88f032..3035c0ae4adba 100644 --- a/packages/react-devtools-shared/src/devtools/store.js +++ b/packages/react-devtools-shared/src/devtools/store.js @@ -95,6 +95,7 @@ export default class Store extends EventEmitter<{ backendVersion: [], collapseNodesByDefault: [], componentFilters: [], + enableSuspenseTab: [], error: [Error], hookSettings: [$ReadOnly], hostInstanceSelected: [Element['id']], @@ -172,6 +173,8 @@ export default class Store extends EventEmitter<{ _supportsClickToInspect: boolean = false; _supportsTimeline: boolean = false; _supportsTraceUpdates: boolean = false; + // Dynamically set if the renderer supports the Suspense tab. + _supportsSuspenseTab: boolean = false; _isReloadAndProfileFrontendSupported: boolean = false; _isReloadAndProfileBackendSupported: boolean = false; @@ -275,6 +278,7 @@ export default class Store extends EventEmitter<{ bridge.addListener('hookSettings', this.onHookSettings); bridge.addListener('backendInitialized', this.onBackendInitialized); bridge.addListener('selectElement', this.onHostInstanceSelected); + bridge.addListener('enableSuspenseTab', this.onEnableSuspenseTab); } // This is only used in tests to avoid memory leaks. @@ -1624,6 +1628,15 @@ export default class Store extends EventEmitter<{ } } + get supportsSuspenseTab(): boolean { + return this._supportsSuspenseTab; + } + + onEnableSuspenseTab = (): void => { + this._supportsSuspenseTab = true; + this.emit('enableSuspenseTab'); + }; + // The Store should never throw an Error without also emitting an event. // Otherwise Store errors will be invisible to users, // but the downstream errors they cause will be reported as bugs. diff --git a/packages/react-devtools-shared/src/devtools/views/DevTools.js b/packages/react-devtools-shared/src/devtools/views/DevTools.js index 0fe6293b9f2fe..fa02555e4cad4 100644 --- a/packages/react-devtools-shared/src/devtools/views/DevTools.js +++ b/packages/react-devtools-shared/src/devtools/views/DevTools.js @@ -23,6 +23,7 @@ import { } from './context'; import Components from './Components/Components'; import Profiler from './Profiler/Profiler'; +import SuspenseTab from './SuspenseTab/SuspenseTab'; import TabBar from './TabBar'; import EditorPane from './Editor/EditorPane'; import {SettingsContextController} from './Settings/SettingsContext'; @@ -54,7 +55,7 @@ import type {BrowserTheme} from 'react-devtools-shared/src/frontend/types'; import type {ReactFunctionLocation, ReactCallSite} from 'shared/ReactTypes'; import type {SourceSelection} from './Editor/EditorPane'; -export type TabID = 'components' | 'profiler'; +export type TabID = 'components' | 'profiler' | 'suspense'; export type ViewElementSource = ( source: ReactFunctionLocation | ReactCallSite, @@ -99,7 +100,9 @@ export type Props = { // but individual tabs (e.g. Components, Profiling) can be rendered into portals within their browser panels. componentsPortalContainer?: Element, profilerPortalContainer?: Element, + suspensePortalContainer?: Element, editorPortalContainer?: Element, + currentSelectedSource?: null | SourceSelection, // Loads and parses source maps for function components @@ -122,16 +125,37 @@ const profilerTab = { label: 'Profiler', title: 'React Profiler', }; +const suspenseTab = { + id: ('suspense': TabID), + icon: 'suspense', + label: 'Suspense', + title: 'React Suspense', +}; -const tabs = [componentsTab, profilerTab]; +const defaultTabs = [componentsTab, profilerTab]; +const tabsWithSuspense = [componentsTab, profilerTab, suspenseTab]; + +function useIsSuspenseTabEnabled(store: Store): boolean { + const subscribe = useCallback( + (onStoreChange: () => void) => { + store.addListener('enableSuspenseTab', onStoreChange); + return () => { + store.removeListener('enableSuspenseTab', onStoreChange); + }; + }, + [store], + ); + return React.useSyncExternalStore(subscribe, () => store.supportsSuspenseTab); +} export default function DevTools({ bridge, browserTheme = 'light', canViewElementSourceFunction, componentsPortalContainer, - profilerPortalContainer, editorPortalContainer, + profilerPortalContainer, + suspensePortalContainer, currentSelectedSource, defaultTab = 'components', enabledInspectedElementContextMenu = false, @@ -155,6 +179,8 @@ export default function DevTools({ LOCAL_STORAGE_DEFAULT_TAB_KEY, defaultTab, ); + const enableSuspenseTab = useIsSuspenseTabEnabled(store); + const tabs = enableSuspenseTab ? tabsWithSuspense : defaultTabs; let tab = currentTab; @@ -171,6 +197,8 @@ export default function DevTools({ if (showTabBar === true) { if (tabId === 'components') { logEvent({event_name: 'selected-components-tab'}); + } else if (tabId === 'suspense') { + logEvent({event_name: 'selected-suspense-tab'}); } else { logEvent({event_name: 'selected-profiler-tab'}); } @@ -241,6 +269,13 @@ export default function DevTools({ event.preventDefault(); event.stopPropagation(); break; + case '3': + if (tabs.length > 2) { + selectTab(tabs[2].id); + event.preventDefault(); + event.stopPropagation(); + } + break; } } }; @@ -321,6 +356,13 @@ export default function DevTools({ portalContainer={profilerPortalContainer} />
+
{editorPortalContainer ? ( + viewBox={viewBox}> {title && {title}} @@ -185,4 +191,9 @@ const PATH_STRICT_MODE_NON_COMPLIANT = ` 14c-.55 0-1-.45-1-1v-2c0-.55.45-1 1-1s1 .45 1 1v2c0 .55-.45 1-1 1zm1 4h-2v-2h2v2z `; +const PATH_SUSPEND = ` + M15 1H9v2h6V1zm-4 13h2V8h-2v6zm8.03-6.61l1.42-1.42c-.43-.51-.9-.99-1.41-1.41l-1.42 1.42C16.07 4.74 14.12 4 12 4c-4.97 + 0-9 4.03-9 9s4.02 9 9 9 9-4.03 9-9c0-2.12-.74-4.07-1.97-5.61zM12 20c-3.87 0-7-3.13-7-7s3.13-7 7-7 7 3.13 7 7-3.13 7-7 7z +`; + const PATH_WARNING = `M12 1l-12 22h24l-12-22zm-1 8h2v7h-2v-7zm1 11.25c-.69 0-1.25-.56-1.25-1.25s.56-1.25 1.25-1.25 1.25.56 1.25 1.25-.56 1.25-1.25 1.25z`; diff --git a/packages/react-devtools-shared/src/devtools/views/Settings/SettingsContext.js b/packages/react-devtools-shared/src/devtools/views/Settings/SettingsContext.js index 196ea806f6aac..c20249e89942f 100644 --- a/packages/react-devtools-shared/src/devtools/views/Settings/SettingsContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Settings/SettingsContext.js @@ -83,6 +83,7 @@ type Props = { children: React$Node, componentsPortalContainer?: Element, profilerPortalContainer?: Element, + suspensePortalContainer?: Element, }; function SettingsContextController({ @@ -90,6 +91,7 @@ function SettingsContextController({ children, componentsPortalContainer, profilerPortalContainer, + suspensePortalContainer, }: Props): React.Node { const bridge = useContext(BridgeContext); @@ -128,8 +130,18 @@ function SettingsContextController({ .documentElement: any): HTMLElement), ); } + if (suspensePortalContainer != null) { + array.push( + ((suspensePortalContainer.ownerDocument + .documentElement: any): HTMLElement), + ); + } return array; - }, [componentsPortalContainer, profilerPortalContainer]); + }, [ + componentsPortalContainer, + profilerPortalContainer, + suspensePortalContainer, + ]); useLayoutEffect(() => { switch (displayDensity) { diff --git a/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTab.js b/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTab.js new file mode 100644 index 0000000000000..01d11b3d7e130 --- /dev/null +++ b/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTab.js @@ -0,0 +1,8 @@ +import * as React from 'react'; +import portaledContent from '../portaledContent'; + +function SuspenseTab() { + return 'Under construction'; +} + +export default (portaledContent(SuspenseTab): React.ComponentType<{}>); From 904989f044d122ea332b7ee08185fcdc3424a40b Mon Sep 17 00:00:00 2001 From: dan Date: Mon, 28 Jul 2025 17:32:23 +0100 Subject: [PATCH 05/10] Clean up 19.1.1 changelog (#34023) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/facebook/react/pull/34021#issuecomment-3128006800. The purpose of the changelog is to communicate to React users what changed in the release. Therefore, it is important that the changelog is written oriented towards React end users. Historically this means that we omit internal-only changes, i.e. changes that have no effect on the end user behavior. If internal changes are mentioned in the changelog (e.g. if they affect end user behavior), they should be phrased in a way that is understandable to the end user — in particular, they should not refer to internal API names or concepts. We also try to group changes according to the publicly known packages. In this PR: - Make #33680 an actual link (otherwise it isn't linkified in CHANGELOG.md on GitHub). - Remove two changelog entries listed under "React" that don't affect anyone who upgrades the "React" package, that are phrased using terminology and internal function names unfamiliar to React users, and that seem to be RN-specific changes (so should probably go into the RN changelog that goes out with the next renderer sync that includes these changes). --- CHANGELOG.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index edc80475d3db1..1836850ff3ea2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,9 +1,7 @@ ## 19.1.1 (July 28, 2025) ### React -* Fixed Owner Stacks to work with ES2015 function.name semantics (#33680 by @hoxyq) -* Move the Fabric completeRoot call from `finalizeContainerChildren` to `replaceContainerChildren` to align how JS API and Fabric interpret `completeRoot` (#30513, #33064 by @kassens and @jackpope) -* Fix React retaining shadow nodes longer that it needs to (#33161, #33447 by @sammy-SC and @yungsters) +* Fixed Owner Stacks to work with ES2015 function.name semantics ([#33680](https://github.com/facebook/react/pull/33680) by @hoxyq) ## 19.1.0 (March 28, 2025) From 5dd622eabe38e01781b3699d0d81c4a16e302f09 Mon Sep 17 00:00:00 2001 From: lauren Date: Mon, 28 Jul 2025 12:46:30 -0400 Subject: [PATCH 06/10] [compiler] Disambiguate between void, implicit, and explicit returns (#33989) Adds a new property to ReturnTerminals to disambiguate whether it was explicit, implicit (arrow function expressions), or void (where it was omitted). I will use this property in the next PR adding a new validation pass. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33989). * #34022 * #34002 * #34001 * #33990 * __->__ #33989 --- .../babel-plugin-react-compiler/src/HIR/BuildHIR.ts | 3 +++ .../babel-plugin-react-compiler/src/HIR/HIR.ts | 12 ++++++++++++ .../babel-plugin-react-compiler/src/HIR/PrintHIR.ts | 2 +- .../babel-plugin-react-compiler/src/HIR/visitors.ts | 1 + .../src/Optimization/LowerContextAccess.ts | 1 + .../src/Optimization/OutlineJsx.ts | 1 + 6 files changed, 19 insertions(+), 1 deletion(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts index aa940c99e6128..3b11670146b60 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts @@ -189,6 +189,7 @@ export function lower( const fallthrough = builder.reserve('block'); const terminal: ReturnTerminal = { kind: 'return', + returnVariant: 'Implicit', loc: GeneratedSource, value: lowerExpressionToTemporary(builder, body), id: makeInstructionId(0), @@ -219,6 +220,7 @@ export function lower( builder.terminate( { kind: 'return', + returnVariant: 'Void', loc: GeneratedSource, value: lowerValueToTemporary(builder, { kind: 'Primitive', @@ -302,6 +304,7 @@ function lowerStatement( } const terminal: ReturnTerminal = { kind: 'return', + returnVariant: 'Explicit', loc: stmt.node.loc ?? GeneratedSource, value, id: makeInstructionId(0), diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts index deb725a0482e1..f9caa844f3c52 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -446,8 +446,20 @@ export type ThrowTerminal = { }; export type Case = {test: Place | null; block: BlockId}; +export type ReturnVariant = 'Void' | 'Implicit' | 'Explicit'; export type ReturnTerminal = { kind: 'return'; + /** + * Void: + * () => { ... } + * function() { ... } + * Implicit (ArrowFunctionExpression only): + * () => foo + * Explicit: + * () => { return ... } + * function () { return ... } + */ + returnVariant: ReturnVariant; loc: SourceLocation; value: Place; id: InstructionId; diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts index 869799073e49c..aa6a7b0c65cea 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts @@ -215,7 +215,7 @@ export function printTerminal(terminal: Terminal): Array | string { break; } case 'return': { - value = `[${terminal.id}] Return${ + value = `[${terminal.id}] Return ${terminal.returnVariant}${ terminal.value != null ? ' ' + printPlace(terminal.value) : '' }`; if (terminal.effects != null) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/visitors.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/visitors.ts index 88786bc5dd2bb..64702c8abcdb6 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/visitors.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/visitors.ts @@ -777,6 +777,7 @@ export function mapTerminalSuccessors( case 'return': { return { kind: 'return', + returnVariant: terminal.returnVariant, loc: terminal.loc, value: terminal.value, id: makeInstructionId(0), diff --git a/compiler/packages/babel-plugin-react-compiler/src/Optimization/LowerContextAccess.ts b/compiler/packages/babel-plugin-react-compiler/src/Optimization/LowerContextAccess.ts index 921ec59ecd2a1..2d0b073a04596 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Optimization/LowerContextAccess.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Optimization/LowerContextAccess.ts @@ -237,6 +237,7 @@ function emitSelectorFn(env: Environment, keys: Array): Instruction { terminal: { id: makeInstructionId(0), kind: 'return', + returnVariant: 'Explicit', loc: GeneratedSource, value: arrayInstr.lvalue, effects: null, diff --git a/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineJsx.ts b/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineJsx.ts index b7590a570197a..e59d60271a0d4 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineJsx.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineJsx.ts @@ -352,6 +352,7 @@ function emitOutlinedFn( terminal: { id: makeInstructionId(0), kind: 'return', + returnVariant: 'Explicit', loc: GeneratedSource, value: instructions.at(-1)!.lvalue, effects: null, From c60eebffea94a67f35c6ebbf7019e5b2145d4284 Mon Sep 17 00:00:00 2001 From: lauren Date: Mon, 28 Jul 2025 12:46:42 -0400 Subject: [PATCH 07/10] [compiler] Add new ValidateNoVoidUseMemo pass (#33990) Adds a new validation pass to validate against `useMemo`s that don't return anything. This usually indicates some kind of "useEffect"-like code that has side effects that need to be memoized to prevent overfiring, and is an anti-pattern. A follow up validation could also look at the return value of `useMemo`s to see if they are being used. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33990). * #34022 * #34002 * #34001 * __->__ #33990 * #33989 --- .../src/Entrypoint/Pipeline.ts | 4 + .../src/HIR/Environment.ts | 11 ++ .../src/Validation/ValidateNoVoidUseMemo.ts | 156 ++++++++++++++++++ .../src/Validation/index.ts | 1 + .../error.useMemo-no-return-value.expect.md | 41 +++++ .../compiler/error.useMemo-no-return-value.js | 15 ++ .../useMemo-arrow-implicit-return.expect.md | 40 +++++ .../compiler/useMemo-arrow-implicit-return.js | 5 + .../compiler/useMemo-empty-return.expect.md | 34 ++++ .../fixtures/compiler/useMemo-empty-return.js | 7 + .../useMemo-explicit-null-return.expect.md | 34 ++++ .../compiler/useMemo-explicit-null-return.js | 7 + .../useMemo-multiple-returns.expect.md | 51 ++++++ .../compiler/useMemo-multiple-returns.js | 10 ++ 14 files changed, 416 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoVoidUseMemo.ts create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useMemo-no-return-value.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useMemo-no-return-value.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-arrow-implicit-return.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-arrow-implicit-return.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-empty-return.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-empty-return.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-explicit-null-return.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-explicit-null-return.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-multiple-returns.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-multiple-returns.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts index 648ff01ba713c..1f9b3d3f8498b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -82,6 +82,7 @@ import { import {inferTypes} from '../TypeInference'; import { validateContextVariableLValues, + validateNoVoidUseMemo, validateHooksUsage, validateMemoizedEffectDependencies, validateNoCapitalizedCalls, @@ -167,6 +168,9 @@ function runWithEnvironment( validateContextVariableLValues(hir); validateUseMemo(hir).unwrap(); + if (env.config.validateNoVoidUseMemo) { + validateNoVoidUseMemo(hir).unwrap(); + } if ( env.isInferredMemoEnabled && diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts index 5b5b78fc5202c..ba7396e0d7d13 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -631,6 +631,17 @@ export const EnvironmentConfigSchema = z.object({ * ``` */ lowerContextAccess: ExternalFunctionSchema.nullable().default(null), + + /** + * If enabled, will validate useMemos that don't return any values: + * + * Valid: + * useMemo(() => foo, [foo]); + * useMemo(() => { return foo }, [foo]); + * Invalid: + * useMemo(() => { ... }, [...]); + */ + validateNoVoidUseMemo: z.boolean().default(false), }); export type EnvironmentConfig = z.infer; diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoVoidUseMemo.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoVoidUseMemo.ts new file mode 100644 index 0000000000000..c60ea6269d991 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoVoidUseMemo.ts @@ -0,0 +1,156 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import {CompilerError, ErrorSeverity} from '../CompilerError'; +import { + HIRFunction, + IdentifierId, + FunctionExpression, + SourceLocation, + Environment, + Instruction, + getHookKindForType, +} from '../HIR'; +import {Result} from '../Utils/Result'; + +type TemporariesSidemap = { + useMemoHooks: Map; + funcExprs: Map; + react: Set; +}; + +/** + * Validates that useMemo has at least one explicit return statement. + * + * Valid cases: + * - useMemo(() => value) // implicit arrow function return + * - useMemo(() => { return value; }) // explicit return + * - useMemo(() => { return; }) // explicit undefined + * - useMemo(() => { if (cond) return val; }) // at least one return + * + * Invalid cases: + * - useMemo(() => { console.log(); }) // no return statement at all + */ +export function validateNoVoidUseMemo( + fn: HIRFunction, +): Result { + const errors = new CompilerError(); + const sidemap: TemporariesSidemap = { + useMemoHooks: new Map(), + funcExprs: new Map(), + react: new Set(), + }; + + for (const [, block] of fn.body.blocks) { + for (const instr of block.instructions) { + collectTemporaries(instr, fn.env, sidemap); + } + } + + for (const [, block] of fn.body.blocks) { + for (const instr of block.instructions) { + if (instr.value.kind === 'CallExpression') { + const callee = instr.value.callee.identifier; + const useMemoHook = sidemap.useMemoHooks.get(callee.id); + + if (useMemoHook !== undefined && instr.value.args.length > 0) { + const firstArg = instr.value.args[0]; + if (firstArg.kind !== 'Identifier') { + continue; + } + + let funcToCheck = sidemap.funcExprs.get(firstArg.identifier.id); + + if (!funcToCheck) { + for (const [, searchBlock] of fn.body.blocks) { + for (const searchInstr of searchBlock.instructions) { + if ( + searchInstr.lvalue && + searchInstr.lvalue.identifier.id === firstArg.identifier.id && + searchInstr.value.kind === 'FunctionExpression' + ) { + funcToCheck = searchInstr.value; + break; + } + } + if (funcToCheck) break; + } + } + + if (funcToCheck) { + const hasReturn = checkFunctionHasNonVoidReturn( + funcToCheck.loweredFunc.func, + ); + + if (!hasReturn) { + errors.push({ + severity: ErrorSeverity.InvalidReact, + reason: `React Compiler has skipped optimizing this component because ${useMemoHook.name} doesn't return a value. ${useMemoHook.name} should only be used for memoizing values, not running arbitrary side effects.`, + loc: useMemoHook.loc, + suggestions: null, + description: null, + }); + } + } + } + } + } + } + return errors.asResult(); +} + +function checkFunctionHasNonVoidReturn(func: HIRFunction): boolean { + for (const [, block] of func.body.blocks) { + if (block.terminal.kind === 'return') { + if ( + block.terminal.returnVariant === 'Explicit' || + block.terminal.returnVariant === 'Implicit' + ) { + return true; + } + } + } + return false; +} + +function collectTemporaries( + instr: Instruction, + env: Environment, + sidemap: TemporariesSidemap, +): void { + const {value, lvalue} = instr; + switch (value.kind) { + case 'FunctionExpression': { + sidemap.funcExprs.set(lvalue.identifier.id, value); + break; + } + case 'LoadGlobal': { + const global = env.getGlobalDeclaration(value.binding, value.loc); + const hookKind = global !== null ? getHookKindForType(env, global) : null; + if (hookKind === 'useMemo') { + sidemap.useMemoHooks.set(lvalue.identifier.id, { + name: value.binding.name, + loc: instr.loc, + }); + } else if (value.binding.name === 'React') { + sidemap.react.add(lvalue.identifier.id); + } + break; + } + case 'PropertyLoad': { + if (sidemap.react.has(value.object.identifier.id)) { + if (value.property === 'useMemo') { + sidemap.useMemoHooks.set(lvalue.identifier.id, { + name: value.property, + loc: instr.loc, + }); + } + } + break; + } + } +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/index.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/index.ts index 3bf03f362fa19..2e7676dcd9788 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/index.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/index.ts @@ -6,6 +6,7 @@ */ export {validateContextVariableLValues} from './ValidateContextVariableLValues'; +export {validateNoVoidUseMemo} from './ValidateNoVoidUseMemo'; export {validateHooksUsage} from './ValidateHooksUsage'; export {validateMemoizedEffectDependencies} from './ValidateMemoizedEffectDependencies'; export {validateNoCapitalizedCalls} from './ValidateNoCapitalizedCalls'; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useMemo-no-return-value.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useMemo-no-return-value.expect.md new file mode 100644 index 0000000000000..e87c222e6ee39 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useMemo-no-return-value.expect.md @@ -0,0 +1,41 @@ + +## Input + +```javascript +// @validateNoVoidUseMemo +function Component() { + const value = useMemo(() => { + console.log('computing'); + }, []); + const value2 = React.useMemo(() => { + console.log('computing'); + }, []); + return ( +
+ {value} + {value2} +
+ ); +} + +``` + + +## Error + +``` +Found 1 error: + +Error: React Compiler has skipped optimizing this component because useMemo doesn't return a value. useMemo should only be used for memoizing values, not running arbitrary side effects. + +error.useMemo-no-return-value.ts:3:16 + 1 | // @validateNoVoidUseMemo + 2 | function Component() { +> 3 | const value = useMemo(() => { + | ^^^^^^^ React Compiler has skipped optimizing this component because useMemo doesn't return a value. useMemo should only be used for memoizing values, not running arbitrary side effects. + 4 | console.log('computing'); + 5 | }, []); + 6 | const value2 = React.useMemo(() => { +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useMemo-no-return-value.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useMemo-no-return-value.js new file mode 100644 index 0000000000000..0ce35e12f4ff8 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useMemo-no-return-value.js @@ -0,0 +1,15 @@ +// @validateNoVoidUseMemo +function Component() { + const value = useMemo(() => { + console.log('computing'); + }, []); + const value2 = React.useMemo(() => { + console.log('computing'); + }, []); + return ( +
+ {value} + {value2} +
+ ); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-arrow-implicit-return.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-arrow-implicit-return.expect.md new file mode 100644 index 0000000000000..df3dae1d83a0c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-arrow-implicit-return.expect.md @@ -0,0 +1,40 @@ + +## Input + +```javascript +// @validateNoVoidUseMemo +function Component() { + const value = useMemo(() => computeValue(), []); + return
{value}
; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateNoVoidUseMemo +function Component() { + const $ = _c(2); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = computeValue(); + $[0] = t0; + } else { + t0 = $[0]; + } + const value = t0; + let t1; + if ($[1] === Symbol.for("react.memo_cache_sentinel")) { + t1 =
{value}
; + $[1] = t1; + } else { + t1 = $[1]; + } + return t1; +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-arrow-implicit-return.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-arrow-implicit-return.js new file mode 100644 index 0000000000000..0ea121430d1da --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-arrow-implicit-return.js @@ -0,0 +1,5 @@ +// @validateNoVoidUseMemo +function Component() { + const value = useMemo(() => computeValue(), []); + return
{value}
; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-empty-return.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-empty-return.expect.md new file mode 100644 index 0000000000000..7be708ef5017c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-empty-return.expect.md @@ -0,0 +1,34 @@ + +## Input + +```javascript +// @validateNoVoidUseMemo +function Component() { + const value = useMemo(() => { + return; + }, []); + return
{value}
; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateNoVoidUseMemo +function Component() { + const $ = _c(1); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 =
{undefined}
; + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-empty-return.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-empty-return.js new file mode 100644 index 0000000000000..7985884d5657c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-empty-return.js @@ -0,0 +1,7 @@ +// @validateNoVoidUseMemo +function Component() { + const value = useMemo(() => { + return; + }, []); + return
{value}
; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-explicit-null-return.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-explicit-null-return.expect.md new file mode 100644 index 0000000000000..d35213b00800a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-explicit-null-return.expect.md @@ -0,0 +1,34 @@ + +## Input + +```javascript +// @validateNoVoidUseMemo +function Component() { + const value = useMemo(() => { + return null; + }, []); + return
{value}
; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateNoVoidUseMemo +function Component() { + const $ = _c(1); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 =
{null}
; + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-explicit-null-return.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-explicit-null-return.js new file mode 100644 index 0000000000000..9b0a1a8253dcb --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-explicit-null-return.js @@ -0,0 +1,7 @@ +// @validateNoVoidUseMemo +function Component() { + const value = useMemo(() => { + return null; + }, []); + return
{value}
; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-multiple-returns.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-multiple-returns.expect.md new file mode 100644 index 0000000000000..0fa2700721b4c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-multiple-returns.expect.md @@ -0,0 +1,51 @@ + +## Input + +```javascript +// @validateNoVoidUseMemo +function Component({items}) { + const value = useMemo(() => { + for (let item of items) { + if (item.match) return item; + } + return null; + }, [items]); + return
{value}
; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateNoVoidUseMemo +function Component(t0) { + const $ = _c(2); + const { items } = t0; + let t1; + bb0: { + for (const item of items) { + if (item.match) { + t1 = item; + break bb0; + } + } + + t1 = null; + } + const value = t1; + let t2; + if ($[0] !== value) { + t2 =
{value}
; + $[0] = value; + $[1] = t2; + } else { + t2 = $[1]; + } + return t2; +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-multiple-returns.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-multiple-returns.js new file mode 100644 index 0000000000000..dce32663f1cb4 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-multiple-returns.js @@ -0,0 +1,10 @@ +// @validateNoVoidUseMemo +function Component({items}) { + const value = useMemo(() => { + for (let item of items) { + if (item.match) return item; + } + return null; + }, [items]); + return
{value}
; +} From b5c16371091c1abbf77ec944c755bd139abc7568 Mon Sep 17 00:00:00 2001 From: lauren Date: Mon, 28 Jul 2025 12:54:43 -0400 Subject: [PATCH 08/10] [compiler] Reuse DropManualMemoization for ValidateNoVoidUseMemo (#34001) Much of the logic in the new validation pass is already implemented in DropManualMemoization, so let's combine them. I opted to keep the environment flag so we can more precisely control the rollout. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34001). * #34022 * #34002 * __->__ #34001 --- .../src/Entrypoint/Pipeline.ts | 6 +- .../src/Inference/DropManualMemoization.ts | 67 +++++++- .../src/Validation/ValidateNoVoidUseMemo.ts | 156 ------------------ .../src/Validation/index.ts | 1 - .../error.useMemo-no-return-value.expect.md | 33 +++- 5 files changed, 94 insertions(+), 169 deletions(-) delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoVoidUseMemo.ts diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts index 1f9b3d3f8498b..b7a679dd6241e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -82,7 +82,6 @@ import { import {inferTypes} from '../TypeInference'; import { validateContextVariableLValues, - validateNoVoidUseMemo, validateHooksUsage, validateMemoizedEffectDependencies, validateNoCapitalizedCalls, @@ -168,9 +167,6 @@ function runWithEnvironment( validateContextVariableLValues(hir); validateUseMemo(hir).unwrap(); - if (env.config.validateNoVoidUseMemo) { - validateNoVoidUseMemo(hir).unwrap(); - } if ( env.isInferredMemoEnabled && @@ -178,7 +174,7 @@ function runWithEnvironment( !env.config.disableMemoizationForDebugging && !env.config.enableChangeDetectionForDebugging ) { - dropManualMemoization(hir); + dropManualMemoization(hir).unwrap(); log({kind: 'hir', name: 'DropManualMemoization', value: hir}); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts index 306e636b12b3a..10303a9b6eaba 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts @@ -5,7 +5,12 @@ * LICENSE file in the root directory of this source tree. */ -import {CompilerError, SourceLocation} from '..'; +import { + CompilerDiagnostic, + CompilerError, + ErrorSeverity, + SourceLocation, +} from '..'; import { CallExpression, Effect, @@ -30,6 +35,7 @@ import { makeInstructionId, } from '../HIR'; import {createTemporaryPlace, markInstructionIds} from '../HIR/HIRBuilder'; +import {Result} from '../Utils/Result'; type ManualMemoCallee = { kind: 'useMemo' | 'useCallback'; @@ -341,8 +347,14 @@ function extractManualMemoizationArgs( * rely on type inference to find useMemo/useCallback invocations, and instead does basic tracking * of globals and property loads to find both direct calls as well as usage via the React namespace, * eg `React.useMemo()`. + * + * This pass also validates that useMemo callbacks return a value (not void), ensuring that useMemo + * is only used for memoizing values and not for running arbitrary side effects. */ -export function dropManualMemoization(func: HIRFunction): void { +export function dropManualMemoization( + func: HIRFunction, +): Result { + const errors = new CompilerError(); const isValidationEnabled = func.env.config.validatePreserveExistingMemoizationGuarantees || func.env.config.validateNoSetStateInRender || @@ -390,6 +402,41 @@ export function dropManualMemoization(func: HIRFunction): void { manualMemo.kind, sidemap, ); + + /** + * Bailout on void return useMemos. This is an anti-pattern where code might be using + * useMemo like useEffect: running arbirtary side-effects synced to changes in specific + * values. + */ + if ( + func.env.config.validateNoVoidUseMemo && + manualMemo.kind === 'useMemo' + ) { + const funcToCheck = sidemap.functions.get( + fnPlace.identifier.id, + )?.value; + if (funcToCheck !== undefined && funcToCheck.loweredFunc.func) { + if (!hasNonVoidReturn(funcToCheck.loweredFunc.func)) { + errors.pushDiagnostic( + CompilerDiagnostic.create({ + severity: ErrorSeverity.InvalidReact, + category: 'useMemo() callbacks must return a value', + description: `This ${ + manualMemo.loadInstr.value.kind === 'PropertyLoad' + ? 'React.useMemo' + : 'useMemo' + } callback doesn't return a value. useMemo is for computing and caching values, not for arbitrary side effects.`, + suggestions: null, + }).withDetail({ + kind: 'error', + loc: instr.value.loc, + message: 'useMemo() callbacks must return a value', + }), + ); + } + } + } + instr.value = getManualMemoizationReplacement( fnPlace, instr.value.loc, @@ -486,6 +533,8 @@ export function dropManualMemoization(func: HIRFunction): void { markInstructionIds(func.body); } } + + return errors.asResult(); } function findOptionalPlaces(fn: HIRFunction): Set { @@ -530,3 +579,17 @@ function findOptionalPlaces(fn: HIRFunction): Set { } return optionals; } + +function hasNonVoidReturn(func: HIRFunction): boolean { + for (const [, block] of func.body.blocks) { + if (block.terminal.kind === 'return') { + if ( + block.terminal.returnVariant === 'Explicit' || + block.terminal.returnVariant === 'Implicit' + ) { + return true; + } + } + } + return false; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoVoidUseMemo.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoVoidUseMemo.ts deleted file mode 100644 index c60ea6269d991..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoVoidUseMemo.ts +++ /dev/null @@ -1,156 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -import {CompilerError, ErrorSeverity} from '../CompilerError'; -import { - HIRFunction, - IdentifierId, - FunctionExpression, - SourceLocation, - Environment, - Instruction, - getHookKindForType, -} from '../HIR'; -import {Result} from '../Utils/Result'; - -type TemporariesSidemap = { - useMemoHooks: Map; - funcExprs: Map; - react: Set; -}; - -/** - * Validates that useMemo has at least one explicit return statement. - * - * Valid cases: - * - useMemo(() => value) // implicit arrow function return - * - useMemo(() => { return value; }) // explicit return - * - useMemo(() => { return; }) // explicit undefined - * - useMemo(() => { if (cond) return val; }) // at least one return - * - * Invalid cases: - * - useMemo(() => { console.log(); }) // no return statement at all - */ -export function validateNoVoidUseMemo( - fn: HIRFunction, -): Result { - const errors = new CompilerError(); - const sidemap: TemporariesSidemap = { - useMemoHooks: new Map(), - funcExprs: new Map(), - react: new Set(), - }; - - for (const [, block] of fn.body.blocks) { - for (const instr of block.instructions) { - collectTemporaries(instr, fn.env, sidemap); - } - } - - for (const [, block] of fn.body.blocks) { - for (const instr of block.instructions) { - if (instr.value.kind === 'CallExpression') { - const callee = instr.value.callee.identifier; - const useMemoHook = sidemap.useMemoHooks.get(callee.id); - - if (useMemoHook !== undefined && instr.value.args.length > 0) { - const firstArg = instr.value.args[0]; - if (firstArg.kind !== 'Identifier') { - continue; - } - - let funcToCheck = sidemap.funcExprs.get(firstArg.identifier.id); - - if (!funcToCheck) { - for (const [, searchBlock] of fn.body.blocks) { - for (const searchInstr of searchBlock.instructions) { - if ( - searchInstr.lvalue && - searchInstr.lvalue.identifier.id === firstArg.identifier.id && - searchInstr.value.kind === 'FunctionExpression' - ) { - funcToCheck = searchInstr.value; - break; - } - } - if (funcToCheck) break; - } - } - - if (funcToCheck) { - const hasReturn = checkFunctionHasNonVoidReturn( - funcToCheck.loweredFunc.func, - ); - - if (!hasReturn) { - errors.push({ - severity: ErrorSeverity.InvalidReact, - reason: `React Compiler has skipped optimizing this component because ${useMemoHook.name} doesn't return a value. ${useMemoHook.name} should only be used for memoizing values, not running arbitrary side effects.`, - loc: useMemoHook.loc, - suggestions: null, - description: null, - }); - } - } - } - } - } - } - return errors.asResult(); -} - -function checkFunctionHasNonVoidReturn(func: HIRFunction): boolean { - for (const [, block] of func.body.blocks) { - if (block.terminal.kind === 'return') { - if ( - block.terminal.returnVariant === 'Explicit' || - block.terminal.returnVariant === 'Implicit' - ) { - return true; - } - } - } - return false; -} - -function collectTemporaries( - instr: Instruction, - env: Environment, - sidemap: TemporariesSidemap, -): void { - const {value, lvalue} = instr; - switch (value.kind) { - case 'FunctionExpression': { - sidemap.funcExprs.set(lvalue.identifier.id, value); - break; - } - case 'LoadGlobal': { - const global = env.getGlobalDeclaration(value.binding, value.loc); - const hookKind = global !== null ? getHookKindForType(env, global) : null; - if (hookKind === 'useMemo') { - sidemap.useMemoHooks.set(lvalue.identifier.id, { - name: value.binding.name, - loc: instr.loc, - }); - } else if (value.binding.name === 'React') { - sidemap.react.add(lvalue.identifier.id); - } - break; - } - case 'PropertyLoad': { - if (sidemap.react.has(value.object.identifier.id)) { - if (value.property === 'useMemo') { - sidemap.useMemoHooks.set(lvalue.identifier.id, { - name: value.property, - loc: instr.loc, - }); - } - } - break; - } - } -} diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/index.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/index.ts index 2e7676dcd9788..3bf03f362fa19 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/index.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/index.ts @@ -6,7 +6,6 @@ */ export {validateContextVariableLValues} from './ValidateContextVariableLValues'; -export {validateNoVoidUseMemo} from './ValidateNoVoidUseMemo'; export {validateHooksUsage} from './ValidateHooksUsage'; export {validateMemoizedEffectDependencies} from './ValidateMemoizedEffectDependencies'; export {validateNoCapitalizedCalls} from './ValidateNoCapitalizedCalls'; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useMemo-no-return-value.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useMemo-no-return-value.expect.md index e87c222e6ee39..fb5959640662c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useMemo-no-return-value.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useMemo-no-return-value.expect.md @@ -24,18 +24,41 @@ function Component() { ## Error ``` -Found 1 error: +Found 2 errors: -Error: React Compiler has skipped optimizing this component because useMemo doesn't return a value. useMemo should only be used for memoizing values, not running arbitrary side effects. +Error: useMemo() callbacks must return a value + +This useMemo callback doesn't return a value. useMemo is for computing and caching values, not for arbitrary side effects. error.useMemo-no-return-value.ts:3:16 1 | // @validateNoVoidUseMemo 2 | function Component() { > 3 | const value = useMemo(() => { - | ^^^^^^^ React Compiler has skipped optimizing this component because useMemo doesn't return a value. useMemo should only be used for memoizing values, not running arbitrary side effects. - 4 | console.log('computing'); - 5 | }, []); + | ^^^^^^^^^^^^^^^ +> 4 | console.log('computing'); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +> 5 | }, []); + | ^^^^^^^^^ useMemo() callbacks must return a value 6 | const value2 = React.useMemo(() => { + 7 | console.log('computing'); + 8 | }, []); + +Error: useMemo() callbacks must return a value + +This React.useMemo callback doesn't return a value. useMemo is for computing and caching values, not for arbitrary side effects. + +error.useMemo-no-return-value.ts:6:17 + 4 | console.log('computing'); + 5 | }, []); +> 6 | const value2 = React.useMemo(() => { + | ^^^^^^^^^^^^^^^^^^^^^ +> 7 | console.log('computing'); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +> 8 | }, []); + | ^^^^^^^^^ useMemo() callbacks must return a value + 9 | return ( + 10 |
+ 11 | {value} ``` \ No newline at end of file From 6b22f31f1ac88cce1b38c67a5c97c7ab0e832823 Mon Sep 17 00:00:00 2001 From: lauren Date: Mon, 28 Jul 2025 13:25:25 -0400 Subject: [PATCH 09/10] [compiler] Aggregate all errors reported from DropManualMemoization (#34002) Noticed this from my previous PR that this pass was throwing on the first error. This PR is a small refactor to aggregate every violation and report them all at once. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34002). * #34022 * __->__ #34002 --- .../src/Inference/DropManualMemoization.ts | 105 +++++++++++++----- ...ror.useMemo-non-literal-depslist.expect.md | 2 + ....validate-useMemo-named-function.expect.md | 2 + 3 files changed, 80 insertions(+), 29 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts index 10303a9b6eaba..7aeb3edb22a11 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts @@ -289,26 +289,43 @@ function extractManualMemoizationArgs( instr: TInstruction | TInstruction, kind: 'useCallback' | 'useMemo', sidemap: IdentifierSidemap, + errors: CompilerError, ): { - fnPlace: Place; + fnPlace: Place | null; depsList: Array | null; } { const [fnPlace, depsListPlace] = instr.value.args as Array< Place | SpreadPattern | undefined >; if (fnPlace == null) { - CompilerError.throwInvalidReact({ - reason: `Expected a callback function to be passed to ${kind}`, - loc: instr.value.loc, - suggestions: null, - }); + errors.pushDiagnostic( + CompilerDiagnostic.create({ + severity: ErrorSeverity.InvalidReact, + category: `Expected a callback function to be passed to ${kind}`, + description: `Expected a callback function to be passed to ${kind}`, + suggestions: null, + }).withDetail({ + kind: 'error', + loc: instr.value.loc, + message: `Expected a callback function to be passed to ${kind}`, + }), + ); + return {fnPlace: null, depsList: null}; } if (fnPlace.kind === 'Spread' || depsListPlace?.kind === 'Spread') { - CompilerError.throwInvalidReact({ - reason: `Unexpected spread argument to ${kind}`, - loc: instr.value.loc, - suggestions: null, - }); + errors.pushDiagnostic( + CompilerDiagnostic.create({ + severity: ErrorSeverity.InvalidReact, + category: `Unexpected spread argument to ${kind}`, + description: `Unexpected spread argument to ${kind}`, + suggestions: null, + }).withDetail({ + kind: 'error', + loc: instr.value.loc, + message: `Unexpected spread argument to ${kind}`, + }), + ); + return {fnPlace: null, depsList: null}; } let depsList: Array | null = null; if (depsListPlace != null) { @@ -316,23 +333,40 @@ function extractManualMemoizationArgs( depsListPlace.identifier.id, ); if (maybeDepsList == null) { - CompilerError.throwInvalidReact({ - reason: `Expected the dependency list for ${kind} to be an array literal`, - suggestions: null, - loc: depsListPlace.loc, - }); + errors.pushDiagnostic( + CompilerDiagnostic.create({ + severity: ErrorSeverity.InvalidReact, + category: `Expected the dependency list for ${kind} to be an array literal`, + description: `Expected the dependency list for ${kind} to be an array literal`, + suggestions: null, + }).withDetail({ + kind: 'error', + loc: depsListPlace.loc, + message: `Expected the dependency list for ${kind} to be an array literal`, + }), + ); + return {fnPlace, depsList: null}; } - depsList = maybeDepsList.map(dep => { + depsList = []; + for (const dep of maybeDepsList) { const maybeDep = sidemap.maybeDeps.get(dep.identifier.id); if (maybeDep == null) { - CompilerError.throwInvalidReact({ - reason: `Expected the dependency list to be an array of simple expressions (e.g. \`x\`, \`x.y.z\`, \`x?.y?.z\`)`, - suggestions: null, - loc: dep.loc, - }); + errors.pushDiagnostic( + CompilerDiagnostic.create({ + severity: ErrorSeverity.InvalidReact, + category: `Expected the dependency list to be an array of simple expressions (e.g. \`x\`, \`x.y.z\`, \`x?.y?.z\`)`, + description: `Expected the dependency list to be an array of simple expressions (e.g. \`x\`, \`x.y.z\`, \`x?.y?.z\`)`, + suggestions: null, + }).withDetail({ + kind: 'error', + loc: dep.loc, + message: `Expected the dependency list to be an array of simple expressions (e.g. \`x\`, \`x.y.z\`, \`x?.y?.z\`)`, + }), + ); + } else { + depsList.push(maybeDep); } - return maybeDep; - }); + } } return { fnPlace, @@ -401,8 +435,13 @@ export function dropManualMemoization( instr as TInstruction | TInstruction, manualMemo.kind, sidemap, + errors, ); + if (fnPlace == null) { + continue; + } + /** * Bailout on void return useMemos. This is an anti-pattern where code might be using * useMemo like useEffect: running arbirtary side-effects synced to changes in specific @@ -457,11 +496,19 @@ export function dropManualMemoization( * is rare and likely sketchy. */ if (!sidemap.functions.has(fnPlace.identifier.id)) { - CompilerError.throwInvalidReact({ - reason: `Expected the first argument to be an inline function expression`, - suggestions: [], - loc: fnPlace.loc, - }); + errors.pushDiagnostic( + CompilerDiagnostic.create({ + severity: ErrorSeverity.InvalidReact, + category: `Expected the first argument to be an inline function expression`, + description: `Expected the first argument to be an inline function expression`, + suggestions: [], + }).withDetail({ + kind: 'error', + loc: fnPlace.loc, + message: `Expected the first argument to be an inline function expression`, + }), + ); + continue; } const memoDecl: Place = manualMemo.kind === 'useMemo' diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useMemo-non-literal-depslist.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useMemo-non-literal-depslist.expect.md index 4e1e2543a79e0..188814ee02ce8 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useMemo-non-literal-depslist.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useMemo-non-literal-depslist.expect.md @@ -32,6 +32,8 @@ Found 1 error: Error: Expected the dependency list for useMemo to be an array literal +Expected the dependency list for useMemo to be an array literal + error.useMemo-non-literal-depslist.ts:10:4 8 | return text.toUpperCase(); 9 | }, diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.validate-useMemo-named-function.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.validate-useMemo-named-function.expect.md index f433f8cb889b0..27af59e175d44 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.validate-useMemo-named-function.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.validate-useMemo-named-function.expect.md @@ -24,6 +24,8 @@ Found 1 error: Error: Expected the first argument to be an inline function expression +Expected the first argument to be an inline function expression + error.validate-useMemo-named-function.ts:9:20 7 | // for now. 8 | function Component(props) { From 7ee7571212bc02354b852752a98b23bc90546fdf Mon Sep 17 00:00:00 2001 From: lauren Date: Mon, 28 Jul 2025 13:42:14 -0400 Subject: [PATCH 10/10] [compiler] Enable validateNoVoidUseMemo in eslint & playground (#34022) Enables `validateNoVoidUseMemo` by default only in eslint (it defaults to false otherwise) as well as the playground. --- compiler/apps/playground/components/Editor/EditorImpl.tsx | 1 + .../eslint-plugin-react-compiler/src/rules/ReactCompilerRule.ts | 1 + packages/eslint-plugin-react-hooks/src/rules/ReactCompiler.ts | 1 + 3 files changed, 3 insertions(+) diff --git a/compiler/apps/playground/components/Editor/EditorImpl.tsx b/compiler/apps/playground/components/Editor/EditorImpl.tsx index 0ced1e54ed76d..2a8697157a3bd 100644 --- a/compiler/apps/playground/components/Editor/EditorImpl.tsx +++ b/compiler/apps/playground/components/Editor/EditorImpl.tsx @@ -219,6 +219,7 @@ function compile( validateNoImpureFunctionsInRender: true, validateStaticComponents: true, validateNoFreezingKnownMutableFunctions: true, + validateNoVoidUseMemo: true, } : { /* use defaults for compiler mode */ diff --git a/compiler/packages/eslint-plugin-react-compiler/src/rules/ReactCompilerRule.ts b/compiler/packages/eslint-plugin-react-compiler/src/rules/ReactCompilerRule.ts index 1d326012864b9..618d3b78b9dfd 100644 --- a/compiler/packages/eslint-plugin-react-compiler/src/rules/ReactCompilerRule.ts +++ b/compiler/packages/eslint-plugin-react-compiler/src/rules/ReactCompilerRule.ts @@ -108,6 +108,7 @@ const COMPILER_OPTIONS: Partial = { validateNoImpureFunctionsInRender: true, validateStaticComponents: true, validateNoFreezingKnownMutableFunctions: true, + validateNoVoidUseMemo: true, }), }; diff --git a/packages/eslint-plugin-react-hooks/src/rules/ReactCompiler.ts b/packages/eslint-plugin-react-hooks/src/rules/ReactCompiler.ts index 0492c2c30c309..8961561ef965a 100644 --- a/packages/eslint-plugin-react-hooks/src/rules/ReactCompiler.ts +++ b/packages/eslint-plugin-react-hooks/src/rules/ReactCompiler.ts @@ -110,6 +110,7 @@ const COMPILER_OPTIONS: Partial = { validateNoImpureFunctionsInRender: true, validateStaticComponents: true, validateNoFreezingKnownMutableFunctions: true, + validateNoVoidUseMemo: true, }), };