@samitouri / QOS-React-1 / commits / 808e7ed8e2

[compiler] Fix set-state-in-effect false negative with NewExpression default param (#36107)

## Summary Fixes #36101 When a component function has a destructured prop with a `NewExpression` default value (e.g. `{ value = new Number() }`), the React Compiler bails out during HIR construction when trying to lower the default value via `lowerReorderableExpression`. This causes `validateNoSetStateInEffects` to never run, silently suppressing the `set-state-in-effect` diagnostic. **Root cause:** `isReorderableExpression` did not have a case for `NewExpression`, so it fell through to the `default: return false` branch. `lowerReorderableExpression` then recorded a `Todo` error and aborted compilation of the function before any validation passes ran. **Fix:** Add a `NewExpression` case to `isReorderableExpression` that mirrors the existing `CallExpression` case — the expression is safe to reorder when the callee and all arguments are themselves reorderable (e.g. global identifiers and literals). ## How did you test this change? Added a new compiler fixture `invalid-setState-in-useEffect-new-expression-default-param` that reproduces the bug from the issue. The fixture verifies that the `EffectSetState` diagnostic is correctly emitted for a component with a `NewExpression` default prop value. All 1720 compiler snapshot tests pass.

Dmitrii committed Apr 8, 2026 at 19:52 UTC 808e7ed8e26c07dc15c088105673b639760477f9
3 files changed +70
compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts
+15
@@ -3268,6 +3268,21 @@ function isReorderableExpression(
3268 )
3269 );
3270 }
3271 + case 'NewExpression': {
3272 + const newExpr = expr as NodePath<t.NewExpression>;
3273 + const callee = newExpr.get('callee');
3274 + return (
3275 + callee.isExpression() &&
3276 + isReorderableExpression(builder, callee, allowLocalIdentifiers) &&
3277 + newExpr
3278 + .get('arguments')
3279 + .every(
3280 + arg =>
3281 + arg.isExpression() &&
3282 + isReorderableExpression(builder, arg, allowLocalIdentifiers),
3283 + )
3284 + );
3285 + }
3286 default: {
3287 return false;
3288 }
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-setState-in-useEffect-new-expression-default-param.expect.md new
+44
@@ -0,0 +1,44 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// @loggerTestOnly @validateNoSetStateInEffects @outputMode:"lint"
6 +import {useEffect, useState} from 'react';
7 +
8 +// Bug: NewExpression default param value should not prevent set-state-in-effect validation
9 +function Component({value = new Number()}) {
10 + const [state, setState] = useState(0);
11 + useEffect(() => {
12 + setState(s => s + 1);
13 + });
14 + return state;
15 +}
16 +
17 +```
18 +
19 +## Code
20 +
21 +```javascript
22 +// @loggerTestOnly @validateNoSetStateInEffects @outputMode:"lint"
23 +import { useEffect, useState } from "react";
24 +
25 +// Bug: NewExpression default param value should not prevent set-state-in-effect validation
26 +function Component({ value = new Number() }) {
27 + const [state, setState] = useState(0);
28 + useEffect(() => {
29 + setState((s) => s + 1);
30 + });
31 + return state;
32 +}
33 +
34 +```
35 +
36 +## Logs
37 +
38 +```
39 +{"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}
40 +{"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}
41 +```
42 +
43 +### Eval output
44 +(kind: exception) Fixture not implemented
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-setState-in-useEffect-new-expression-default-param.js new
+11
@@ -0,0 +1,11 @@
1 +// @loggerTestOnly @validateNoSetStateInEffects @outputMode:"lint"
2 +import {useEffect, useState} from 'react';
3 +
4 +// Bug: NewExpression default param value should not prevent set-state-in-effect validation
5 +function Component({value = new Number()}) {
6 + const [state, setState] = useState(0);
7 + useEffect(() => {
8 + setState(s => s + 1);
9 + });
10 + return state;
11 +}