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;