From 9c9136b4415f0288bcff236f488b5961dfb1dd12 Mon Sep 17 00:00:00 2001 From: "Sebastian \"Sebbie\" Silbermann" Date: Tue, 29 Jul 2025 17:23:35 +0200 Subject: [PATCH 01/12] [DevTools] Swap Components tab layout based on container size (#34035) --- .../src/devtools/views/Components/Components.css | 2 +- .../react-devtools-shared/src/devtools/views/DevTools.css | 2 ++ .../src/devtools/views/portaledContent.js | 7 ++++++- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/react-devtools-shared/src/devtools/views/Components/Components.css b/packages/react-devtools-shared/src/devtools/views/Components/Components.css index 5624f23489346..5f259cd525543 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/Components.css +++ b/packages/react-devtools-shared/src/devtools/views/Components/Components.css @@ -38,7 +38,7 @@ cursor: ew-resize; } -@media screen and (max-width: 600px) { +@container devtools (width < 600px) { .Components { flex-direction: column; } diff --git a/packages/react-devtools-shared/src/devtools/views/DevTools.css b/packages/react-devtools-shared/src/devtools/views/DevTools.css index 8091c15b8484f..0230860b9454d 100644 --- a/packages/react-devtools-shared/src/devtools/views/DevTools.css +++ b/packages/react-devtools-shared/src/devtools/views/DevTools.css @@ -5,6 +5,8 @@ flex-direction: column; background-color: var(--color-background); color: var(--color-text); + container-name: devtools; + container-type: inline-size; } .TabBar { diff --git a/packages/react-devtools-shared/src/devtools/views/portaledContent.js b/packages/react-devtools-shared/src/devtools/views/portaledContent.js index b2be6867d93b8..fb4455d0a08ea 100644 --- a/packages/react-devtools-shared/src/devtools/views/portaledContent.js +++ b/packages/react-devtools-shared/src/devtools/views/portaledContent.js @@ -36,7 +36,12 @@ export default function portaledContent(
+ style={{ + width: '100vw', + height: '100vh', + containerName: 'devtools', + containerType: 'inline-size', + }}> {children}
From b1cbb482d59d5b756294c3409fded141e809f080 Mon Sep 17 00:00:00 2001 From: "Sebastian \"Sebbie\" Silbermann" Date: Tue, 29 Jul 2025 17:45:00 +0200 Subject: [PATCH 02/12] [DevTools] More robust resize handling (#34036) --- .eslintrc.js | 1 + .../devtools/views/Components/Components.css | 12 +- .../devtools/views/Components/Components.js | 121 ++++++++---------- .../src/devtools/views/Components/Tree.css | 1 + 4 files changed, 67 insertions(+), 68 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 4e023cd9d333b..2c9ad7a4c925d 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -608,6 +608,7 @@ module.exports = { symbol: 'readonly', SyntheticEvent: 'readonly', SyntheticMouseEvent: 'readonly', + SyntheticPointerEvent: 'readonly', Thenable: 'readonly', TimeoutID: 'readonly', WheelEventHandler: 'readonly', diff --git a/packages/react-devtools-shared/src/devtools/views/Components/Components.css b/packages/react-devtools-shared/src/devtools/views/Components/Components.css index 5f259cd525543..8df59f72f1654 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/Components.css +++ b/packages/react-devtools-shared/src/devtools/views/Components/Components.css @@ -16,7 +16,6 @@ .TreeWrapper { flex: 0 0 var(--horizontal-resize-percentage); - overflow: auto; } .InspectedElementWrapper { @@ -32,7 +31,14 @@ .ResizeBar { position: absolute; - left: -2px; + /* + * moving the bar out of its bounding box might cause its hitbox to overlap + * with another scrollbar creating disorienting UX where you both resize and scroll + * at the same time. + * If you adjust this value, double check that starting resize right on this edge + * doesn't also cause scroll + */ + left: 1px; width: 5px; height: 100%; cursor: ew-resize; @@ -52,7 +58,7 @@ } .ResizeBar { - top: -2px; + top: 1px; left: 0; width: 100%; height: 5px; diff --git a/packages/react-devtools-shared/src/devtools/views/Components/Components.js b/packages/react-devtools-shared/src/devtools/views/Components/Components.js index 1d99317ba0909..b6278c2db993d 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/Components.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/Components.js @@ -29,7 +29,6 @@ type Orientation = 'horizontal' | 'vertical'; type ResizeActionType = | 'ACTION_SET_DID_MOUNT' - | 'ACTION_SET_IS_RESIZING' | 'ACTION_SET_HORIZONTAL_PERCENTAGE' | 'ACTION_SET_VERTICAL_PERCENTAGE'; @@ -40,7 +39,6 @@ type ResizeAction = { type ResizeState = { horizontalPercentage: number, - isResizing: boolean, verticalPercentage: number, }; @@ -81,82 +79,81 @@ function Components(_: {}) { return () => clearTimeout(timeoutID); }, [horizontalPercentage, verticalPercentage]); - const {isResizing} = state; + const onResizeStart = (event: SyntheticPointerEvent) => { + const element = event.currentTarget; + element.setPointerCapture(event.pointerId); + }; + + const onResizeEnd = (event: SyntheticPointerEvent) => { + const element = event.currentTarget; + element.releasePointerCapture(event.pointerId); + }; + + const onResize = (event: SyntheticPointerEvent) => { + const element = event.currentTarget; + const isResizing = element.hasPointerCapture(event.pointerId); + if (!isResizing) { + return; + } - const onResizeStart = () => - dispatch({type: 'ACTION_SET_IS_RESIZING', payload: true}); + const resizeElement = resizeElementRef.current; + const wrapperElement = wrapperElementRef.current; - let onResize; - let onResizeEnd; - if (isResizing) { - onResizeEnd = () => - dispatch({type: 'ACTION_SET_IS_RESIZING', payload: false}); + if (wrapperElement === null || resizeElement === null) { + return; + } - // $FlowFixMe[missing-local-annot] - onResize = event => { - const resizeElement = resizeElementRef.current; - const wrapperElement = wrapperElementRef.current; + event.preventDefault(); - if (!isResizing || wrapperElement === null || resizeElement === null) { - return; - } + const orientation = getOrientation(wrapperElement); - event.preventDefault(); + const {height, width, left, top} = wrapperElement.getBoundingClientRect(); - const orientation = getOrientation(wrapperElement); + const currentMousePosition = + orientation === 'horizontal' ? event.clientX - left : event.clientY - top; - const {height, width, left, top} = wrapperElement.getBoundingClientRect(); + const boundaryMin = MINIMUM_SIZE; + const boundaryMax = + orientation === 'horizontal' + ? width - MINIMUM_SIZE + : height - MINIMUM_SIZE; - const currentMousePosition = - orientation === 'horizontal' - ? event.clientX - left - : event.clientY - top; + const isMousePositionInBounds = + currentMousePosition > boundaryMin && currentMousePosition < boundaryMax; - const boundaryMin = MINIMUM_SIZE; - const boundaryMax = + if (isMousePositionInBounds) { + const resizedElementDimension = + orientation === 'horizontal' ? width : height; + const actionType = orientation === 'horizontal' - ? width - MINIMUM_SIZE - : height - MINIMUM_SIZE; - - const isMousePositionInBounds = - currentMousePosition > boundaryMin && - currentMousePosition < boundaryMax; - - if (isMousePositionInBounds) { - const resizedElementDimension = - orientation === 'horizontal' ? width : height; - const actionType = - orientation === 'horizontal' - ? 'ACTION_SET_HORIZONTAL_PERCENTAGE' - : 'ACTION_SET_VERTICAL_PERCENTAGE'; - const percentage = - (currentMousePosition / resizedElementDimension) * 100; - - setResizeCSSVariable(resizeElement, orientation, percentage); - - dispatch({ - type: actionType, - payload: currentMousePosition / resizedElementDimension, - }); - } - }; - } + ? 'ACTION_SET_HORIZONTAL_PERCENTAGE' + : 'ACTION_SET_VERTICAL_PERCENTAGE'; + const percentage = (currentMousePosition / resizedElementDimension) * 100; + + setResizeCSSVariable(resizeElement, orientation, percentage); + + dispatch({ + type: actionType, + payload: currentMousePosition / resizedElementDimension, + }); + } + }; return ( -
+
-
+
@@ -193,18 +190,12 @@ function initResizeState(): ResizeState { return { horizontalPercentage, - isResizing: false, verticalPercentage, }; } function resizeReducer(state: ResizeState, action: ResizeAction): ResizeState { switch (action.type) { - case 'ACTION_SET_IS_RESIZING': - return { - ...state, - isResizing: action.payload, - }; case 'ACTION_SET_HORIZONTAL_PERCENTAGE': return { ...state, diff --git a/packages/react-devtools-shared/src/devtools/views/Components/Tree.css b/packages/react-devtools-shared/src/devtools/views/Components/Tree.css index a65cda45aac9f..cb2799d4a9c3f 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/Tree.css +++ b/packages/react-devtools-shared/src/devtools/views/Components/Tree.css @@ -38,6 +38,7 @@ font-family: var(--font-family-monospace); font-size: var(--font-size-monospace-normal); line-height: var(--line-height-data); + user-select: none; } .VRule { From 9be531cd37f5558c72f7de360eb921b0074e8544 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 29 Jul 2025 11:50:12 -0400 Subject: [PATCH 03/12] [Fiber] Treat unwrapping React.lazy more like a use() (#34031) While we want to get rid of React.lazy's special wrapper type and just use a Promise for the type, we still have the wrapper. However, this is still conceptually the same as a Usable in that it should be have the same if you `use(promise)` or render a Promise as a child or type position. This PR makes it behave like a `use()` when we unwrap them. We could move to a model where it actually reaches the internal of the Lazy's Promise when it unwraps but for now I leave the lazy API signature intact by just catching the Promise and then "use()" that. This lets us align on the semantics with `use()` such as the suspense yield optimization. It also lets us warn or fork based on legacy throw-a-Promise behavior where as `React.lazy` is not deprecated. --- .../react-reconciler/src/ReactChildFiber.js | 56 ++----------------- .../src/ReactFiberBeginWork.js | 16 +----- .../src/ReactFiberThenable.js | 25 +++++++++ .../src/__tests__/ReactLazy-test.internal.js | 17 +----- 4 files changed, 37 insertions(+), 77 deletions(-) diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js index f574162b41b0c..3bde0d6db9ac3 100644 --- a/packages/react-reconciler/src/ReactChildFiber.js +++ b/packages/react-reconciler/src/ReactChildFiber.js @@ -68,9 +68,9 @@ import { SuspenseActionException, createThenableState, trackUsedThenable, + resolveLazy, } from './ReactFiberThenable'; import {readContextDuringReconciliation} from './ReactFiberNewContext'; -import {callLazyInitInDEV} from './ReactFiberCallUserSpace'; import {runWithFiberInDEV} from './ReactCurrentFiber'; @@ -364,15 +364,6 @@ function warnOnSymbolType(returnFiber: Fiber, invalidChild: symbol) { } } -function resolveLazy(lazyType: any) { - if (__DEV__) { - return callLazyInitInDEV(lazyType); - } - const payload = lazyType._payload; - const init = lazyType._init; - return init(payload); -} - type ChildReconciler = ( returnFiber: Fiber, currentFirstChild: Fiber | null, @@ -698,14 +689,7 @@ function createChildReconciler( } case REACT_LAZY_TYPE: { const prevDebugInfo = pushDebugInfo(newChild._debugInfo); - let resolvedChild; - if (__DEV__) { - resolvedChild = callLazyInitInDEV(newChild); - } else { - const payload = newChild._payload; - const init = newChild._init; - resolvedChild = init(payload); - } + const resolvedChild = resolveLazy((newChild: any)); const created = createChild(returnFiber, resolvedChild, lanes); currentDebugInfo = prevDebugInfo; return created; @@ -830,14 +814,7 @@ function createChildReconciler( } case REACT_LAZY_TYPE: { const prevDebugInfo = pushDebugInfo(newChild._debugInfo); - let resolvedChild; - if (__DEV__) { - resolvedChild = callLazyInitInDEV(newChild); - } else { - const payload = newChild._payload; - const init = newChild._init; - resolvedChild = init(payload); - } + const resolvedChild = resolveLazy((newChild: any)); const updated = updateSlot( returnFiber, oldFiber, @@ -962,14 +939,7 @@ function createChildReconciler( } case REACT_LAZY_TYPE: { const prevDebugInfo = pushDebugInfo(newChild._debugInfo); - let resolvedChild; - if (__DEV__) { - resolvedChild = callLazyInitInDEV(newChild); - } else { - const payload = newChild._payload; - const init = newChild._init; - resolvedChild = init(payload); - } + const resolvedChild = resolveLazy((newChild: any)); const updated = updateFromMap( existingChildren, returnFiber, @@ -1086,14 +1056,7 @@ function createChildReconciler( }); break; case REACT_LAZY_TYPE: { - let resolvedChild; - if (__DEV__) { - resolvedChild = callLazyInitInDEV((child: any)); - } else { - const payload = child._payload; - const init = (child._init: any); - resolvedChild = init(payload); - } + const resolvedChild = resolveLazy((child: any)); warnOnInvalidKey( returnFiber, workInProgress, @@ -1809,14 +1772,7 @@ function createChildReconciler( ); case REACT_LAZY_TYPE: { const prevDebugInfo = pushDebugInfo(newChild._debugInfo); - let result; - if (__DEV__) { - result = callLazyInitInDEV(newChild); - } else { - const payload = newChild._payload; - const init = newChild._init; - result = init(payload); - } + const result = resolveLazy((newChild: any)); const firstChild = reconcileChildFibersImpl( returnFiber, currentFirstChild, diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 372bab95cea57..33b9e79edfaf3 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -302,11 +302,8 @@ import { pushRootMarkerInstance, TransitionTracingMarker, } from './ReactFiberTracingMarkerComponent'; -import { - callLazyInitInDEV, - callComponentInDEV, - callRenderInDEV, -} from './ReactFiberCallUserSpace'; +import {callComponentInDEV, callRenderInDEV} from './ReactFiberCallUserSpace'; +import {resolveLazy} from './ReactFiberThenable'; // A special exception that's used to unwind the stack when an update flows // into a dehydrated boundary. @@ -2020,14 +2017,7 @@ function mountLazyComponent( const props = workInProgress.pendingProps; const lazyComponent: LazyComponentType = elementType; - let Component; - if (__DEV__) { - Component = callLazyInitInDEV(lazyComponent); - } else { - const payload = lazyComponent._payload; - const init = lazyComponent._init; - Component = init(payload); - } + let Component = resolveLazy(lazyComponent); // Store the unwrapped component in the type. workInProgress.type = Component; diff --git a/packages/react-reconciler/src/ReactFiberThenable.js b/packages/react-reconciler/src/ReactFiberThenable.js index 4f52131a649cd..f4ae1d45b271e 100644 --- a/packages/react-reconciler/src/ReactFiberThenable.js +++ b/packages/react-reconciler/src/ReactFiberThenable.js @@ -14,6 +14,10 @@ import type { RejectedThenable, } from 'shared/ReactTypes'; +import type {LazyComponent as LazyComponentType} from 'react/src/ReactLazy'; + +import {callLazyInitInDEV} from './ReactFiberCallUserSpace'; + import {getWorkInProgressRoot} from './ReactFiberWorkLoop'; import ReactSharedInternals from 'shared/ReactSharedInternals'; @@ -260,6 +264,27 @@ export function suspendCommit(): void { throw SuspenseyCommitException; } +export function resolveLazy(lazyType: LazyComponentType): T { + try { + if (__DEV__) { + return callLazyInitInDEV(lazyType); + } + const payload = lazyType._payload; + const init = lazyType._init; + return init(payload); + } catch (x) { + if (x !== null && typeof x === 'object' && typeof x.then === 'function') { + // This lazy Suspended. Treat this as if we called use() to unwrap it. + suspendedThenable = x; + if (__DEV__) { + needsToResetSuspendedThenableDEV = true; + } + throw SuspenseException; + } + throw x; + } +} + // This is used to track the actual thenable that suspended so it can be // passed to the rest of the Suspense implementation — which, for historical // reasons, expects to receive a thenable. diff --git a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js index 27404eee5e2c8..e594e4fbedb28 100644 --- a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js @@ -198,10 +198,7 @@ describe('ReactLazy', () => { await resolveFakeImport(Foo); - await waitForAll([ - 'Foo', - ...(gate('alwaysThrottleRetries') ? [] : ['Foo']), - ]); + await waitForAll(['Foo']); expect(root).not.toMatchRenderedOutput('FooBar'); await act(() => resolveFakeImport(Bar)); @@ -1329,11 +1326,7 @@ describe('ReactLazy', () => { expect(ref.current).toBe(null); await act(() => resolveFakeImport(Foo)); - assertLog([ - 'Foo', - // pre-warming - 'Foo', - ]); + assertLog(['Foo']); await act(() => resolveFakeImport(ForwardRefBar)); assertLog(['Foo', 'forwardRef', 'Bar']); @@ -1493,11 +1486,7 @@ describe('ReactLazy', () => { expect(root).not.toMatchRenderedOutput('AB'); await act(() => resolveFakeImport(ChildA)); - assertLog([ - 'A', - // pre-warming - 'A', - ]); + assertLog(['A']); await act(() => resolveFakeImport(ChildB)); assertLog(['A', 'B', 'Did mount: A', 'Did mount: B']); From 820af2097103309fdc5675d2bde744103a439eff Mon Sep 17 00:00:00 2001 From: lauren Date: Tue, 29 Jul 2025 12:33:42 -0400 Subject: [PATCH 04/12] [eslint] Disallow use within try/catch blocks (#34040) Follow up to #34032. The linter now ensures that `use` cannot be used within try/catch. --- .../__tests__/ESLintRulesOfHooks-test.js | 38 ++++++++++++++++++- .../src/rules/RulesOfHooks.ts | 30 ++++++++++++++- 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js index 1690f20ede490..7cb3ef0495341 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js @@ -1324,6 +1324,34 @@ const allTests = { `, errors: [asyncComponentHookError('use')], }, + { + code: normalizeIndent` + function App({p1, p2}) { + try { + use(p1); + } catch (error) { + console.error(error); + } + use(p2); + return
App
; + } + `, + errors: [tryCatchUseError('use')], + }, + { + code: normalizeIndent` + function App({p1, p2}) { + try { + doSomething(); + } catch { + use(p1); + } + use(p2); + return
App
; + } + `, + errors: [tryCatchUseError('use')], + }, ], }; @@ -1383,7 +1411,7 @@ if (__EXPERIMENTAL__) { const onEvent = useEffectEvent((text) => { console.log(text); }); - + useEffect(() => { onEvent('Hello world'); }); @@ -1421,7 +1449,7 @@ if (__EXPERIMENTAL__) { }); return onClick()} /> } - + // The useEffectEvent function shares an identifier name with the above function MyLastComponent({theme}) { const onClick = useEffectEvent(() => { @@ -1573,6 +1601,12 @@ function asyncComponentHookError(fn) { }; } +function tryCatchUseError(fn) { + return { + message: `React Hook "${fn}" cannot be called in a try/catch block.`, + }; +} + // For easier local testing if (!process.env.CI) { let only = []; diff --git a/packages/eslint-plugin-react-hooks/src/rules/RulesOfHooks.ts b/packages/eslint-plugin-react-hooks/src/rules/RulesOfHooks.ts index 8d42b319b4976..f0a2ffbda9e18 100644 --- a/packages/eslint-plugin-react-hooks/src/rules/RulesOfHooks.ts +++ b/packages/eslint-plugin-react-hooks/src/rules/RulesOfHooks.ts @@ -7,7 +7,13 @@ /* eslint-disable no-for-of-loops/no-for-of-loops */ import type {Rule, Scope} from 'eslint'; -import type {CallExpression, DoWhileStatement, Node} from 'estree'; +import type { + CallExpression, + CatchClause, + DoWhileStatement, + Node, + TryStatement, +} from 'estree'; // @ts-expect-error untyped module import CodePathAnalyzer from '../code-path-analysis/code-path-analyzer'; @@ -111,6 +117,18 @@ function isInsideDoWhileLoop(node: Node | undefined): node is DoWhileStatement { return false; } +function isInsideTryCatch( + node: Node | undefined, +): node is TryStatement | CatchClause { + while (node) { + if (node.type === 'TryStatement' || node.type === 'CatchClause') { + return true; + } + node = node.parent; + } + return false; +} + function isUseEffectEventIdentifier(node: Node): boolean { if (__EXPERIMENTAL__) { return node.type === 'Identifier' && node.name === 'useEffectEvent'; @@ -532,6 +550,16 @@ const rule = { continue; } + // Report an error if use() is called inside try/catch. + if (isUseIdentifier(hook) && isInsideTryCatch(hook)) { + context.report({ + node: hook, + message: `React Hook "${getSourceCode().getText( + hook, + )}" cannot be called in a try/catch block.`, + }); + } + // Report an error if a hook may be called more then once. // `use(...)` can be called in loops. if ( From 85bbe39ef8e24a192b5e9f2987b1babf8ce772e1 Mon Sep 17 00:00:00 2001 From: Joseph Savona <6425824+josephsavona@users.noreply.github.com> Date: Tue, 29 Jul 2025 09:57:48 -0700 Subject: [PATCH 05/12] [compiler] Fixes to enableTreatRefLikeIdentifiersAsRefs (#34000) We added the `@enableTreatRefLikeIdentifiersAsRefs` feature a while back but never enabled it. Since then we've continued to see examples that motivate this mode, so here we're fixing it up to prepare to enable by default. It now works as follows: * If we find a property load or property store where both a) the object's name is ref-like (`ref` or `-Ref`) and b) the property is `current`, we infer the object itself as a ref and the value of the property as a ref value. Originally the feature only detected property loads, not stores. * Inferred refs are not considered stable (this is a change from the original implementation). The only way to get a stable ref is by calling `useRef()`. We've seen issues with assuming refs are stable. With this change, cases like the following now correctly error: ```js function Foo(props) { const fooRef = props.fooRef; fooRef.current = true; ^^^^^^^^^^^^^^ cannot modify ref in render } ``` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34000). * #34027 * #34026 * #34025 * #34024 * #34005 * #34006 * #34004 * #34003 * __->__ #34000 --- .../src/HIR/ObjectShape.ts | 2 ++ .../src/Inference/InferReactivePlaces.ts | 8 ----- .../src/TypeInference/InferTypes.ts | 31 ++++++++++++++++- ...rrent-inferred-ref-during-render.expect.md | 34 +++++++++++++++++++ ...sign-current-inferred-ref-during-render.js | 9 +++++ .../compiler/reanimated-no-memo-arg.expect.md | 4 +-- .../compiler/reanimated-no-memo-arg.js | 2 +- .../ref-like-name-in-effect.expect.md | 24 +++++++------ .../ref-like-name-in-useCallback-2.expect.md | 18 +++++----- .../ref-like-name-in-useCallback-2.js | 2 +- .../ref-like-name-in-useCallback.expect.md | 18 +++++----- .../compiler/ref-like-name-in-useCallback.js | 2 +- 12 files changed, 114 insertions(+), 40 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assign-current-inferred-ref-during-render.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assign-current-inferred-ref-during-render.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts index baaf40d67e7bb..eaf728db95119 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts @@ -1211,6 +1211,8 @@ addObject(BUILTIN_SHAPES, BuiltInRefValueId, [ ['*', {kind: 'Object', shapeId: BuiltInRefValueId}], ]); +addObject(BUILTIN_SHAPES, ReanimatedSharedValueId, []); + addFunction( BUILTIN_SHAPES, [], diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts index 88faccd8cf3b6..19e220b235694 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts @@ -21,7 +21,6 @@ import { isStableType, isStableTypeContainer, isUseOperator, - isUseRefType, } from '../HIR'; import {PostDominator} from '../HIR/Dominator'; import { @@ -70,13 +69,6 @@ class StableSidemap { isStable: false, }); } - } else if ( - this.env.config.enableTreatRefLikeIdentifiersAsRefs && - isUseRefType(lvalue.identifier) - ) { - this.map.set(lvalue.identifier.id, { - isStable: true, - }); } break; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts b/compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts index 596244b3834c9..73088fd852194 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts @@ -466,7 +466,36 @@ function* generateInstructionTypes( yield equation(left, returnType); break; } - case 'PropertyStore': + case 'PropertyStore': { + /** + * Infer types based on assignments to known object properties + * This is important for refs, where assignment to `.current` + * can help us infer that an object itself is a ref + */ + yield equation( + /** + * Our property type declarations are best-effort and we haven't tested + * using them to drive inference of rvalues from lvalues. We want to emit + * a Property type in order to infer refs from `.current` accesses, but + * stay conservative by not otherwise inferring anything about rvalues. + * So we use a dummy type here. + * + * TODO: consider using the rvalue type here + */ + makeType(), + // unify() only handles properties in the second position + { + kind: 'Property', + objectType: value.object.identifier.type, + objectName: getName(names, value.object.identifier.id), + propertyName: { + kind: 'literal', + value: value.property, + }, + }, + ); + break; + } case 'DeclareLocal': case 'RegExpLiteral': case 'MetaProperty': diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assign-current-inferred-ref-during-render.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assign-current-inferred-ref-during-render.expect.md new file mode 100644 index 0000000000000..9c12d955ae430 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assign-current-inferred-ref-during-render.expect.md @@ -0,0 +1,34 @@ + +## Input + +```javascript +// @flow @enableTreatRefLikeIdentifiersAsRefs @validateRefAccessDuringRender +import {makeObject_Primitives} from 'shared-runtime'; + +component Example() { + const fooRef = makeObject_Primitives(); + fooRef.current = true; + + return ; +} + +``` + + +## Error + +``` +Found 1 error: + +Error: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) + + 4 | component Example() { + 5 | const fooRef = makeObject_Primitives(); +> 6 | fooRef.current = true; + | ^^^^^^^^^^^^^^ Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) + 7 | + 8 | return ; + 9 | } +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assign-current-inferred-ref-during-render.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assign-current-inferred-ref-during-render.js new file mode 100644 index 0000000000000..39df293ba6258 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assign-current-inferred-ref-during-render.js @@ -0,0 +1,9 @@ +// @flow @enableTreatRefLikeIdentifiersAsRefs @validateRefAccessDuringRender +import {makeObject_Primitives} from 'shared-runtime'; + +component Example() { + const fooRef = makeObject_Primitives(); + fooRef.current = true; + + return ; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reanimated-no-memo-arg.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reanimated-no-memo-arg.expect.md index 5ebc821375187..1f5fb54eaeb76 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reanimated-no-memo-arg.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reanimated-no-memo-arg.expect.md @@ -3,7 +3,7 @@ ```javascript // @enableCustomTypeDefinitionForReanimated -import {useAnimatedProps} from 'react-native-reanimated'; +import {useAnimatedProps, useSharedValue} from 'react-native-reanimated'; function Component() { const radius = useSharedValue(50); @@ -39,7 +39,7 @@ export const FIXTURE_ENTRYPOINT = { ```javascript import { c as _c } from "react/compiler-runtime"; // @enableCustomTypeDefinitionForReanimated -import { useAnimatedProps } from "react-native-reanimated"; +import { useAnimatedProps, useSharedValue } from "react-native-reanimated"; function Component() { const $ = _c(2); const radius = useSharedValue(50); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reanimated-no-memo-arg.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reanimated-no-memo-arg.js index ad87d08cdc49a..d2865ce99ae6c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reanimated-no-memo-arg.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reanimated-no-memo-arg.js @@ -1,5 +1,5 @@ // @enableCustomTypeDefinitionForReanimated -import {useAnimatedProps} from 'react-native-reanimated'; +import {useAnimatedProps, useSharedValue} from 'react-native-reanimated'; function Component() { const radius = useSharedValue(50); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-like-name-in-effect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-like-name-in-effect.expect.md index 156c723360268..a750afe6fe878 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-like-name-in-effect.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-like-name-in-effect.expect.md @@ -47,28 +47,32 @@ function useCustomRef() { function _temp() {} function Foo() { - const $ = _c(3); + const $ = _c(4); const ref = useCustomRef(); let t0; - let t1; - if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + if ($[0] !== ref) { t0 = () => { ref.current?.click(); }; + $[0] = ref; + $[1] = t0; + } else { + t0 = $[1]; + } + let t1; + if ($[2] === Symbol.for("react.memo_cache_sentinel")) { t1 = []; - $[0] = t0; - $[1] = t1; + $[2] = t1; } else { - t0 = $[0]; - t1 = $[1]; + t1 = $[2]; } useEffect(t0, t1); let t2; - if ($[2] === Symbol.for("react.memo_cache_sentinel")) { + if ($[3] === Symbol.for("react.memo_cache_sentinel")) { t2 =
foo
; - $[2] = t2; + $[3] = t2; } else { - t2 = $[2]; + t2 = $[3]; } return t2; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-like-name-in-useCallback-2.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-like-name-in-useCallback-2.expect.md index c5f2f6a762850..f74962e0d65a3 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-like-name-in-useCallback-2.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-like-name-in-useCallback-2.expect.md @@ -14,7 +14,7 @@ function Foo() { const onClick = useCallback(() => { ref.current?.click(); - }, []); + }, [ref]); return