From 87970668a963a9baf856def904191a78cab59338 Mon Sep 17 00:00:00 2001 From: Dmitrii Troitskii Date: Thu, 19 Mar 2026 18:39:16 +0000 Subject: [PATCH] [compiler] Fix set-state-in-effect false negative with NewExpression default param MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a component function has a destructured prop with a NewExpression default value (e.g. `{ value = new Number() }`), the compiler would bail out during BuildHIR when trying to lower the default value via `lowerReorderableExpression`. This caused `validateNoSetStateInEffects` to never run, silently suppressing the set-state-in-effect diagnostic. The fix adds a 'NewExpression' case to `isReorderableExpression`, treating it the same as 'CallExpression' — safe to reorder when the callee and all arguments are themselves reorderable (e.g., global identifiers and literals). Fixes #36101 --- .../src/HIR/BuildHIR.ts | 15 +++++++ ...ect-new-expression-default-param.expect.md | 44 +++++++++++++++++++ ...-useEffect-new-expression-default-param.js | 11 +++++ 3 files changed, 70 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-setState-in-useEffect-new-expression-default-param.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-setState-in-useEffect-new-expression-default-param.js 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 8f44594c0031..452aa0ce329d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts @@ -3268,6 +3268,21 @@ function isReorderableExpression( ) ); } + case 'NewExpression': { + const newExpr = expr as NodePath; + const callee = newExpr.get('callee'); + return ( + callee.isExpression() && + isReorderableExpression(builder, callee, allowLocalIdentifiers) && + newExpr + .get('arguments') + .every( + arg => + arg.isExpression() && + isReorderableExpression(builder, arg, allowLocalIdentifiers), + ) + ); + } default: { return false; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-setState-in-useEffect-new-expression-default-param.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-setState-in-useEffect-new-expression-default-param.expect.md new file mode 100644 index 000000000000..5f2f3619b88f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-setState-in-useEffect-new-expression-default-param.expect.md @@ -0,0 +1,44 @@ + +## Input + +```javascript +// @loggerTestOnly @validateNoSetStateInEffects @outputMode:"lint" +import {useEffect, useState} from 'react'; + +// Bug: NewExpression default param value should not prevent set-state-in-effect validation +function Component({value = new Number()}) { + const [state, setState] = useState(0); + useEffect(() => { + setState(s => s + 1); + }); + return state; +} + +``` + +## Code + +```javascript +// @loggerTestOnly @validateNoSetStateInEffects @outputMode:"lint" +import { useEffect, useState } from "react"; + +// Bug: NewExpression default param value should not prevent set-state-in-effect validation +function Component({ value = new Number() }) { + const [state, setState] = useState(0); + useEffect(() => { + setState((s) => s + 1); + }); + return state; +} + +``` + +## Logs + +``` +{"kind":"CompileError","detail":{"options":{"category":"EffectSetState","reason":"Calling setState synchronously within an effect can trigger cascading renders","description":"Effects are intended to synchronize state between React and external systems such as manually updating the DOM, state management libraries, or other platform APIs. In general, the body of an effect should do one or both of the following:\n* Update external systems with the latest state from React.\n* Subscribe for updates from some external system, calling setState in a callback function when external state changes.\n\nCalling setState synchronously within an effect body causes cascading renders that can hurt performance, and is not recommended. (https://react.dev/learn/you-might-not-need-an-effect)","suggestions":null,"details":[{"kind":"error","loc":{"start":{"line":8,"column":4,"index":313},"end":{"line":8,"column":12,"index":321},"filename":"invalid-setState-in-useEffect-new-expression-default-param.ts","identifierName":"setState"},"message":"Avoid calling setState() directly within an effect"}]}},"fnLoc":null} +{"kind":"CompileSuccess","fnLoc":{"start":{"line":5,"column":0,"index":203},"end":{"line":11,"column":1,"index":358},"filename":"invalid-setState-in-useEffect-new-expression-default-param.ts"},"fnName":"Component","memoSlots":1,"memoBlocks":1,"memoValues":1,"prunedMemoBlocks":1,"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/invalid-setState-in-useEffect-new-expression-default-param.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-setState-in-useEffect-new-expression-default-param.js new file mode 100644 index 000000000000..a239f743a6cc --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-setState-in-useEffect-new-expression-default-param.js @@ -0,0 +1,11 @@ +// @loggerTestOnly @validateNoSetStateInEffects @outputMode:"lint" +import {useEffect, useState} from 'react'; + +// Bug: NewExpression default param value should not prevent set-state-in-effect validation +function Component({value = new Number()}) { + const [state, setState] = useState(0); + useEffect(() => { + setState(s => s + 1); + }); + return state; +}