From f4adddf911958cdeff3b306b4a1a4cc3104cfae3 Mon Sep 17 00:00:00 2001 From: KimHyeongRae0 Date: Fri, 24 Apr 2026 11:20:20 +0900 Subject: [PATCH] Fix false positive for self-referential useCallback --- .../Inference/InferMutationAliasingEffects.ts | 140 +++++++++++++++++- .../ReactCompilerRuleTypescript-test.ts | 44 ++++++ 2 files changed, 183 insertions(+), 1 deletion(-) 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 4cdbb6aea6c1..60416fb5fb46 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts @@ -227,7 +227,14 @@ function findHoistedContextDeclarations( fn: HIRFunction, ): Map { const hoisted = new Map(); + const initialized = new Set(); + const selfReferentialMemoizedCallbackCaptures = + findSelfReferentialMemoizedCallbackCaptures(fn); + function visit(place: Place): void { + if (initialized.has(place.identifier.declarationId)) { + return; + } if ( hoisted.has(place.identifier.declarationId) && hoisted.get(place.identifier.declarationId) == null @@ -239,13 +246,37 @@ function findHoistedContextDeclarations( for (const block of fn.body.blocks.values()) { for (const instr of block.instructions) { if (instr.value.kind === 'DeclareContext') { + const declarationId = instr.value.lvalue.place.identifier.declarationId; const kind = instr.value.lvalue.kind; if ( kind == InstructionKind.HoistedConst || kind == InstructionKind.HoistedFunction || kind == InstructionKind.HoistedLet ) { - hoisted.set(instr.value.lvalue.place.identifier.declarationId, null); + hoisted.set(declarationId, null); + } else if (hoisted.has(declarationId)) { + initialized.add(declarationId); + } + } else if (instr.value.kind === 'FunctionExpression') { + const skipDeclarations = + instr.lvalue != null + ? selfReferentialMemoizedCallbackCaptures.get( + instr.lvalue.identifier.id, + ) ?? null + : null; + for (const operand of instr.value.loweredFunc.func.context) { + if (skipDeclarations?.has(operand.identifier.declarationId)) { + continue; + } + visit(operand); + } + } else if (instr.value.kind === 'StoreContext') { + visit(instr.value.value); + if ( + hoisted.has(instr.value.lvalue.place.identifier.declarationId) && + instr.value.lvalue.kind !== InstructionKind.Reassign + ) { + initialized.add(instr.value.lvalue.place.identifier.declarationId); } } else { for (const operand of eachInstructionValueOperand(instr.value)) { @@ -257,9 +288,116 @@ function findHoistedContextDeclarations( visit(operand); } } + for (const [declarationId, firstAccess] of hoisted) { + if (firstAccess == null) { + hoisted.delete(declarationId); + } + } return hoisted; } +function findSelfReferentialMemoizedCallbackCaptures( + fn: HIRFunction, +): Map> { + const values = new Map(); + for (const block of fn.body.blocks.values()) { + for (const instr of block.instructions) { + if (instr.lvalue != null) { + values.set(instr.lvalue.identifier.id, instr.value); + } + } + } + + const captures = new Map>(); + for (const block of fn.body.blocks.values()) { + for (const instr of block.instructions) { + if ( + instr.value.kind !== 'StoreContext' || + instr.value.lvalue.kind === InstructionKind.Reassign + ) { + continue; + } + + const declarationId = instr.value.lvalue.place.identifier.declarationId; + const functionExpressions = findSelfReferentialMemoizedFunctions( + fn, + values, + declarationId, + instr.value.value.identifier.id, + ); + for (const functionExpression of functionExpressions) { + getOrInsertDefault(captures, functionExpression, new Set()).add( + declarationId, + ); + } + } + } + return captures; +} + +function findSelfReferentialMemoizedFunctions( + fn: HIRFunction, + values: ReadonlyMap, + declarationId: DeclarationId, + startId: IdentifierId, +): Set { + const matches = new Set(); + const seen = new Set(); + const queue = [startId]; + while (queue.length !== 0) { + const identifierId = queue.pop()!; + if (seen.has(identifierId)) { + continue; + } + seen.add(identifierId); + + const value = values.get(identifierId); + if (value == null) { + continue; + } + + switch (value.kind) { + case 'CallExpression': + case 'MethodCall': { + const callee = + value.kind === 'CallExpression' ? value.callee : value.property; + if (getHookKind(fn.env, callee.identifier) === 'useCallback') { + const callback = value.args[0]; + if (callback != null && callback.kind === 'Identifier') { + queue.push(callback.identifier.id); + } + } + break; + } + case 'FinishMemoize': { + queue.push(value.decl.identifier.id); + break; + } + case 'LoadContext': + case 'LoadLocal': { + queue.push(value.place.identifier.id); + break; + } + case 'StoreLocal': + case 'StoreContext': { + queue.push(value.value.identifier.id); + break; + } + case 'FunctionExpression': { + if ( + value.loweredFunc.func.context.some( + place => place.identifier.declarationId === declarationId, + ) + ) { + matches.add(identifierId); + } + break; + } + } + } + return matches; +} + class Context { internedEffects: Map = new Map(); instructionSignatureCache: Map = new Map(); diff --git a/packages/eslint-plugin-react-hooks/__tests__/ReactCompilerRuleTypescript-test.ts b/packages/eslint-plugin-react-hooks/__tests__/ReactCompilerRuleTypescript-test.ts index a0d0f6bdbc8e..d02983357409 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ReactCompilerRuleTypescript-test.ts +++ b/packages/eslint-plugin-react-hooks/__tests__/ReactCompilerRuleTypescript-test.ts @@ -36,6 +36,29 @@ const tests: CompilerTestCases = { } `, }, + { + name: 'Allows stable callback self-reference inside useCallback', + filename: 'test.tsx', + code: normalizeIndent` + import {useCallback, useEffect} from 'react'; + + function Test() { + const onMouseDown = useCallback(() => { + window.removeEventListener('mousedown', onMouseDown); + }, []); + + useEffect(() => { + window.addEventListener('mousedown', onMouseDown); + + return () => { + window.removeEventListener('mousedown', onMouseDown); + }; + }, [onMouseDown]); + + return
Hello
; + } + `, + }, { name: 'Repro for hooks as normal values', filename: 'test.tsx', @@ -97,6 +120,27 @@ const tests: CompilerTestCases = { }, ], }, + { + name: 'Still rejects later variable capture inside useCallback', + filename: 'test.tsx', + code: normalizeIndent` + import {useCallback} from 'react'; + + function Test({content, refetch}) { + const onRefetch = useCallback(() => { + refetch(data); + }, [refetch]); + + const {data = null} = content; + return ; + } + `, + errors: [ + { + message: /Cannot access variable before it is declared/, + }, + ], + }, // =========================================== // Tests for mayContainReactCode heuristic // Files that SHOULD be compiled (have React-like function names)