From 1bcdd224b10999138ef5aa1c1917b28d918e5421 Mon Sep 17 00:00:00 2001 From: Joseph Savona <6425824+josephsavona@users.noreply.github.com> Date: Thu, 18 Sep 2025 09:26:10 -0700 Subject: [PATCH 1/4] [compiler] Don't show hint about ref-like naming if we infer another type (#34521) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some components accept a union of a ref callback function or ref object. In this case we may infer the type as a function due to the presence of invoking the ref callback function. In that case, we currently report a "Hint: name `fooRef` as "ref" or with a "-Ref" suffix..." even though the variable is already named appropriately — the problem is that we inferred a non-ref type. So here we check the type and don't report this hint if we inferred another type. --- .../Inference/InferMutationAliasingEffects.ts | 10 +++- ...to-inferred-ref-prop-in-callback.expect.md | 48 +++++++++++++++++++ ...igning-to-inferred-ref-prop-in-callback.js | 20 ++++++++ 3 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-allow-assigning-to-inferred-ref-prop-in-callback.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-allow-assigning-to-inferred-ref-prop-in-callback.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts index 911f71329b440..d2f2ba61d695a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts @@ -1816,8 +1816,16 @@ function computeSignatureForInstruction( } case 'PropertyStore': case 'ComputedStore': { + /** + * Add a hint about naming as "ref"/"-Ref", but only if we weren't able to infer any + * type for the object. In some cases the variable may be named like a ref, but is + * also used as a ref callback such that we infer the type as a function rather than + * a ref. + */ const mutationReason: MutationReason | null = - value.kind === 'PropertyStore' && value.property === 'current' + value.kind === 'PropertyStore' && + value.property === 'current' && + value.object.identifier.type.kind === 'Type' ? {kind: 'AssignCurrentProperty'} : null; effects.push({ diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-allow-assigning-to-inferred-ref-prop-in-callback.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-allow-assigning-to-inferred-ref-prop-in-callback.expect.md new file mode 100644 index 0000000000000..ec187f6bc23b0 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-allow-assigning-to-inferred-ref-prop-in-callback.expect.md @@ -0,0 +1,48 @@ + +## Input + +```javascript +// @validateRefAccessDuringRender + +function useHook(parentRef) { + // Some components accept a union of "callback" refs and ref objects, which + // we can't currently represent + const elementRef = useRef(null); + const handler = instance => { + elementRef.current = instance; + if (parentRef != null) { + if (typeof parentRef === 'function') { + // This call infers the type of `parentRef` as a function... + parentRef(instance); + } else { + // So this assignment fails since we don't know its a ref + parentRef.current = instance; + } + } + }; + return handler; +} + +``` + + +## Error + +``` +Found 1 error: + +Error: This value cannot be modified + +Modifying component props or hook arguments is not allowed. Consider using a local variable instead. + +error.todo-allow-assigning-to-inferred-ref-prop-in-callback.ts:15:8 + 13 | } else { + 14 | // So this assignment fails since we don't know its a ref +> 15 | parentRef.current = instance; + | ^^^^^^^^^ `parentRef` cannot be modified + 16 | } + 17 | } + 18 | }; +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-allow-assigning-to-inferred-ref-prop-in-callback.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-allow-assigning-to-inferred-ref-prop-in-callback.js new file mode 100644 index 0000000000000..de2e1d0c968c6 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-allow-assigning-to-inferred-ref-prop-in-callback.js @@ -0,0 +1,20 @@ +// @validateRefAccessDuringRender + +function useHook(parentRef) { + // Some components accept a union of "callback" refs and ref objects, which + // we can't currently represent + const elementRef = useRef(null); + const handler = instance => { + elementRef.current = instance; + if (parentRef != null) { + if (typeof parentRef === 'function') { + // This call infers the type of `parentRef` as a function... + parentRef(instance); + } else { + // So this assignment fails since we don't know its a ref + parentRef.current = instance; + } + } + }; + return handler; +} From 581321160fe0d9d9ef461174c7e14cbbed341d28 Mon Sep 17 00:00:00 2001 From: zeki <153834382+zekariasasaminew@users.noreply.github.com> Date: Thu, 18 Sep 2025 11:34:47 -0500 Subject: [PATCH 2/4] [Compiler Bug] Complier mark ts instantiation expression as reorderable in build hir (#34488) ## Summary The React Compiler rejected a default parameter that contains a TSInstantiationExpression with the todo message that the expression cannot be safely reordered. This change teaches the reorder check in BuildHIR.ts to treat TSInstantiationExpression as reorderable. This is safe because TypeScript instantiation only affects types and is erased at runtime, so it has no side effects and does not change semantics. ## How did you test this change? ``` Set-Content testfilter.txt 'ts-instantiation-default-param' yarn test --filter --update yarn test --filter ``` I added a fixture: > compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ts-instantiation-default-param.js --- .../src/HIR/BuildHIR.ts | 6 ++ .../ts-instantiation-default-param.expect.md | 62 +++++++++++++++++++ .../ts-instantiation-default-param.tsx | 13 ++++ 3 files changed, 81 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ts-instantiation-default-param.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ts-instantiation-default-param.tsx 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 702eaf0f692a2..0ae338f5c7863 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts @@ -3081,6 +3081,12 @@ function isReorderableExpression( return true; } } + case 'TSInstantiationExpression': { + const innerExpr = (expr as NodePath).get( + 'expression', + ) as NodePath; + return isReorderableExpression(builder, innerExpr, allowLocalIdentifiers); + } case 'RegExpLiteral': case 'StringLiteral': case 'NumericLiteral': diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ts-instantiation-default-param.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ts-instantiation-default-param.expect.md new file mode 100644 index 0000000000000..b80c0370f798b --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ts-instantiation-default-param.expect.md @@ -0,0 +1,62 @@ + +## Input + +```javascript +function id(x: T): T { + return x; +} + +export function Component({fn = id}: {fn?: (x: T) => T}) { + const value = fn('hi' as T); + return
{String(value)}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +function id(x) { + return x; +} + +export function Component(t0) { + const $ = _c(4); + const { fn: t1 } = t0; + const fn = t1 === undefined ? id : t1; + let t2; + if ($[0] !== fn) { + t2 = fn("hi" as T); + $[0] = fn; + $[1] = t2; + } else { + t2 = $[1]; + } + const value = t2; + const t3 = String(value); + let t4; + if ($[2] !== t3) { + t4 =
{t3}
; + $[2] = t3; + $[3] = t4; + } else { + t4 = $[3]; + } + return t4; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + +### Eval output +(kind: ok)
hi
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ts-instantiation-default-param.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ts-instantiation-default-param.tsx new file mode 100644 index 0000000000000..6382962edda9b --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ts-instantiation-default-param.tsx @@ -0,0 +1,13 @@ +function id(x: T): T { + return x; +} + +export function Component({fn = id}: {fn?: (x: T) => T}) { + const value = fn('hi' as T); + return
{String(value)}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; From 755cebad6b80ef3d248d5ee38e925dceedd8f217 Mon Sep 17 00:00:00 2001 From: "Sebastian \"Sebbie\" Silbermann" Date: Thu, 18 Sep 2025 18:37:00 +0200 Subject: [PATCH 3/4] [DevTools] Elevate Suspense rects to visualize hierarchy (#34455) --- .../src/devtools/constants.js | 6 + .../views/SuspenseTab/SuspenseRects.css | 42 ++++- .../views/SuspenseTab/SuspenseRects.js | 154 ++++++++++++------ 3 files changed, 147 insertions(+), 55 deletions(-) diff --git a/packages/react-devtools-shared/src/devtools/constants.js b/packages/react-devtools-shared/src/devtools/constants.js index ee13e5b1630a2..cfb73713b089e 100644 --- a/packages/react-devtools-shared/src/devtools/constants.js +++ b/packages/react-devtools-shared/src/devtools/constants.js @@ -163,6 +163,9 @@ export const THEME_STYLES: {[style: Theme | DisplayDensity]: any, ...} = { '--color-scroll-track': '#fafafa', '--color-tooltip-background': 'rgba(0, 0, 0, 0.9)', '--color-tooltip-text': '#ffffff', + + '--elevation-4': + '0 2px 4px -1px rgba(0,0,0,.2),0 4px 5px 0 rgba(0,0,0,.14),0 1px 10px 0 rgba(0,0,0,.12)', }, dark: { '--color-attribute-name': '#9d87d2', @@ -315,6 +318,9 @@ export const THEME_STYLES: {[style: Theme | DisplayDensity]: any, ...} = { '--color-scroll-track': '#313640', '--color-tooltip-background': 'rgba(255, 255, 255, 0.95)', '--color-tooltip-text': '#000000', + + '--elevation-4': + '0 2px 8px 0 rgba(0,0,0,0.32),0 4px 12px 0 rgba(0,0,0,0.24),0 1px 10px 0 rgba(0,0,0,0.18)', }, compact: { '--font-size-monospace-small': '9px', diff --git a/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseRects.css b/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseRects.css index d9ecf2cad75d6..7e74f21eef08c 100644 --- a/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseRects.css +++ b/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseRects.css @@ -2,14 +2,40 @@ padding: .25rem; } -.SuspenseRect { - fill: transparent; - stroke: var(--color-background-selected); - stroke-width: 1px; - vector-effect: non-scaling-stroke; - paint-order: stroke; +.SuspenseRectsViewBox { + position: relative; } -[data-highlighted='true'] > .SuspenseRect { - fill: var(--color-selected-tree-highlight-active); +.SuspenseRectsBoundary { + pointer-events: all; +} + +.SuspenseRectsBoundaryChildren { + pointer-events: none; + /** + * So that the shadow of Boundaries within is clipped off. + * Otherwise it would look like this boundary is further elevated. + */ + overflow: hidden; +} + +.SuspenseRectsRect { + box-shadow: var(--elevation-4); + pointer-events: all; + outline-style: solid; + outline-width: 1px; +} + +.SuspenseRectsScaledRect { + position: absolute; + outline-color: var(--color-background-selected); +} + +/* highlight this boundary */ +.SuspenseRectsBoundary:hover:not(:has(.SuspenseRectsBoundary:hover)) > .SuspenseRectsRect, .SuspenseRectsBoundary[data-highlighted='true'] > .SuspenseRectsRect { + background-color: var(--color-background-hover); +} + +.SuspenseRectsRect[data-highlighted='true'] { + background-color: var(--color-selected-tree-highlight-active); } diff --git a/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseRects.js b/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseRects.js index a03439c07d9df..39c6f1c492517 100644 --- a/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseRects.js +++ b/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseRects.js @@ -12,9 +12,13 @@ import type { SuspenseNode, Rect, } from 'react-devtools-shared/src/frontend/types'; +import typeof { + SyntheticMouseEvent, + SyntheticPointerEvent, +} from 'react-dom-bindings/src/events/SyntheticEvent'; import * as React from 'react'; -import {useContext} from 'react'; +import {createContext, useContext} from 'react'; import { TreeDispatcherContext, TreeStateContext, @@ -26,19 +30,32 @@ import { SuspenseTreeStateContext, SuspenseTreeDispatcherContext, } from './SuspenseTreeContext'; -import typeof { - SyntheticMouseEvent, - SyntheticPointerEvent, -} from 'react-dom-bindings/src/events/SyntheticEvent'; -function SuspenseRect({rect}: {rect: Rect}): React$Node { +function ScaledRect({ + className, + rect, + ...props +}: { + className: string, + rect: Rect, + ... +}): React$Node { + const viewBox = useContext(ViewBox); + const width = (rect.width / viewBox.width) * 100 + '%'; + const height = (rect.height / viewBox.height) * 100 + '%'; + const x = ((rect.x - viewBox.x) / viewBox.width) * 100 + '%'; + const y = ((rect.y - viewBox.y) / viewBox.height) * 100 + '%'; + return ( - ); } @@ -97,24 +114,67 @@ function SuspenseRects({ // TODO: Use the nearest Suspense boundary const selected = inspectedElementID === suspenseID; + const boundingBox = getBoundingBox(suspense.rects); + return ( - - {suspense.name} - {suspense.rects !== null && - suspense.rects.map((rect, index) => { - return ; - })} - {suspense.children.map(childID => { - return ; - })} - + + + {suspense.rects !== null && + suspense.rects.map((rect, index) => { + return ( + + ); + })} + {suspense.children.length > 0 && ( + + {suspense.children.map(childID => { + return ; + })} + + )} + + ); } +function getBoundingBox(rects: $ReadOnlyArray | null): Rect { + if (rects === null || rects.length === 0) { + return {x: 0, y: 0, width: 0, height: 0}; + } + + let minX = Number.POSITIVE_INFINITY; + let minY = Number.POSITIVE_INFINITY; + let maxX = Number.NEGATIVE_INFINITY; + let maxY = Number.NEGATIVE_INFINITY; + + for (let i = 0; i < rects.length; i++) { + const rect = rects[i]; + minX = Math.min(minX, rect.x); + minY = Math.min(minY, rect.y); + maxX = Math.max(maxX, rect.x + rect.width); + maxY = Math.max(maxY, rect.y + rect.height); + } + + return { + x: minX, + y: minY, + width: maxX - minX, + height: maxY - minY, + }; +} + function getDocumentBoundingRect( store: Store, roots: $ReadOnlyArray, @@ -169,42 +229,42 @@ function SuspenseRectsShell({ const store = useContext(StoreContext); const root = store.getSuspenseByID(rootID); if (root === null) { - console.warn(` Could not find suspense node id ${rootID}`); + // getSuspenseByID will have already warned return null; } - return ( - - {root.children.map(childID => { - return ; - })} - - ); + return root.children.map(childID => { + return ; + }); } +const ViewBox = createContext((null: any)); + function SuspenseRectsContainer(): React$Node { const store = useContext(StoreContext); // TODO: This relies on a full re-render of all children when the Suspense tree changes. const {roots} = useContext(SuspenseTreeStateContext); - const boundingRect = getDocumentBoundingRect(store, roots); + const boundingBox = getDocumentBoundingRect(store, roots); + const boundingBoxWidth = boundingBox.width; + const heightScale = + boundingBoxWidth === 0 ? 1 : boundingBox.height / boundingBoxWidth; + // Scales the inspected document to fit into the available width const width = '100%'; - const boundingRectWidth = boundingRect.width; - const height = - (boundingRectWidth === 0 ? 0 : boundingRect.height / boundingRect.width) * - 100 + - '%'; + const aspectRatio = `1 / ${heightScale}`; return (
- - {roots.map(rootID => { - return ; - })} - + +
+ {roots.map(rootID => { + return ; + })} +
+
); } From 03a96c75db12eeac36b4d5d88c13e3a12d46efe1 Mon Sep 17 00:00:00 2001 From: "Sebastian \"Sebbie\" Silbermann" Date: Thu, 18 Sep 2025 18:50:23 +0200 Subject: [PATCH 4/4] [DevTools] Record Suspense node for roots in legacy renderers (#34516) --- .../src/backend/legacy/renderer.js | 22 ++++++++++++++++++- .../src/devtools/store.js | 4 ++-- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/legacy/renderer.js b/packages/react-devtools-shared/src/backend/legacy/renderer.js index 2915d2cd30554..b59c0292942c6 100644 --- a/packages/react-devtools-shared/src/backend/legacy/renderer.js +++ b/packages/react-devtools-shared/src/backend/legacy/renderer.js @@ -34,6 +34,8 @@ import { TREE_OPERATION_ADD, TREE_OPERATION_REMOVE, TREE_OPERATION_REORDER_CHILDREN, + SUSPENSE_TREE_OPERATION_ADD, + SUSPENSE_TREE_OPERATION_REMOVE, UNKNOWN_SUSPENDERS_NONE, } from '../../constants'; import {decorateMany, forceUpdate, restoreMany} from './utils'; @@ -411,6 +413,13 @@ export function attach( pushOperation(0); // StrictMode supported? pushOperation(hasOwnerMetadata ? 1 : 0); pushOperation(supportsTogglingSuspense ? 1 : 0); + + pushOperation(SUSPENSE_TREE_OPERATION_ADD); + pushOperation(id); + pushOperation(parentID); + pushOperation(getStringID(null)); // name + // TODO: Measure rect of root + pushOperation(-1); } else { const type = getElementType(internalInstance); const {displayName, key} = getData(internalInstance); @@ -449,7 +458,12 @@ export function attach( } function recordUnmount(internalInstance: InternalInstance, id: number) { - pendingUnmountedIDs.push(id); + const isRoot = parentIDStack.length === 0; + if (isRoot) { + pendingUnmountedRootID = id; + } else { + pendingUnmountedIDs.push(id); + } idToInternalInstanceMap.delete(id); } @@ -519,6 +533,8 @@ export function attach( // All unmounts are batched in a single message. // [TREE_OPERATION_REMOVE, removedIDLength, ...ids] (numUnmountIDs > 0 ? 2 + numUnmountIDs : 0) + + // [SUSPENSE_TREE_OPERATION_REMOVE, 1, pendingUnmountedRootID] + (pendingUnmountedRootID === null ? 0 : 3) + // Mount operations pendingOperations.length, ); @@ -555,6 +571,10 @@ export function attach( if (pendingUnmountedRootID !== null) { operations[i] = pendingUnmountedRootID; i++; + + operations[i++] = SUSPENSE_TREE_OPERATION_REMOVE; + operations[i++] = 1; + operations[i++] = pendingUnmountedRootID; } } diff --git a/packages/react-devtools-shared/src/devtools/store.js b/packages/react-devtools-shared/src/devtools/store.js index bb540f09daf30..79e6957aecfe4 100644 --- a/packages/react-devtools-shared/src/devtools/store.js +++ b/packages/react-devtools-shared/src/devtools/store.js @@ -895,11 +895,11 @@ export default class Store extends EventEmitter<{ if (root === null) { return []; } - if (!this.supportsTogglingSuspense(root.id)) { + if (!this.supportsTogglingSuspense(rootID)) { return []; } const list: SuspenseNode['id'][] = []; - const suspense = this.getSuspenseByID(root.id); + const suspense = this.getSuspenseByID(rootID); if (suspense !== null) { const stack = [suspense]; while (stack.length > 0) {