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/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; +} 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: [{}], +}; 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/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/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) { 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 ; + })} +
+
); }