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 571fe61c81..70aa4eeea8 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccessInRender.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccessInRender.ts @@ -27,6 +27,7 @@ import { eachTerminalOperand, } from '../HIR/visitors'; import {Err, Ok, Result} from '../Utils/Result'; +import {retainWhere} from '../Utils/utils'; /** * Validates that a function does not access a ref value during render. This includes a partial check @@ -279,9 +280,10 @@ function validateNoRefAccessInRenderImpl( for (let i = 0; (i == 0 || env.hasChanged()) && i < 10; i++) { env.resetChanged(); returnValues = []; - const safeBlocks = new Map(); + const safeBlocks: Array<{block: BlockId; ref: RefId}> = []; const errors = new CompilerError(); for (const [, block] of fn.body.blocks) { + retainWhere(safeBlocks, entry => entry.block !== block.id); for (const phi of block.phis) { env.set( phi.place.identifier.id, @@ -503,15 +505,17 @@ function validateNoRefAccessInRenderImpl( case 'PropertyStore': case 'ComputedDelete': case 'ComputedStore': { - const safe = safeBlocks.get(block.id); const target = env.get(instr.value.object.identifier.id); + let safe: (typeof safeBlocks)['0'] | null | undefined = null; if ( instr.value.kind === 'PropertyStore' && - safe != null && - target?.kind === 'Ref' && - target.refId === safe + target != null && + target.kind === 'Ref' ) { - safeBlocks.delete(block.id); + safe = safeBlocks.find(entry => entry.ref === target.refId); + } + if (safe != null) { + retainWhere(safeBlocks, entry => entry !== safe); } else { validateNoRefUpdate(errors, env, instr.value.object, instr.loc); } @@ -599,8 +603,11 @@ function validateNoRefAccessInRenderImpl( if (block.terminal.kind === 'if') { const test = env.get(block.terminal.test.identifier.id); - if (test?.kind === 'Guard') { - safeBlocks.set(block.terminal.consequent, test.refId); + if ( + test?.kind === 'Guard' && + safeBlocks.find(entry => entry.ref === test.refId) == null + ) { + safeBlocks.push({block: block.terminal.fallthrough, ref: test.refId}); } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-lazy-initialization-with-logical.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-lazy-initialization-with-logical.expect.md new file mode 100644 index 0000000000..3540e842f6 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-lazy-initialization-with-logical.expect.md @@ -0,0 +1,68 @@ + +## Input + +```javascript +// @validateRefAccessDuringRender + +import {useRef} from 'react'; + +function Component(props) { + const ref = useRef(null); + if (ref.current == null) { + // the logical means the ref write is in a different block + // from the if consequent. this tests that the "safe" blocks + // extend up to the if's fallthrough + ref.current = props.unknownKey ?? props.value; + } + return ; +} + +function Child({ref}) { + 'use no memo'; + return ref.current; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{value: 42}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateRefAccessDuringRender + +import { useRef } from "react"; + +function Component(props) { + const $ = _c(1); + const ref = useRef(null); + if (ref.current == null) { + ref.current = props.unknownKey ?? props.value; + } + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = ; + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} + +function Child({ ref }) { + "use no memo"; + return ref.current; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ value: 42 }], +}; + +``` + +### Eval output +(kind: ok) 42 \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-lazy-initialization-with-logical.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-lazy-initialization-with-logical.js new file mode 100644 index 0000000000..2e1b03a28d --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-lazy-initialization-with-logical.js @@ -0,0 +1,24 @@ +// @validateRefAccessDuringRender + +import {useRef} from 'react'; + +function Component(props) { + const ref = useRef(null); + if (ref.current == null) { + // the logical means the ref write is in a different block + // from the if consequent. this tests that the "safe" blocks + // extend up to the if's fallthrough + ref.current = props.unknownKey ?? props.value; + } + return ; +} + +function Child({ref}) { + 'use no memo'; + return ref.current; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{value: 42}], +};