From d42f4355f867581d461e0f4dff7828de1da0c454 Mon Sep 17 00:00:00 2001 From: kolvian Date: Thu, 6 Nov 2025 12:23:21 -0700 Subject: [PATCH 1/2] [compiler] Allow ref access in callbacks passed to DOM event handlers Fixes a false positive where the compiler incorrectly flags ref access in callbacks passed to event handler wrappers on built-in DOM elements. For example, with react-hook-form:
Where onSubmit accesses ref.current, this was incorrectly flagged as "Cannot access refs during render". This is a false positive because built-in DOM event handlers are guaranteed by React to only execute in response to actual events, never during render. This fix only relaxes validation for built-in DOM elements (not custom components) to maintain safety. Custom components could call their onFoo props during render, which would violate ref access rules. Adds four test fixtures demonstrating allowed and disallowed patterns. Issue: #35040 --- .../Validation/ValidateNoRefAccessInRender.ts | 44 ++++++ ...s-in-async-event-handler-wrapper.expect.md | 148 ++++++++++++++++++ ...-access-in-async-event-handler-wrapper.tsx | 48 ++++++ ...-access-in-event-handler-wrapper.expect.md | 101 ++++++++++++ ...ow-ref-access-in-event-handler-wrapper.tsx | 36 +++++ ...-component-event-handler-wrapper.expect.md | 69 ++++++++ ...custom-component-event-handler-wrapper.tsx | 41 +++++ ...f-value-in-event-handler-wrapper.expect.md | 55 +++++++ ...ror.ref-value-in-event-handler-wrapper.tsx | 27 ++++ .../__tests__/ReactHooksInspection-test.js | 4 + 10 files changed, 573 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-async-event-handler-wrapper.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-async-event-handler-wrapper.tsx create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-event-handler-wrapper.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-event-handler-wrapper.tsx create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-custom-component-event-handler-wrapper.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-custom-component-event-handler-wrapper.tsx create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-event-handler-wrapper.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-event-handler-wrapper.tsx diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccessInRender.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccessInRender.ts index abbb7d847698..b0961f5fd7e9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccessInRender.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccessInRender.ts @@ -354,6 +354,7 @@ function validateNoRefAccessInRenderImpl( } const interpolatedAsJsx = new Set(); + const usedAsEventHandlerProp = new Set(); for (const block of fn.body.blocks.values()) { for (const instr of block.instructions) { const {value} = instr; @@ -363,6 +364,27 @@ function validateNoRefAccessInRenderImpl( interpolatedAsJsx.add(child.identifier.id); } } + if (value.kind === 'JsxExpression') { + /* + * Track identifiers used as event handler props on built-in DOM elements. + * We only allow this optimization for native DOM elements because their + * event handlers are guaranteed by React to only execute in response to + * events, never during render. Custom components could technically call + * their onFoo props during render, which would violate ref access rules. + */ + if (value.tag.kind === 'BuiltinTag') { + for (const prop of value.props) { + if ( + prop.kind === 'JsxAttribute' && + prop.name.startsWith('on') && + prop.name.length > 2 && + prop.name[2] === prop.name[2].toUpperCase() + ) { + usedAsEventHandlerProp.add(prop.place.identifier.id); + } + } + } + } } } } @@ -519,6 +541,9 @@ function validateNoRefAccessInRenderImpl( */ if (!didError) { const isRefLValue = isUseRefType(instr.lvalue.identifier); + const isUsedAsEventHandler = usedAsEventHandlerProp.has( + instr.lvalue.identifier.id, + ); for (const operand of eachInstructionValueOperand(instr.value)) { /** * By default we check that function call operands are not refs, @@ -560,6 +585,25 @@ function validateNoRefAccessInRenderImpl( * render function which attempts to obey the rules. */ validateNoRefValueAccess(errors, env, operand); + } else if (isUsedAsEventHandler) { + /** + * Special case: the lvalue is used as an event handler prop + * on a built-in DOM element + * + * For example ``. Here + * handleSubmit is wrapping onSubmit to create an event handler. + * Functions passed to event handler wrappers can safely access + * refs because built-in DOM event handlers are guaranteed by React + * to only execute in response to actual events, never during render. + * + * We only allow this for built-in DOM elements (not custom components) + * because custom components could technically call their onFoo props + * during render, which would violate ref access rules. + * + * We allow passing functions with ref access to these wrappers, + * but still validate that direct ref values aren't passed. + */ + validateNoDirectRefValueAccess(errors, operand, env); } else { validateNoRefPassedToFunction( errors, diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-async-event-handler-wrapper.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-async-event-handler-wrapper.expect.md new file mode 100644 index 000000000000..c3d65a42e951 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-async-event-handler-wrapper.expect.md @@ -0,0 +1,148 @@ + +## Input + +```javascript +// @validateRefAccessDuringRender +import {useRef} from 'react'; + +// Simulates react-hook-form's handleSubmit +function handleSubmit(callback: (data: T) => void | Promise) { + return (event: any) => { + event.preventDefault(); + callback({} as T); + }; +} + +// Simulates an upload function +async function upload(file: any): Promise<{blob: {url: string}}> { + return {blob: {url: 'https://example.com/file.jpg'}}; +} + +interface SignatureRef { + toFile(): any; +} + +function Component() { + const ref = useRef(null); + + const onSubmit = async (value: any) => { + // This should be allowed: accessing ref.current in an async event handler + // that's wrapped and passed to onSubmit prop + let sigUrl: string; + if (value.hasSignature) { + const {blob} = await upload(ref.current?.toFile()); + sigUrl = blob?.url || ''; + } else { + sigUrl = value.signature; + } + console.log('Signature URL:', sigUrl); + }; + + return ( + + + + + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateRefAccessDuringRender +import { useRef } from "react"; + +// Simulates react-hook-form's handleSubmit +function handleSubmit(callback) { + const $ = _c(2); + let t0; + if ($[0] !== callback) { + t0 = (event) => { + event.preventDefault(); + callback({} as T); + }; + $[0] = callback; + $[1] = t0; + } else { + t0 = $[1]; + } + return t0; +} + +// Simulates an upload function +async function upload(file) { + const $ = _c(1); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = { blob: { url: "https://example.com/file.jpg" } }; + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} + +interface SignatureRef { + toFile(): any; +} + +function Component() { + const $ = _c(4); + const ref = useRef(null); + + const onSubmit = async (value) => { + let sigUrl; + if (value.hasSignature) { + const { blob } = await upload(ref.current?.toFile()); + sigUrl = blob?.url || ""; + } else { + sigUrl = value.signature; + } + + console.log("Signature URL:", sigUrl); + }; + + const t0 = handleSubmit(onSubmit); + let t1; + let t2; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t1 = ; + t2 = ; + $[0] = t1; + $[1] = t2; + } else { + t1 = $[0]; + t2 = $[1]; + } + let t3; + if ($[2] !== t0) { + t3 = ( +
+ {t1} + {t2} +
+ ); + $[2] = t0; + $[3] = t3; + } else { + t3 = $[3]; + } + return t3; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + +### Eval output +(kind: ok)
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-async-event-handler-wrapper.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-async-event-handler-wrapper.tsx new file mode 100644 index 000000000000..dfb1f84961c8 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-async-event-handler-wrapper.tsx @@ -0,0 +1,48 @@ +// @validateRefAccessDuringRender +import {useRef} from 'react'; + +// Simulates react-hook-form's handleSubmit +function handleSubmit(callback: (data: T) => void | Promise) { + return (event: any) => { + event.preventDefault(); + callback({} as T); + }; +} + +// Simulates an upload function +async function upload(file: any): Promise<{blob: {url: string}}> { + return {blob: {url: 'https://example.com/file.jpg'}}; +} + +interface SignatureRef { + toFile(): any; +} + +function Component() { + const ref = useRef(null); + + const onSubmit = async (value: any) => { + // This should be allowed: accessing ref.current in an async event handler + // that's wrapped and passed to onSubmit prop + let sigUrl: string; + if (value.hasSignature) { + const {blob} = await upload(ref.current?.toFile()); + sigUrl = blob?.url || ''; + } else { + sigUrl = value.signature; + } + console.log('Signature URL:', sigUrl); + }; + + return ( +
+ + +
+ ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-event-handler-wrapper.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-event-handler-wrapper.expect.md new file mode 100644 index 000000000000..805b31e92bf8 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-event-handler-wrapper.expect.md @@ -0,0 +1,101 @@ + +## Input + +```javascript +// @validateRefAccessDuringRender +import {useRef} from 'react'; + +// Simulates react-hook-form's handleSubmit or similar event handler wrappers +function handleSubmit(callback: (data: T) => void) { + return (event: any) => { + event.preventDefault(); + callback({} as T); + }; +} + +function Component() { + const ref = useRef(null); + + const onSubmit = (data: any) => { + // This should be allowed: accessing ref.current in an event handler + // that's wrapped by handleSubmit and passed to onSubmit prop + if (ref.current !== null) { + console.log(ref.current.value); + } + }; + + return ( + <> + +
+ +
+ + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateRefAccessDuringRender +import { useRef } from "react"; + +// Simulates react-hook-form's handleSubmit or similar event handler wrappers +function handleSubmit(callback) { + const $ = _c(2); + let t0; + if ($[0] !== callback) { + t0 = (event) => { + event.preventDefault(); + callback({} as T); + }; + $[0] = callback; + $[1] = t0; + } else { + t0 = $[1]; + } + return t0; +} + +function Component() { + const $ = _c(1); + const ref = useRef(null); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + const onSubmit = (data) => { + if (ref.current !== null) { + console.log(ref.current.value); + } + }; + + t0 = ( + <> + +
+ +
+ + ); + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + +### Eval output +(kind: ok)
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-event-handler-wrapper.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-event-handler-wrapper.tsx new file mode 100644 index 000000000000..59ee12e5c5a7 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-event-handler-wrapper.tsx @@ -0,0 +1,36 @@ +// @validateRefAccessDuringRender +import {useRef} from 'react'; + +// Simulates react-hook-form's handleSubmit or similar event handler wrappers +function handleSubmit(callback: (data: T) => void) { + return (event: any) => { + event.preventDefault(); + callback({} as T); + }; +} + +function Component() { + const ref = useRef(null); + + const onSubmit = (data: any) => { + // This should be allowed: accessing ref.current in an event handler + // that's wrapped by handleSubmit and passed to onSubmit prop + if (ref.current !== null) { + console.log(ref.current.value); + } + }; + + return ( + <> + +
+ +
+ + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-custom-component-event-handler-wrapper.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-custom-component-event-handler-wrapper.expect.md new file mode 100644 index 000000000000..97f6732f279e --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-custom-component-event-handler-wrapper.expect.md @@ -0,0 +1,69 @@ + +## Input + +```javascript +// @validateRefAccessDuringRender +import {useRef} from 'react'; + +// Simulates a custom component wrapper +function CustomForm({onSubmit, children}: any) { + return
{children}
; +} + +// Simulates react-hook-form's handleSubmit +function handleSubmit(callback: (data: T) => void) { + return (event: any) => { + event.preventDefault(); + callback({} as T); + }; +} + +function Component() { + const ref = useRef(null); + + const onSubmit = (data: any) => { + // This should error: passing function with ref access to custom component + // event handler, even though it would be safe on a native
+ if (ref.current !== null) { + console.log(ref.current.value); + } + }; + + return ( + <> + + + + + + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + + +## Error + +``` +Found 1 error: + +Error: Cannot access refs during render + +React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef). + +error.ref-value-in-custom-component-event-handler-wrapper.ts:31:41 + 29 | <> + 30 | +> 31 | + | ^^^^^^^^ Passing a ref to a function may read its value during render + 32 | + 33 | + 34 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-custom-component-event-handler-wrapper.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-custom-component-event-handler-wrapper.tsx new file mode 100644 index 000000000000..467486ff7b3a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-custom-component-event-handler-wrapper.tsx @@ -0,0 +1,41 @@ +// @validateRefAccessDuringRender +import {useRef} from 'react'; + +// Simulates a custom component wrapper +function CustomForm({onSubmit, children}: any) { + return {children}
; +} + +// Simulates react-hook-form's handleSubmit +function handleSubmit(callback: (data: T) => void) { + return (event: any) => { + event.preventDefault(); + callback({} as T); + }; +} + +function Component() { + const ref = useRef(null); + + const onSubmit = (data: any) => { + // This should error: passing function with ref access to custom component + // event handler, even though it would be safe on a native
+ if (ref.current !== null) { + console.log(ref.current.value); + } + }; + + return ( + <> + + + + + + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-event-handler-wrapper.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-event-handler-wrapper.expect.md new file mode 100644 index 000000000000..41586dd744aa --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-event-handler-wrapper.expect.md @@ -0,0 +1,55 @@ + +## Input + +```javascript +// @validateRefAccessDuringRender +import {useRef} from 'react'; + +// Simulates a handler wrapper +function handleClick(value: any) { + return () => { + console.log(value); + }; +} + +function Component() { + const ref = useRef(null); + + // This should still error: passing ref.current directly to a wrapper + // The ref value is accessed during render, not in the event handler + return ( + <> + + + + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + + +## Error + +``` +Found 1 error: + +Error: Cannot access refs during render + +React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef). + +error.ref-value-in-event-handler-wrapper.ts:19:35 + 17 | <> + 18 | +> 19 | + | ^^^^^^^^^^^ Cannot access ref value during render + 20 | + 21 | ); + 22 | } +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-event-handler-wrapper.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-event-handler-wrapper.tsx new file mode 100644 index 000000000000..5002e910aceb --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-event-handler-wrapper.tsx @@ -0,0 +1,27 @@ +// @validateRefAccessDuringRender +import {useRef} from 'react'; + +// Simulates a handler wrapper +function handleClick(value: any) { + return () => { + console.log(value); + }; +} + +function Component() { + const ref = useRef(null); + + // This should still error: passing ref.current directly to a wrapper + // The ref value is accessed during render, not in the event handler + return ( + <> + + + + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; diff --git a/packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.js b/packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.js index 18dab0710f25..c5024bf5d676 100644 --- a/packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.js +++ b/packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.js @@ -20,6 +20,10 @@ function normalizeSourceLoc(tree) { node.hookSource.lineNumber = 0; node.hookSource.columnNumber = 0; } + // Normalize Promise objects to hide internal symbols + if (node.value instanceof Promise) { + node.value = Object.create(Promise.prototype); + } normalizeSourceLoc(node.subHooks); }); return tree; From 42f3da55f9b60956a62458c610e67327a12fe6e9 Mon Sep 17 00:00:00 2001 From: kolvian Date: Fri, 7 Nov 2025 17:31:39 -0700 Subject: [PATCH 2/2] [compiler] Refactor event handler validation to use type inference --- .../src/HIR/Environment.ts | 9 +++ .../src/HIR/Globals.ts | 4 +- .../src/HIR/ObjectShape.ts | 18 ++++- .../src/TypeInference/InferTypes.ts | 36 +++++++++ .../Validation/ValidateNoRefAccessInRender.ts | 75 ++++--------------- ...s-in-async-event-handler-wrapper.expect.md | 4 +- ...-access-in-async-event-handler-wrapper.tsx | 2 +- ...-access-in-event-handler-wrapper.expect.md | 4 +- ...ow-ref-access-in-event-handler-wrapper.tsx | 2 +- ...-component-event-handler-wrapper.expect.md | 2 +- ...custom-component-event-handler-wrapper.tsx | 2 +- ...f-value-in-event-handler-wrapper.expect.md | 2 +- ...ror.ref-value-in-event-handler-wrapper.tsx | 2 +- .../__tests__/ReactHooksInspection-test.js | 4 - 14 files changed, 87 insertions(+), 79 deletions(-) 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 efee8e080e3b..9a22d4ace6af 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -670,6 +670,15 @@ export const EnvironmentConfigSchema = z.object({ * from refs need to be stored in state during mount. */ enableAllowSetStateFromRefsInEffects: z.boolean().default(true), + + /** + * Enables inference of event handler types for JSX props on built-in DOM elements. + * When enabled, functions passed to event handler props (props starting with "on") + * on primitive JSX tags are inferred to have the BuiltinEventHandlerId type, which + * allows ref access within those functions since DOM event handlers are guaranteed + * by React to only execute in response to events, not during render. + */ + enableInferEventHandlers: z.boolean().default(false), }); export type EnvironmentConfig = z.infer; diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts index 561bdab6982d..f42d6fc5bd94 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts @@ -29,7 +29,7 @@ import { BuiltInUseTransitionId, BuiltInWeakMapId, BuiltInWeakSetId, - BuiltinEffectEventId, + BuiltInEffectEventId, ReanimatedSharedValueId, ShapeRegistry, addFunction, @@ -863,7 +863,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [ returnType: { kind: 'Function', return: {kind: 'Poly'}, - shapeId: BuiltinEffectEventId, + shapeId: BuiltInEffectEventId, isConstructor: false, }, calleeEffect: Effect.Read, 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 beaff321e267..eb771615619c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts @@ -403,8 +403,9 @@ export const BuiltInStartTransitionId = 'BuiltInStartTransition'; export const BuiltInFireId = 'BuiltInFire'; export const BuiltInFireFunctionId = 'BuiltInFireFunction'; export const BuiltInUseEffectEventId = 'BuiltInUseEffectEvent'; -export const BuiltinEffectEventId = 'BuiltInEffectEventFunction'; +export const BuiltInEffectEventId = 'BuiltInEffectEventFunction'; export const BuiltInAutodepsId = 'BuiltInAutoDepsId'; +export const BuiltInEventHandlerId = 'BuiltInEventHandlerId'; // See getReanimatedModuleType() in Globals.ts — this is part of supporting Reanimated's ref-like types export const ReanimatedSharedValueId = 'ReanimatedSharedValueId'; @@ -1243,7 +1244,20 @@ addFunction( calleeEffect: Effect.ConditionallyMutate, returnValueKind: ValueKind.Mutable, }, - BuiltinEffectEventId, + BuiltInEffectEventId, +); + +addFunction( + BUILTIN_SHAPES, + [], + { + positionalParams: [], + restParam: Effect.ConditionallyMutate, + returnType: {kind: 'Poly'}, + calleeEffect: Effect.ConditionallyMutate, + returnValueKind: ValueKind.Mutable, + }, + BuiltInEventHandlerId, ); /** 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 55974db14ce1..b6ec11fdb4ff 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts @@ -25,6 +25,7 @@ import { } from '../HIR/HIR'; import { BuiltInArrayId, + BuiltInEventHandlerId, BuiltInFunctionId, BuiltInJsxId, BuiltInMixedReadonlyId, @@ -471,6 +472,41 @@ function* generateInstructionTypes( } } } + if (env.config.enableInferEventHandlers) { + if ( + value.kind === 'JsxExpression' && + value.tag.kind === 'BuiltinTag' && + !value.tag.name.includes('-') + ) { + /* + * Infer event handler types for built-in DOM elements. + * Props starting with "on" (e.g., onClick, onSubmit) on primitive tags + * are inferred as event handlers. This allows functions with ref access + * to be passed to these props, since DOM event handlers are guaranteed + * by React to only execute in response to events, never during render. + * + * We exclude tags with hyphens to avoid web components (custom elements), + * which are required by the HTML spec to contain a hyphen. Web components + * may call event handler props during their lifecycle methods (e.g., + * connectedCallback), which would be unsafe for ref access. + */ + for (const prop of value.props) { + if ( + prop.kind === 'JsxAttribute' && + prop.name.startsWith('on') && + prop.name.length > 2 && + prop.name[2] === prop.name[2].toUpperCase() + ) { + yield equation(prop.place.identifier.type, { + kind: 'Function', + shapeId: BuiltInEventHandlerId, + return: makeType(), + isConstructor: false, + }); + } + } + } + } yield equation(left, {kind: 'Object', shapeId: BuiltInJsxId}); break; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccessInRender.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccessInRender.ts index b0961f5fd7e9..232e9f55bbcf 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccessInRender.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccessInRender.ts @@ -14,12 +14,14 @@ import { BlockId, HIRFunction, IdentifierId, + Identifier, Place, SourceLocation, getHookKindForType, isRefValueType, isUseRefType, } from '../HIR'; +import {BuiltInEventHandlerId} from '../HIR/ObjectShape'; import { eachInstructionOperand, eachInstructionValueOperand, @@ -183,6 +185,11 @@ function refTypeOfType(place: Place): RefAccessType { } } +function isEventHandlerType(identifier: Identifier): boolean { + const type = identifier.type; + return type.kind === 'Function' && type.shapeId === BuiltInEventHandlerId; +} + function tyEqual(a: RefAccessType, b: RefAccessType): boolean { if (a.kind !== b.kind) { return false; @@ -354,7 +361,6 @@ function validateNoRefAccessInRenderImpl( } const interpolatedAsJsx = new Set(); - const usedAsEventHandlerProp = new Set(); for (const block of fn.body.blocks.values()) { for (const instr of block.instructions) { const {value} = instr; @@ -364,27 +370,6 @@ function validateNoRefAccessInRenderImpl( interpolatedAsJsx.add(child.identifier.id); } } - if (value.kind === 'JsxExpression') { - /* - * Track identifiers used as event handler props on built-in DOM elements. - * We only allow this optimization for native DOM elements because their - * event handlers are guaranteed by React to only execute in response to - * events, never during render. Custom components could technically call - * their onFoo props during render, which would violate ref access rules. - */ - if (value.tag.kind === 'BuiltinTag') { - for (const prop of value.props) { - if ( - prop.kind === 'JsxAttribute' && - prop.name.startsWith('on') && - prop.name.length > 2 && - prop.name[2] === prop.name[2].toUpperCase() - ) { - usedAsEventHandlerProp.add(prop.place.identifier.id); - } - } - } - } } } } @@ -541,8 +526,8 @@ function validateNoRefAccessInRenderImpl( */ if (!didError) { const isRefLValue = isUseRefType(instr.lvalue.identifier); - const isUsedAsEventHandler = usedAsEventHandlerProp.has( - instr.lvalue.identifier.id, + const isEventHandlerLValue = isEventHandlerType( + instr.lvalue.identifier, ); for (const operand of eachInstructionValueOperand(instr.value)) { /** @@ -551,29 +536,16 @@ function validateNoRefAccessInRenderImpl( */ if ( isRefLValue || + isEventHandlerLValue || (hookKind != null && hookKind !== 'useState' && hookKind !== 'useReducer') ) { /** - * Special cases: - * - * 1. the lvalue is a ref - * In general passing a ref to a function may access that ref - * value during render, so we disallow it. - * - * The main exception is the "mergeRefs" pattern, ie a function - * that accepts multiple refs as arguments (or an array of refs) - * and returns a new, aggregated ref. If the lvalue is a ref, - * we assume that the user is doing this pattern and allow passing - * refs. - * - * Eg `const mergedRef = mergeRefs(ref1, ref2)` - * - * 2. calling hooks - * - * Hooks are independently checked to ensure they don't access refs - * during render. + * Allow passing refs or ref-accessing functions when: + * 1. lvalue is a ref (mergeRefs pattern: `mergeRefs(ref1, ref2)`) + * 2. lvalue is an event handler (DOM events execute outside render) + * 3. calling hooks (independently validated for ref safety) */ validateNoDirectRefValueAccess(errors, operand, env); } else if (interpolatedAsJsx.has(instr.lvalue.identifier.id)) { @@ -585,25 +557,6 @@ function validateNoRefAccessInRenderImpl( * render function which attempts to obey the rules. */ validateNoRefValueAccess(errors, env, operand); - } else if (isUsedAsEventHandler) { - /** - * Special case: the lvalue is used as an event handler prop - * on a built-in DOM element - * - * For example ``. Here - * handleSubmit is wrapping onSubmit to create an event handler. - * Functions passed to event handler wrappers can safely access - * refs because built-in DOM event handlers are guaranteed by React - * to only execute in response to actual events, never during render. - * - * We only allow this for built-in DOM elements (not custom components) - * because custom components could technically call their onFoo props - * during render, which would violate ref access rules. - * - * We allow passing functions with ref access to these wrappers, - * but still validate that direct ref values aren't passed. - */ - validateNoDirectRefValueAccess(errors, operand, env); } else { validateNoRefPassedToFunction( errors, diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-async-event-handler-wrapper.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-async-event-handler-wrapper.expect.md index c3d65a42e951..5cee9a68ceba 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-async-event-handler-wrapper.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-async-event-handler-wrapper.expect.md @@ -2,7 +2,7 @@ ## Input ```javascript -// @validateRefAccessDuringRender +// @enableInferEventHandlers import {useRef} from 'react'; // Simulates react-hook-form's handleSubmit @@ -56,7 +56,7 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; // @validateRefAccessDuringRender +import { c as _c } from "react/compiler-runtime"; // @enableInferEventHandlers import { useRef } from "react"; // Simulates react-hook-form's handleSubmit diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-async-event-handler-wrapper.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-async-event-handler-wrapper.tsx index dfb1f84961c8..be6f6656e18f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-async-event-handler-wrapper.tsx +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-async-event-handler-wrapper.tsx @@ -1,4 +1,4 @@ -// @validateRefAccessDuringRender +// @enableInferEventHandlers import {useRef} from 'react'; // Simulates react-hook-form's handleSubmit diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-event-handler-wrapper.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-event-handler-wrapper.expect.md index 805b31e92bf8..d40a1e080ad3 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-event-handler-wrapper.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-event-handler-wrapper.expect.md @@ -2,7 +2,7 @@ ## Input ```javascript -// @validateRefAccessDuringRender +// @enableInferEventHandlers import {useRef} from 'react'; // Simulates react-hook-form's handleSubmit or similar event handler wrappers @@ -44,7 +44,7 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; // @validateRefAccessDuringRender +import { c as _c } from "react/compiler-runtime"; // @enableInferEventHandlers import { useRef } from "react"; // Simulates react-hook-form's handleSubmit or similar event handler wrappers diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-event-handler-wrapper.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-event-handler-wrapper.tsx index 59ee12e5c5a7..f305a1f9ac6e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-event-handler-wrapper.tsx +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-event-handler-wrapper.tsx @@ -1,4 +1,4 @@ -// @validateRefAccessDuringRender +// @enableInferEventHandlers import {useRef} from 'react'; // Simulates react-hook-form's handleSubmit or similar event handler wrappers diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-custom-component-event-handler-wrapper.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-custom-component-event-handler-wrapper.expect.md index 97f6732f279e..2a3657eda34f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-custom-component-event-handler-wrapper.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-custom-component-event-handler-wrapper.expect.md @@ -2,7 +2,7 @@ ## Input ```javascript -// @validateRefAccessDuringRender +// @enableInferEventHandlers import {useRef} from 'react'; // Simulates a custom component wrapper diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-custom-component-event-handler-wrapper.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-custom-component-event-handler-wrapper.tsx index 467486ff7b3a..b90a12171654 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-custom-component-event-handler-wrapper.tsx +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-custom-component-event-handler-wrapper.tsx @@ -1,4 +1,4 @@ -// @validateRefAccessDuringRender +// @enableInferEventHandlers import {useRef} from 'react'; // Simulates a custom component wrapper diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-event-handler-wrapper.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-event-handler-wrapper.expect.md index 41586dd744aa..718e2c81419f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-event-handler-wrapper.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-event-handler-wrapper.expect.md @@ -2,7 +2,7 @@ ## Input ```javascript -// @validateRefAccessDuringRender +// @enableInferEventHandlers import {useRef} from 'react'; // Simulates a handler wrapper diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-event-handler-wrapper.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-event-handler-wrapper.tsx index 5002e910aceb..58313e560ce7 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-event-handler-wrapper.tsx +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.ref-value-in-event-handler-wrapper.tsx @@ -1,4 +1,4 @@ -// @validateRefAccessDuringRender +// @enableInferEventHandlers import {useRef} from 'react'; // Simulates a handler wrapper diff --git a/packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.js b/packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.js index c5024bf5d676..18dab0710f25 100644 --- a/packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.js +++ b/packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.js @@ -20,10 +20,6 @@ function normalizeSourceLoc(tree) { node.hookSource.lineNumber = 0; node.hookSource.columnNumber = 0; } - // Normalize Promise objects to hide internal symbols - if (node.value instanceof Promise) { - node.value = Object.create(Promise.prototype); - } normalizeSourceLoc(node.subHooks); }); return tree;