From 808e7ed8e26c07dc15c088105673b639760477f9 Mon Sep 17 00:00:00 2001 From: Dmitrii Date: Wed, 8 Apr 2026 19:52:49 +0100 Subject: [PATCH 1/3] [compiler] Fix set-state-in-effect false negative with NewExpression default param (#36107) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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. --- .../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; +} From 404b38c764cf86e6f2ec42f873bb33ce114256d3 Mon Sep 17 00:00:00 2001 From: "Sebastian \"Sebbie\" Silbermann" Date: Wed, 8 Apr 2026 21:01:27 +0200 Subject: [PATCH 2/3] [Flight] Add more cycle protections (#36236) --- packages/react-server/src/ReactFlightReplyServer.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/react-server/src/ReactFlightReplyServer.js b/packages/react-server/src/ReactFlightReplyServer.js index 5f58e918f3e8..34a4942ce076 100644 --- a/packages/react-server/src/ReactFlightReplyServer.js +++ b/packages/react-server/src/ReactFlightReplyServer.js @@ -1123,8 +1123,9 @@ function createMap( if ((model as any).$$consumed === true) { throw new Error('Already initialized Map.'); } - const map = new Map(model); + // This needs to come first to prevent the model from being consumed again in case of a cyclic reference. (model as any).$$consumed = true; + const map = new Map(model); return map; } @@ -1135,8 +1136,9 @@ function createSet(response: Response, model: Array): Set { if ((model as any).$$consumed === true) { throw new Error('Already initialized Set.'); } - const set = new Set(model); + // This needs to come first to prevent the model from being consumed again in case of a cyclic reference. (model as any).$$consumed = true; + const set = new Set(model); return set; } @@ -1147,9 +1149,10 @@ function extractIterator(response: Response, model: Array): Iterator { if ((model as any).$$consumed === true) { throw new Error('Already initialized Iterator.'); } + // This needs to come first to prevent the model from being consumed again in case of a cyclic reference. + (model as any).$$consumed = true; // $FlowFixMe[incompatible-use]: This uses raw Symbols because we're extracting from a native array. const iterator = model[Symbol.iterator](); - (model as any).$$consumed = true; return iterator; } From 733d3aaf99e30627ec25174da9d39efbaa97dba3 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Wed, 8 Apr 2026 14:12:35 -0600 Subject: [PATCH 3/3] Fix FB_WWW eprh bundle dev guard (#36238) We use FB_WWW bundle to inject internal feature flag values, but need to use NODE guard type because this is a node script -- __DEV__ is breaking internal builds Follow up to https://github.com/facebook/react/pull/35951 --- scripts/rollup/build.js | 3 ++- scripts/rollup/bundles.js | 5 ++++- scripts/rollup/wrappers.js | 11 +++++++++-- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/scripts/rollup/build.js b/scripts/rollup/build.js index 66b74b436349..087c2e6a43b9 100644 --- a/scripts/rollup/build.js +++ b/scripts/rollup/build.js @@ -453,7 +453,8 @@ function getPlugins( globalName, filename, moduleType, - bundle.wrapWithModuleBoundaries + bundle.wrapWithModuleBoundaries, + bundle.wrapWithNodeDevGuard ); }, }, diff --git a/scripts/rollup/bundles.js b/scripts/rollup/bundles.js index dbf6160a847e..7b9508b9e29c 100644 --- a/scripts/rollup/bundles.js +++ b/scripts/rollup/bundles.js @@ -1235,12 +1235,15 @@ const bundles = [ // currently required in order for the package to be copied over correctly. // So, it would be worth improving that flow. name: 'eslint-plugin-react-hooks', - bundleTypes: [NODE_DEV, NODE_PROD, FB_WWW_DEV, FB_WWW_PROD, CJS_DTS], + bundleTypes: [NODE_DEV, NODE_PROD, FB_WWW_DEV, CJS_DTS], moduleType: ISOMORPHIC, entry: 'eslint-plugin-react-hooks/src/index.ts', global: 'ESLintPluginReactHooks', minifyWithProdErrorCodes: false, wrapWithModuleBoundaries: false, + // This is a Node.js build tool (ESLint plugin), not a www runtime bundle. + // Use process.env.NODE_ENV guard instead of __DEV__ for the dev wrapper. + wrapWithNodeDevGuard: true, preferBuiltins: true, externals: [ '@babel/core', diff --git a/scripts/rollup/wrappers.js b/scripts/rollup/wrappers.js index 1773253cc965..52ae3b21252d 100644 --- a/scripts/rollup/wrappers.js +++ b/scripts/rollup/wrappers.js @@ -510,7 +510,8 @@ function wrapWithTopLevelDefinitions( globalName, filename, moduleType, - wrapWithModuleBoundaries + wrapWithModuleBoundaries, + wrapWithNodeDevGuard ) { if (wrapWithModuleBoundaries) { switch (bundleType) { @@ -553,8 +554,14 @@ function wrapWithTopLevelDefinitions( return wrapper(source, globalName, filename, moduleType); } + // Node.js build tools (e.g. ESLint plugins) use process.env.NODE_ENV instead + // of __DEV__ even when building for FB_WWW, since they run in Node.js where + // __DEV__ is not defined. + const effectiveBundleType = + wrapWithNodeDevGuard && bundleType === FB_WWW_DEV ? NODE_DEV : bundleType; + // All the other packages. - const wrapper = topLevelDefinitionWrappers[bundleType]; + const wrapper = topLevelDefinitionWrappers[effectiveBundleType]; if (typeof wrapper !== 'function') { throw new Error(`Unsupported build type: ${bundleType}.`); }