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 521f55026e..6bb078e8fa 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts @@ -38,6 +38,7 @@ import { import { eachInstructionValueLValue, eachInstructionValueOperand, + eachTerminalOperand, eachTerminalSuccessor, } from '../HIR/visitors'; import {Ok, Result} from '../Utils/Result'; @@ -221,8 +222,19 @@ export function inferMutationAliasingEffects( return Ok(undefined); } -function findHoistedContextDeclarations(fn: HIRFunction): Set { - const hoisted = new Set(); +function findHoistedContextDeclarations( + fn: HIRFunction, +): Map { + const hoisted = new Map(); + function visit(place: Place): void { + if ( + hoisted.has(place.identifier.declarationId) && + hoisted.get(place.identifier.declarationId) == null + ) { + // If this is the first load of the value, store the location + hoisted.set(place.identifier.declarationId, place); + } + } for (const block of fn.body.blocks.values()) { for (const instr of block.instructions) { if (instr.value.kind === 'DeclareContext') { @@ -232,10 +244,17 @@ function findHoistedContextDeclarations(fn: HIRFunction): Set { kind == InstructionKind.HoistedFunction || kind == InstructionKind.HoistedLet ) { - hoisted.add(instr.value.lvalue.place.identifier.declarationId); + hoisted.set(instr.value.lvalue.place.identifier.declarationId, null); + } + } else { + for (const operand of eachInstructionValueOperand(instr.value)) { + visit(operand); } } } + for (const operand of eachTerminalOperand(block.terminal)) { + visit(operand); + } } return hoisted; } @@ -248,12 +267,12 @@ class Context { catchHandlers: Map = new Map(); isFuctionExpression: boolean; fn: HIRFunction; - hoistedContextDeclarations: Set; + hoistedContextDeclarations: Map; constructor( isFunctionExpression: boolean, fn: HIRFunction, - hoistedContextDeclarations: Set, + hoistedContextDeclarations: Map, ) { this.isFuctionExpression = isFunctionExpression; this.fn = fn; @@ -901,48 +920,69 @@ function applyEffect( console.log(prettyFormat(state.debugAbstractValue(value))); } - let reason: string; - let description: string | null = null; - if ( mutationKind === 'mutate-frozen' && context.hoistedContextDeclarations.has( effect.value.identifier.declarationId, ) ) { - reason = `This variable is accessed before it is declared, which prevents the earlier access from updating when this value changes over time`; - if ( + const description = effect.value.identifier.name !== null && effect.value.identifier.name.kind === 'named' - ) { - description = `Move the declaration of \`${effect.value.identifier.name.value}\` to before it is first referenced`; + ? `Variable \`${effect.value.identifier.name.value}\` is accessed before it is declared` + : null; + const hoistedAccess = context.hoistedContextDeclarations.get( + effect.value.identifier.declarationId, + ); + if (hoistedAccess != null && hoistedAccess.loc != effect.value.loc) { + effects.push({ + kind: 'MutateFrozen', + place: effect.value, + error: { + severity: ErrorSeverity.InvalidReact, + reason: `This variable is accessed before it is declared, which may prevent it from updating as the assigned value changes over time`, + description, + loc: hoistedAccess.loc, + suggestions: null, + }, + }); } + + effects.push({ + kind: 'MutateFrozen', + place: effect.value, + error: { + severity: ErrorSeverity.InvalidReact, + reason: `This variable is accessed before it is declared, which prevents the earlier access from updating when this value changes over time`, + description, + loc: effect.value.loc, + suggestions: null, + }, + }); } else { - reason = getWriteErrorReason({ + const reason = getWriteErrorReason({ kind: value.kind, reason: value.reason, context: new Set(), }); - if ( + const description = effect.value.identifier.name !== null && effect.value.identifier.name.kind === 'named' - ) { - description = `Found mutation of \`${effect.value.identifier.name.value}\``; - } + ? `Found mutation of \`${effect.value.identifier.name.value}\`` + : null; + effects.push({ + kind: + value.kind === ValueKind.Frozen ? 'MutateFrozen' : 'MutateGlobal', + place: effect.value, + error: { + severity: ErrorSeverity.InvalidReact, + reason, + description, + loc: effect.value.loc, + suggestions: null, + }, + }); } - - effects.push({ - kind: - value.kind === ValueKind.Frozen ? 'MutateFrozen' : 'MutateGlobal', - place: effect.value, - error: { - severity: ErrorSeverity.InvalidReact, - reason, - description, - loc: effect.value.loc, - suggestions: null, - }, - }); } break; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-hoisting-setstate.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-hoisting-setstate.expect.md index 3fcc84c9a4..1be37ef830 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-hoisting-setstate.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-hoisting-setstate.expect.md @@ -38,13 +38,15 @@ export const FIXTURE_ENTRYPOINT = { ## Error ``` - 19 | useEffect(() => setState(2), []); + 17 | * $2 = Function context=setState + 18 | */ +> 19 | useEffect(() => setState(2), []); + | ^^^^^^^^ InvalidReact: This variable is accessed before it is declared, which may prevent it from updating as the assigned value changes over time. Variable `setState` is accessed before it is declared (19:19) + +InvalidReact: This variable is accessed before it is declared, which prevents the earlier access from updating when this value changes over time. Variable `setState` is accessed before it is declared (21:21) 20 | -> 21 | const [state, setState] = useState(0); - | ^^^^^^^^ InvalidReact: This variable is accessed before it is declared, which prevents the earlier access from updating when this value changes over time. Move the declaration of `setState` to before it is first referenced (21:21) + 21 | const [state, setState] = useState(0); 22 | return ; - 23 | } - 24 | ``` \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.invalid-referencing-frozen-hoisted-storecontext-const.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.invalid-referencing-frozen-hoisted-storecontext-const.expect.md index 7bf3cd0cd3..a95ace1df5 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.invalid-referencing-frozen-hoisted-storecontext-const.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.invalid-referencing-frozen-hoisted-storecontext-const.expect.md @@ -31,13 +31,15 @@ function Component({content, refetch}) { ## Error ``` - 17 | // This has to error: onRefetch needs to memoize with `content` as a - 18 | // dependency, but the dependency comes later -> 19 | const {data = null} = content; - | ^^^^^^^^^^^ InvalidReact: This variable is accessed before it is declared, which prevents the earlier access from updating when this value changes over time. Move the declaration of `data` to before it is first referenced (19:19) - 20 | - 21 | return ; - 22 | } + 9 | // TDZ violation! + 10 | const onRefetch = useCallback(() => { +> 11 | refetch(data); + | ^^^^ InvalidReact: This variable is accessed before it is declared, which may prevent it from updating as the assigned value changes over time. Variable `data` is accessed before it is declared (11:11) + +InvalidReact: This variable is accessed before it is declared, which prevents the earlier access from updating when this value changes over time. Variable `data` is accessed before it is declared (19:19) + 12 | }, [refetch]); + 13 | + 14 | // The context variable gets frozen here since it's passed to a hook ``` \ No newline at end of file