From 257b033fc7b0518e7b1db32ca24e2354933b9d0e Mon Sep 17 00:00:00 2001 From: Jorge Cabiedes <57368278+jorge-cab@users.noreply.github.com> Date: Thu, 13 Nov 2025 22:56:06 -0800 Subject: [PATCH] [Compiler] Avoid capturing global setStates for no-derived-computations lint (#35135) Summary: This only matters when enableTreatSetIdentifiersAsStateSetters=true This pattern is still bad. But Right now the validation can only recommend to move stuff to "calculate in render" A global setState should not be moved to render, not even conditionally and you can't remove state without crossing Component boundaries, which makes this a different kind of fix. So while we are only suggesting "calculate in render" as a fix we should disallow the lint from throwing in this case IMO Test Plan: Added a fixture --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35135). * __->__ #35135 * #35134 --- ...idateNoDerivedComputationsInEffects_exp.ts | 12 ++++ ...rops-setstate-in-effect-no-error.expect.md | 65 +++++++++++++++++++ .../from-props-setstate-in-effect-no-error.js | 9 +++ ...m-prop-no-show-in-data-flow-tree.expect.md | 1 - 4 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/from-props-setstate-in-effect-no-error.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/from-props-setstate-in-effect-no-error.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects_exp.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects_exp.ts index 096e1ce1a0..af5927548a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects_exp.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects_exp.ts @@ -690,6 +690,18 @@ function validateEffect( instr.value.args.length === 1 && instr.value.args[0].kind === 'Identifier' ) { + const calleeMetadata = context.derivationCache.cache.get( + instr.value.callee.identifier.id, + ); + + /* + * If the setState comes from a source other than local state skip + * since the fix is not to calculate in render + */ + if (calleeMetadata?.typeOfValue != 'fromState') { + continue; + } + const argMetadata = context.derivationCache.cache.get( instr.value.args[0].identifier.id, ); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/from-props-setstate-in-effect-no-error.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/from-props-setstate-in-effect-no-error.expect.md new file mode 100644 index 0000000000..f23f51d6cb --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/from-props-setstate-in-effect-no-error.expect.md @@ -0,0 +1,65 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects_exp @enableTreatSetIdentifiersAsStateSetters @loggerTestOnly + +function Component({setParentState, prop}) { + useEffect(() => { + setParentState(prop); + }, [prop]); + + return
{prop}
; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp @enableTreatSetIdentifiersAsStateSetters @loggerTestOnly + +function Component(t0) { + const $ = _c(7); + const { setParentState, prop } = t0; + let t1; + if ($[0] !== prop || $[1] !== setParentState) { + t1 = () => { + setParentState(prop); + }; + $[0] = prop; + $[1] = setParentState; + $[2] = t1; + } else { + t1 = $[2]; + } + let t2; + if ($[3] !== prop) { + t2 = [prop]; + $[3] = prop; + $[4] = t2; + } else { + t2 = $[4]; + } + useEffect(t1, t2); + let t3; + if ($[5] !== prop) { + t3 =
{prop}
; + $[5] = prop; + $[6] = t3; + } else { + t3 = $[6]; + } + return t3; +} + +``` + +## Logs + +``` +{"kind":"CompileSuccess","fnLoc":{"start":{"line":3,"column":0,"index":105},"end":{"line":9,"column":1,"index":240},"filename":"from-props-setstate-in-effect-no-error.ts"},"fnName":"Component","memoSlots":7,"memoBlocks":3,"memoValues":3,"prunedMemoBlocks":0,"prunedMemoValues":0} +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/from-props-setstate-in-effect-no-error.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/from-props-setstate-in-effect-no-error.js new file mode 100644 index 0000000000..4075975b32 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/from-props-setstate-in-effect-no-error.js @@ -0,0 +1,9 @@ +// @validateNoDerivedComputationsInEffects_exp @enableTreatSetIdentifiersAsStateSetters @loggerTestOnly + +function Component({setParentState, prop}) { + useEffect(() => { + setParentState(prop); + }, [prop]); + + return
{prop}
; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/usestate-derived-from-prop-no-show-in-data-flow-tree.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/usestate-derived-from-prop-no-show-in-data-flow-tree.expect.md index 87cf7722da..602fb9fff3 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/usestate-derived-from-prop-no-show-in-data-flow-tree.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/usestate-derived-from-prop-no-show-in-data-flow-tree.expect.md @@ -64,7 +64,6 @@ function Component(t0) { ## Logs ``` -{"kind":"CompileError","detail":{"options":{"description":"Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user\n\nThis setState call is setting a derived value that depends on the following reactive sources:\n\nState: [second]\n\nData Flow Tree:\nā””ā”€ā”€ second (State)\n\nSee: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state","category":"EffectDerivationsOfState","reason":"You might not need an effect. Derive values in render, not effects.","details":[{"kind":"error","loc":{"start":{"line":14,"column":4,"index":443},"end":{"line":14,"column":8,"index":447},"filename":"usestate-derived-from-prop-no-show-in-data-flow-tree.ts","identifierName":"setS"},"message":"This should be computed during render, not in an effect"}]}},"fnLoc":null} {"kind":"CompileSuccess","fnLoc":{"start":{"line":3,"column":0,"index":64},"end":{"line":18,"column":1,"index":500},"filename":"usestate-derived-from-prop-no-show-in-data-flow-tree.ts"},"fnName":"Component","memoSlots":5,"memoBlocks":2,"memoValues":3,"prunedMemoBlocks":0,"prunedMemoValues":0} ```