From 860b6fde3ba90282e68f8923397046fcdba289a1 Mon Sep 17 00:00:00 2001 From: fresh3nough Date: Sun, 1 Mar 2026 02:56:00 +0000 Subject: [PATCH 1/2] fix: detect setState in useEffect for anonymous component callbacks passed to HOFs (#35910) --- .../src/Entrypoint/Program.ts | 7 ++ .../ReactCompilerRuleTypescript-test.ts | 83 +++++++++++++++++++ .../src/shared/RunReactCompiler.ts | 4 +- 3 files changed, 93 insertions(+), 1 deletion(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts index 80ce909f35aa..8016e43597c7 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts @@ -1324,6 +1324,13 @@ function getFunctionName( if (parent.isVariableDeclarator() && parent.get('init').node === path.node) { // const useHook = () => {}; id = parent.get('id'); + } else if ( + parent.isCallExpression() && + parent.parentPath.isVariableDeclarator() && + parent.parentPath.get('init').node === parent.node + ) { + // const MyComponent = wrap(() => {}); + id = parent.parentPath.get('id'); } else if ( parent.isAssignmentExpression() && parent.get('right').node === path.node && diff --git a/packages/eslint-plugin-react-hooks/__tests__/ReactCompilerRuleTypescript-test.ts b/packages/eslint-plugin-react-hooks/__tests__/ReactCompilerRuleTypescript-test.ts index a0d0f6bdbc8e..fb57692c3e47 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ReactCompilerRuleTypescript-test.ts +++ b/packages/eslint-plugin-react-hooks/__tests__/ReactCompilerRuleTypescript-test.ts @@ -192,6 +192,22 @@ const tests: CompilerTestCases = { }, ], }, + { + name: '[Heuristic] Compiles HOF-wrapped PascalCase component - detects prop mutation', + filename: 'component.tsx', + code: normalizeIndent` + const wrap = (value) => value; + const MyComponent = wrap(({a}) => { + a.key = 'value'; + return
; + }); + `, + errors: [ + { + message: /Modifying component props/, + }, + ], + }, ], }; @@ -199,3 +215,70 @@ const eslintTester = new ESLintTesterV8({ parser: require.resolve('@typescript-eslint/parser-v5'), }); eslintTester.run('react-compiler', allRules['immutability'].rule, tests); + +// Tests for set-state-in-effect rule with HOF-wrapped components +const setStateInEffectTests: CompilerTestCases = { + valid: [ + { + name: 'Direct component with no setState in effect', + filename: 'test.tsx', + code: normalizeIndent` + import { useEffect, useState } from 'react'; + function DirectComponent() { + const [value, setValue] = useState(0); + useEffect(() => { + console.log(value); + }, []); + return
{value}
; + } + `, + }, + ], + invalid: [ + { + name: 'Direct component with setState in effect', + filename: 'test.tsx', + code: normalizeIndent` + import { useEffect, useState } from 'react'; + function DirectComponent() { + const [value, setValue] = useState(0); + useEffect(() => { + setValue(1); + }, []); + return
{value}
; + } + `, + errors: [ + { + message: /Calling setState synchronously within an effect/, + }, + ], + }, + { + name: 'Anonymous component callback passed to HOF has setState in effect', + filename: 'test.tsx', + code: normalizeIndent` + import { useEffect, useState } from 'react'; + const wrap = (value) => value; + const WrappedComponent = wrap(() => { + const [value, setValue] = useState(0); + useEffect(() => { + setValue(1); + }, []); + return
{value}
; + }); + `, + errors: [ + { + message: /Calling setState synchronously within an effect/, + }, + ], + }, + ], +}; + +eslintTester.run( + 'react-compiler-set-state-in-effect', + allRules['set-state-in-effect'].rule, + setStateInEffectTests, +); diff --git a/packages/eslint-plugin-react-hooks/src/shared/RunReactCompiler.ts b/packages/eslint-plugin-react-hooks/src/shared/RunReactCompiler.ts index 9aaddb07e656..8dcaf70365c5 100644 --- a/packages/eslint-plugin-react-hooks/src/shared/RunReactCompiler.ts +++ b/packages/eslint-plugin-react-hooks/src/shared/RunReactCompiler.ts @@ -97,6 +97,7 @@ function checkTopLevelNode(node: ESTree.Node): boolean { } // Handle: const MyComponent = () => {} or const useHook = function() {} + // Also handles: const MyComponent = wrap(() => {}) if (node.type === 'VariableDeclaration') { for (const decl of (node as ESTree.VariableDeclaration).declarations) { if (decl.id.type === 'Identifier') { @@ -104,7 +105,8 @@ function checkTopLevelNode(node: ESTree.Node): boolean { if ( init != null && (init.type === 'ArrowFunctionExpression' || - init.type === 'FunctionExpression') + init.type === 'FunctionExpression' || + init.type === 'CallExpression') ) { const name = decl.id.name; if (COMPONENT_NAME_PATTERN.test(name) || HOOK_NAME_PATTERN.test(name)) { From f4cfb20937dad0bae39481f21dbb9141d96dd058 Mon Sep 17 00:00:00 2001 From: fresh3nough Date: Sun, 1 Mar 2026 15:48:03 +0000 Subject: [PATCH 2/2] babel fixes --- .../src/Inference/AliasingEffects.ts | 6 +- .../Inference/InferMutationAliasingEffects.ts | 17 +++ .../Inference/InferMutationAliasingRanges.ts | 104 +++++++++++++++++- 3 files changed, 123 insertions(+), 4 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/AliasingEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/AliasingEffects.ts index 7f30e25a5c06..7d163c84734d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/AliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/AliasingEffects.ts @@ -66,7 +66,11 @@ export type AliasingEffect = /** * Mutates any of the value, its direct aliases, and its transitive captures that are mutable. */ - | {kind: 'MutateTransitiveConditionally'; value: Place} + | { + kind: 'MutateTransitiveConditionally'; + value: Place; + noBackwardRangeExtension?: boolean; + } /** * Records information flow from `from` to `into` in cases where local mutation of the destination * will *not* mutate the source: 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 0fb2cf9823c7..7b55073dbed4 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts @@ -1126,12 +1126,29 @@ function applyEffect( } const operand = arg.kind === 'Identifier' ? arg : arg.place; if (operand !== effect.function || effect.mutatesFunction) { + /* + * For the receiver of a MethodCall (where receiver !== function), + * use noBackwardRangeExtension so that the receiver's mutable + * range is not extended backward through aliases. Without this, + * values like `expensiveProcessing(data)` get pulled into the + * same reactive scope as the method call result, preventing + * independent memoization. The receiver itself and any values + * it was captured into are still marked as mutated (forward + * propagation is preserved), maintaining correctness for + * patterns like `s.add(arr); arr.push(x)`. Known mutating + * methods (e.g. Array.push) have explicit aliasing signatures + * that bypass this default path entirely. + */ + const isMethodReceiver = + operand === effect.receiver && + effect.receiver !== effect.function; applyEffect( context, state, { kind: 'MutateTransitiveConditionally', value: operand, + noBackwardRangeExtension: isMethodReceiver, }, initialized, effects, 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 b8c5eeaa8e23..3c5793d34d87 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts @@ -102,6 +102,7 @@ export function inferMutationAliasingRanges( kind: MutationKind; place: Place; reason: MutationReason | null; + noBackwardRangeExtension: boolean; }> = []; const renders: Array<{index: number; place: Place}> = []; @@ -179,6 +180,9 @@ export function inferMutationAliasingRanges( : MutationKind.Conditional, reason: null, place: effect.value, + noBackwardRangeExtension: + effect.kind === 'MutateTransitiveConditionally' && + (effect.noBackwardRangeExtension ?? false), }); } else if ( effect.kind === 'Mutate' || @@ -194,6 +198,7 @@ export function inferMutationAliasingRanges( : MutationKind.Conditional, reason: effect.kind === 'Mutate' ? (effect.reason ?? null) : null, place: effect.value, + noBackwardRangeExtension: false, }); } else if ( effect.kind === 'MutateFrozen' || @@ -246,6 +251,7 @@ export function inferMutationAliasingRanges( mutation.place.loc, mutation.reason, errors, + mutation.noBackwardRangeExtension, ); } for (const render of renders) { @@ -707,16 +713,85 @@ class AliasingState { loc: SourceLocation, reason: MutationReason | null, errors: CompilerError, + noBackwardRangeExtension: boolean = false, ): void { + /* + * Only skip backward-alias range extension when ALL of these hold: + * 1. The effect requested noBackwardRangeExtension (MethodCall receiver) + * 2. The start node is not a PropertyLoad result (no createdFrom edges), + * since PropertyLoad receivers need backward range extension for + * correctness (e.g. `x.y.push(val)` must extend x and y). + * 3. At least one of the start's backward alias targets was produced by + * a function call (indicated by having maybeAliases from the default + * Apply handler). This distinguishes `expensiveProcessing(data).map(fn)` + * from `localArray.push(val)` — only the former should skip, since the + * call result represents a separately-memoizable value. + */ + const startNode = this.nodes.get(start); + let effectiveSkipAliases = false; + if ( + noBackwardRangeExtension && + startNode != null && + startNode.createdFrom.size === 0 + ) { + /* + * Walk backward through aliases from the start node to check if + * any value in the chain was produced by a function call (Apply). + * Apply results are identifiable by having maybeAliases, since the + * default Apply handler records MaybeAlias from each argument to + * the result. Only walk through alias edges (not maybeAliases or + * captures) to avoid matching catch parameters and similar patterns + * where range extension is required for correctness. + */ + const visited = new Set(); + const aliasQueue: Array = [start]; + while (aliasQueue.length > 0) { + const id = aliasQueue.pop()!; + if (visited.has(id)) { + continue; + } + visited.add(id); + const node = this.nodes.get(id); + if (node == null) { + continue; + } + if (node.maybeAliases.size > 0) { + effectiveSkipAliases = true; + break; + } + for (const [nextAliasId] of node.aliases) { + aliasQueue.push(nextAliasId); + } + } + } const seen = new Map(); const queue: Array<{ place: Identifier; transitive: boolean; direction: 'backwards' | 'forwards'; kind: MutationKind; - }> = [{place: start, transitive, direction: 'backwards', kind: startKind}]; + /* + * When true, skip extending the mutable range for this node. This + * is set for nodes reached via backward alias/maybeAlias edges when + * effectiveSkipAliases is enabled, so that the receiver's backward + * aliases are not pulled into the same reactive scope as the method + * call. Nodes reached via capture edges reset this flag, since + * captures represent structural containment (e.g. function closures) + * where range extension is required for correctness. + */ + skipRangeExtension: boolean; + }> = [ + { + place: start, + transitive, + direction: 'backwards', + kind: startKind, + skipRangeExtension: false, + }, + ]; while (queue.length !== 0) { - const {place: current, transitive, direction, kind} = queue.pop()!; + const {place: current, transitive, direction, kind, skipRangeExtension} = + queue.pop()!; const previousKind = seen.get(current); if (previousKind != null && previousKind >= kind) { continue; @@ -728,7 +803,7 @@ class AliasingState { } node.mutationReason ??= reason; node.lastMutated = Math.max(node.lastMutated, index); - if (end != null) { + if (end != null && !skipRangeExtension) { node.id.mutableRange.end = makeInstructionId( Math.max(node.id.mutableRange.end, end), ); @@ -764,6 +839,7 @@ class AliasingState { direction: 'forwards', // Traversing a maybeAlias edge always downgrades to conditional mutation kind: edge.kind === 'maybeAlias' ? MutationKind.Conditional : kind, + skipRangeExtension: false, }); } for (const [alias, when] of node.createdFrom) { @@ -775,6 +851,8 @@ class AliasingState { transitive: true, direction: 'backwards', kind, + // Propagate parent's skipRangeExtension but don't initiate it + skipRangeExtension, }); } if (direction === 'backwards' || node.value.kind !== 'Phi') { @@ -797,6 +875,16 @@ class AliasingState { transitive, direction: 'backwards', kind, + /* + * Skip range extension when traversing backward through alias + * edges from a conditional method-call mutation. This prevents + * the receiver's creation-site values from having their ranges + * extended to cover the method call instruction, which would + * otherwise cause them to be merged into the same scope. + */ + skipRangeExtension: + skipRangeExtension || + (effectiveSkipAliases && current === start), }); } /** @@ -815,6 +903,9 @@ class AliasingState { transitive, direction: 'backwards', kind: MutationKind.Conditional, + skipRangeExtension: + skipRangeExtension || + (effectiveSkipAliases && current === start), }); } } @@ -831,6 +922,13 @@ class AliasingState { transitive, direction: 'backwards', kind, + /* + * Reset skipRangeExtension for capture edges. Captures + * represent structural containment (e.g. a function closing + * over a value), and the captured value's range must be + * extended to cover the mutation point for correctness. + */ + skipRangeExtension: false, }); } }