From 440d8c28762ebd92bc257d00429ad38a3efdbbfb Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Wed, 25 Jun 2025 09:49:16 -0700 Subject: [PATCH 1/4] [compiler] Fix bug with reassigning function param in destructuring Closes #33577, a bug with ExtractScopeDeclarationsFromDestructuring and codegen when a function param is reassigned. --- .../ReactiveScopes/CodegenReactiveFunction.ts | 8 +-- ...tractScopeDeclarationsFromDestructuring.ts | 4 ++ .../compiler/repro-reassign-props.expect.md | 65 +++++++++++++++++++ .../fixtures/compiler/repro-reassign-props.js | 11 ++++ 4 files changed, 83 insertions(+), 5 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-reassign-props.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-reassign-props.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts index 9e91d481db60..74d5b0a75b61 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -349,11 +349,9 @@ function codegenReactiveFunction( fn: ReactiveFunction, ): Result { for (const param of fn.params) { - if (param.kind === 'Identifier') { - cx.temp.set(param.identifier.declarationId, null); - } else { - cx.temp.set(param.place.identifier.declarationId, null); - } + const place = param.kind === 'Identifier' ? param : param.place; + cx.temp.set(place.identifier.declarationId, null); + cx.declare(place.identifier); } const params = fn.params.map(param => convertParameter(param)); diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/ExtractScopeDeclarationsFromDestructuring.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/ExtractScopeDeclarationsFromDestructuring.ts index eb2caa424e41..642b89747e6e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/ExtractScopeDeclarationsFromDestructuring.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/ExtractScopeDeclarationsFromDestructuring.ts @@ -79,6 +79,10 @@ export function extractScopeDeclarationsFromDestructuring( fn: ReactiveFunction, ): void { const state = new State(fn.env); + for (const param of fn.params) { + const place = param.kind === 'Identifier' ? param : param.place; + state.declared.add(place.identifier.declarationId); + } visitReactiveFunction(fn, new Visitor(), state); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-reassign-props.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-reassign-props.expect.md new file mode 100644 index 000000000000..19c85c943e71 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-reassign-props.expect.md @@ -0,0 +1,65 @@ + +## Input + +```javascript +import {Stringify, useIdentity} from 'shared-runtime'; + +function Component({other, ...props}, ref) { + [props, ref] = useIdentity([props, ref]); + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{a: 0, b: 'hello', children:
Hello
}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { Stringify, useIdentity } from "shared-runtime"; + +function Component(t0, ref) { + const $ = _c(7); + let props; + if ($[0] !== t0) { + let { other, ...t1 } = t0; + props = t1; + $[0] = t0; + $[1] = props; + } else { + props = $[1]; + } + let t1; + if ($[2] !== props || $[3] !== ref) { + t1 = [props, ref]; + $[2] = props; + $[3] = ref; + $[4] = t1; + } else { + t1 = $[4]; + } + [props, ref] = useIdentity(t1); + let t2; + if ($[5] !== props) { + t2 = ; + $[5] = props; + $[6] = t2; + } else { + t2 = $[6]; + } + return t2; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ a: 0, b: "hello", children:
Hello
}], +}; + +``` + +### Eval output +(kind: ok)
{"props":{"a":0,"b":"hello","children":{"type":"div","key":null,"props":{"children":"Hello"},"_owner":"[[ cyclic ref *3 ]]","_store":{}}}}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-reassign-props.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-reassign-props.js new file mode 100644 index 000000000000..329000aedb08 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-reassign-props.js @@ -0,0 +1,11 @@ +import {Stringify, useIdentity} from 'shared-runtime'; + +function Component({other, ...props}, ref) { + [props, ref] = useIdentity([props, ref]); + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{a: 0, b: 'hello', children:
Hello
}], +}; From 6ef9e5bd499864f8f77de4d27949f2a3c2d64f6d Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Wed, 25 Jun 2025 09:49:16 -0700 Subject: [PATCH 2/4] [compiler] Avoid empty switch cases Small cosmetic win, found this when i was looking at some code internally with lots of cases that all share the same logic. Previously, all the but last one would have an empty block. --- .../src/ReactiveScopes/CodegenReactiveFunction.ts | 2 +- ...lock-scoping-switch-variable-scoping.expect.md | 3 +-- .../fixtures/compiler/dominator.expect.md | 3 +-- .../fixtures/compiler/reverse-postorder.expect.md | 6 ++---- .../fixtures/compiler/ssa-switch.expect.md | 3 +-- .../compiler/switch-with-fallthrough.expect.md | 15 +++++---------- 6 files changed, 11 insertions(+), 21 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts index 74d5b0a75b61..f7da5229548a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -1181,7 +1181,7 @@ function codegenTerminal( ? codegenPlaceToExpression(cx, case_.test) : null; const block = codegenBlock(cx, case_.block!); - return t.switchCase(test, [block]); + return t.switchCase(test, block.body.length === 0 ? [] : [block]); }), ); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/block-scoping-switch-variable-scoping.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/block-scoping-switch-variable-scoping.expect.md index d5bf094ef4ff..a225812dbd30 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/block-scoping-switch-variable-scoping.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/block-scoping-switch-variable-scoping.expect.md @@ -50,8 +50,7 @@ function Component(props) { console.log(handlers.value); break bb0; } - default: { - } + default: } t0 = handlers; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/dominator.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/dominator.expect.md index e878d4fb7f82..508a7b62581d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/dominator.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/dominator.expect.md @@ -67,8 +67,7 @@ function Component(props) { case "b": { break bb1; } - case "c": { - } + case "c": default: { x = 6; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reverse-postorder.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reverse-postorder.expect.md index c0ff0f7e7db7..98f2cd2190c2 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reverse-postorder.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reverse-postorder.expect.md @@ -50,10 +50,8 @@ function Component(props) { case 1: { break bb0; } - case 2: { - } - default: { - } + case 2: + default: } } else { if (props.cond2) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-switch.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-switch.expect.md index 4796fbdcc2bc..48a765d3f363 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-switch.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-switch.expect.md @@ -41,8 +41,7 @@ function foo() { case 2: { break bb0; } - default: { - } + default: } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/switch-with-fallthrough.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/switch-with-fallthrough.expect.md index c54631092c0a..6fd911c43271 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/switch-with-fallthrough.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/switch-with-fallthrough.expect.md @@ -43,22 +43,17 @@ export const FIXTURE_ENTRYPOINT = { ```javascript function foo(x) { bb0: switch (x) { - case 0: { - } - case 1: { - } + case 0: + case 1: case 2: { break bb0; } case 3: { break bb0; } - case 4: { - } - case 5: { - } - default: { - } + case 4: + case 5: + default: } } From 2de9ddac73a6b082314eef68036eb160a18f5c87 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Wed, 25 Jun 2025 09:49:16 -0700 Subject: [PATCH 3/4] [compiler] Consolidate HIRFunction return information We now have `HIRFunction.returns: Place` as well as `returnType: Type`. I want to add additional return information, so as a first step i'm consolidating everything under an object at `HIRFunction.returns: {place: Place}`. We use the type of this place as the return type. Next step is to add more properties to this object to represent things like the return kind. --- .../babel-plugin-react-compiler/src/HIR/BuildHIR.ts | 1 - .../babel-plugin-react-compiler/src/HIR/HIR.ts | 1 - .../babel-plugin-react-compiler/src/HIR/PrintHIR.ts | 8 ++++---- .../src/Inference/InferMutationAliasingRanges.ts | 13 +++++++------ .../src/Optimization/LowerContextAccess.ts | 2 -- .../src/Optimization/OutlineJsx.ts | 2 -- .../src/TypeInference/InferTypes.ts | 9 +++++---- 7 files changed, 16 insertions(+), 20 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts index 998ed43b04eb..104b08068151 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts @@ -221,7 +221,6 @@ export function lower( params, fnType: bindings == null ? env.fnType : 'Other', returnTypeAnnotation: null, // TODO: extract the actual return type node if present - returnType: makeType(), returns: createTemporaryPlace(env, func.node.loc ?? GeneratedSource), body: builder.build(), context, diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts index 2869ab94d996..deb725a0482e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -279,7 +279,6 @@ export type HIRFunction = { env: Environment; params: Array; returnTypeAnnotation: t.FlowType | t.TSType | null; - returnType: Type; returns: Place; context: Array; effects: Array | null; diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts index f42f4bcf19b3..89591aca2dfb 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts @@ -54,6 +54,8 @@ export function printFunction(fn: HIRFunction): string { let definition = ''; if (fn.id !== null) { definition += fn.id; + } else { + definition += '<>'; } if (fn.params.length !== 0) { definition += @@ -71,10 +73,8 @@ export function printFunction(fn: HIRFunction): string { } else { definition += '()'; } - if (definition.length !== 0) { - output.push(definition); - } - output.push(`: ${printType(fn.returnType)} @ ${printPlace(fn.returns)}`); + definition += `: ${printPlace(fn.returns)}`; + output.push(definition); output.push(...fn.directives); output.push(printHIR(fn.body)); return output.join('\n'); diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts index 11401ae71567..79f8cf8c0e85 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts @@ -18,6 +18,7 @@ import { ValueKind, ValueReason, Place, + isPrimitiveType, } from '../HIR/HIR'; import { eachInstructionLValue, @@ -471,15 +472,15 @@ export function inferMutationAliasingRanges( * Here we populate an effect to create the return value as well as populating alias/capture * effects for how data flows between the params, context vars, and return. */ + const returns = fn.returns.identifier; functionEffects.push({ kind: 'Create', into: fn.returns, - value: - fn.returnType.kind === 'Primitive' - ? ValueKind.Primitive - : isJsxType(fn.returnType) - ? ValueKind.Frozen - : ValueKind.Mutable, + value: isPrimitiveType(returns) + ? ValueKind.Primitive + : isJsxType(returns.type) + ? ValueKind.Frozen + : ValueKind.Mutable, reason: ValueReason.KnownReturnSignature, }); /** diff --git a/compiler/packages/babel-plugin-react-compiler/src/Optimization/LowerContextAccess.ts b/compiler/packages/babel-plugin-react-compiler/src/Optimization/LowerContextAccess.ts index 32486577fb35..921ec59ecd2a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Optimization/LowerContextAccess.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Optimization/LowerContextAccess.ts @@ -25,7 +25,6 @@ import { makeBlockId, makeInstructionId, makePropertyLiteral, - makeType, markInstructionIds, promoteTemporary, reversePostorderBlocks, @@ -253,7 +252,6 @@ function emitSelectorFn(env: Environment, keys: Array): Instruction { env, params: [obj], returnTypeAnnotation: null, - returnType: makeType(), returns: createTemporaryPlace(env, GeneratedSource), context: [], effects: null, diff --git a/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineJsx.ts b/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineJsx.ts index 667629a3e076..b7590a570197 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineJsx.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineJsx.ts @@ -21,7 +21,6 @@ import { makeBlockId, makeIdentifierName, makeInstructionId, - makeType, ObjectProperty, Place, promoteTemporary, @@ -368,7 +367,6 @@ function emitOutlinedFn( env, params: [propsObj], returnTypeAnnotation: null, - returnType: makeType(), returns: createTemporaryPlace(env, GeneratedSource), context: [], effects: null, 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 69812fc130de..859c871c263a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts @@ -90,7 +90,8 @@ function apply(func: HIRFunction, unifier: Unifier): void { } } } - func.returnType = unifier.get(func.returnType); + const returns = func.returns.identifier; + returns.type = unifier.get(returns.type); } type TypeEquation = { @@ -143,12 +144,12 @@ function* generate( } } if (returnTypes.length > 1) { - yield equation(func.returnType, { + yield equation(func.returns.identifier.type, { kind: 'Phi', operands: returnTypes, }); } else if (returnTypes.length === 1) { - yield equation(func.returnType, returnTypes[0]!); + yield equation(func.returns.identifier.type, returnTypes[0]!); } } @@ -407,7 +408,7 @@ function* generateInstructionTypes( yield equation(left, { kind: 'Function', shapeId: BuiltInFunctionId, - return: value.loweredFunc.func.returnType, + return: value.loweredFunc.func.returns.identifier.type, isConstructor: false, }); break; From 5eda822159ba2314f66eae36ef7e90c38a5bfd15 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Wed, 25 Jun 2025 09:49:16 -0700 Subject: [PATCH 4/4] [compiler] Alternate take on ref validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some of the false positives we've seen have to do with the need to align our ref validation with our understanding of which functions may be called during render. The new mutability/aliasing model makes this much more explicit, with the ability to create Impure effects which we then throw as errors if they are reachable during render. This means we can now revisit ref validation by just emitting impure effects. That's what this new pass does. It's a bit simpler: it implements the check for `ref.current == null` guarded if blocks. Otherwise it disallows access to `ref.current` specifically. Unlike before, we intentionally allow passing ref objects to functions — we just see a lot of many false positives on disallowing things like `children({ref})` or similar. Open to feedback! This is also still WIP. --- .../src/Entrypoint/Pipeline.ts | 2 +- .../Inference/InferMutationAliasingEffects.ts | 129 ++++++++++++++++++ .../compiler/error.hook-ref-value.expect.md | 2 - ...invalid-access-ref-during-render.expect.md | 2 + ...-callback-invoked-during-render-.expect.md | 17 ++- ...tating-refs-in-render-transitive.expect.md | 16 +-- ...d-ref-prop-in-render-destructure.expect.md | 4 + ...ref-prop-in-render-property-load.expect.md | 4 + ...n-callback-invoked-during-render.expect.md | 17 ++- ...d-set-and-read-ref-during-render.expect.md | 2 - .../error.ref-initialization-nonif.expect.md | 4 +- ...ia-function-preserve-memoization.expect.md | 12 +- ...-current-aliased-no-added-to-dep.expect.md | 4 +- .../ref-current-aliased-no-added-to-dep.js | 2 +- 14 files changed, 179 insertions(+), 38 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts index c5ca3434b1b5..d06b53c1b603 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -285,7 +285,7 @@ function runWithEnvironment( } if (env.config.validateRefAccessDuringRender) { - validateNoRefAccessInRender(hir).unwrap(); + // validateNoRefAccessInRender(hir).unwrap(); } if (env.config.validateNoSetStateInRender) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts index b91b606d507e..37defd18b343 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts @@ -28,7 +28,9 @@ import { isMapType, isPrimitiveType, isRefOrRefValue, + isRefValueType, isSetType, + isUseRefType, makeIdentifierId, Phi, Place, @@ -219,6 +221,9 @@ export function inferMutationAliasingEffects( } } } + if (fn.env.config.validateRefAccessDuringRender) { + inferRefAccessEffects(fn, isFunctionExpression); + } return Ok(undefined); } @@ -2513,3 +2518,127 @@ export type AbstractValue = { kind: ValueKind; reason: ReadonlySet; }; + +function inferRefAccessEffects( + fn: HIRFunction, + _isFunctionExpression: boolean, +): void { + const nullish = new Set(); + const nullishTest = new Map(); + let guard: {ref: IdentifierId; fallthrough: BlockId} | null = null; + const temporaries: Map = new Map(); + + function visitOperand(operand: Place): AliasingEffect | null { + const nullTestRef = nullishTest.get(operand.identifier.id); + if (isRefValueType(operand.identifier) || nullTestRef != null) { + const refOperand = nullTestRef ?? operand; + return { + kind: 'Impure', + error: { + severity: ErrorSeverity.InvalidReact, + reason: + 'Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)', + loc: refOperand.loc, + description: + refOperand.identifier.name !== null && + refOperand.identifier.name.kind === 'named' + ? `Cannot access ref value \`${refOperand.identifier.name.value}\`` + : null, + suggestions: null, + }, + place: refOperand, + }; + } + return null; + } + + for (const block of fn.body.blocks.values()) { + if (guard !== null && guard.fallthrough === block.id) { + guard = null; + } + for (const instr of block.instructions) { + const {lvalue, value} = instr; + if (value.kind === 'LoadLocal' && isUseRefType(value.place.identifier)) { + temporaries.set(lvalue.identifier.id, value.place); + } else if ( + value.kind === 'StoreLocal' && + isUseRefType(value.value.identifier) + ) { + temporaries.set(value.lvalue.place.identifier.id, value.value); + temporaries.set(lvalue.identifier.id, value.value); + } else if ( + value.kind === 'BinaryExpression' && + ((isRefValueType(value.left.identifier) && + nullish.has(value.right.identifier.id)) || + (nullish.has(value.left.identifier.id) && + isRefValueType(value.right.identifier))) + ) { + const refOperand = isRefValueType(value.left.identifier) + ? value.left + : value.right; + const operand = temporaries.get(refOperand.identifier.id) ?? refOperand; + nullishTest.set(lvalue.identifier.id, operand); + } else if (value.kind === 'Primitive' && value.value == null) { + nullish.add(lvalue.identifier.id); + } else if ( + value.kind === 'PropertyLoad' && + isUseRefType(value.object.identifier) && + value.property === 'current' + ) { + const refOperand = + temporaries.get(value.object.identifier.id) ?? value.object; + temporaries.set(lvalue.identifier.id, refOperand); + } else if ( + value.kind === 'PropertyStore' && + value.property === 'current' && + isUseRefType(value.object.identifier) + ) { + const refOperand = + temporaries.get(value.object.identifier.id) ?? value.object; + if (guard != null && refOperand.identifier.id === guard.ref) { + // Allow a single write within the guard + guard = null; + } else { + instr.effects ??= []; + instr.effects.push({ + kind: 'Impure', + error: { + severity: ErrorSeverity.InvalidReact, + reason: + 'Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)', + loc: value.loc, + description: + value.object.identifier.name !== null && + value.object.identifier.name.kind === 'named' + ? `Cannot access ref value \`${value.object.identifier.name.value}\`` + : null, + suggestions: null, + }, + place: value.object, + }); + } + } else { + for (const operand of eachInstructionValueOperand(value)) { + const error = visitOperand(operand); + if (error) { + instr.effects ??= []; + instr.effects.push(error); + } + } + } + } + if ( + guard == null && + block.terminal.kind === 'if' && + nullishTest.has(block.terminal.test.identifier.id) + ) { + const ref = nullishTest.get(block.terminal.test.identifier.id)!; + guard = {ref: ref.identifier.id, fallthrough: block.terminal.fallthrough}; + } else { + for (const operand of eachTerminalOperand(block.terminal)) { + const _effect = visitOperand(operand); + // TODO: need a place to store terminal effects generically + } + } + } +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hook-ref-value.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hook-ref-value.expect.md index d92d918fe9f3..05a2b3cc0e7f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hook-ref-value.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hook-ref-value.expect.md @@ -24,8 +24,6 @@ export const FIXTURE_ENTRYPOINT = { 4 | const ref = useRef(); > 5 | useEffect(() => {}, [ref.current]); | ^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (5:5) - -InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (5:5) 6 | } 7 | 8 | export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-access-ref-during-render.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-access-ref-during-render.expect.md index 02748366456f..5e65ecbc1b7e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-access-ref-during-render.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-access-ref-during-render.expect.md @@ -19,6 +19,8 @@ function Component(props) { 3 | const ref = useRef(null); > 4 | const value = ref.current; | ^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (4:4) + +InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value `value` (5:5) 5 | return value; 6 | } 7 | diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-aliased-ref-in-callback-invoked-during-render-.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-aliased-ref-in-callback-invoked-during-render-.expect.md index e2ce2cceae3b..135b547d235e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-aliased-ref-in-callback-invoked-during-render-.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-aliased-ref-in-callback-invoked-during-render-.expect.md @@ -19,12 +19,17 @@ function Component(props) { ## Error ``` - 7 | return ; - 8 | }; -> 9 | return {props.items.map(item => renderItem(item))}; - | ^^^^^^^^^^^^^^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (9:9) - 10 | } - 11 | + 4 | const renderItem = item => { + 5 | const aliasedRef = ref; +> 6 | const current = aliasedRef.current; + | ^^^^^^^^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (6:6) + +InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value `current` (7:7) + +InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (7:7) + 7 | return ; + 8 | }; + 9 | return {props.items.map(item => renderItem(item))}; ``` \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-disallow-mutating-refs-in-render-transitive.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-disallow-mutating-refs-in-render-transitive.expect.md index f23ff6f3c8c5..3d5902bdb301 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-disallow-mutating-refs-in-render-transitive.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-disallow-mutating-refs-in-render-transitive.expect.md @@ -21,15 +21,13 @@ function Component() { ## Error ``` - 7 | }; - 8 | const changeRef = setRef; -> 9 | changeRef(); - | ^^^^^^^^^ InvalidReact: This function accesses a ref value (the `current` property), which may not be accessed during render. (https://react.dev/reference/react/useRef) (9:9) - -InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (9:9) - 10 | - 11 | return