[pull] main from facebook:main

compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts

@@ -3268,6 +3268,21 @@ function isReorderableExpression(
           )
       );
     }
+    case 'NewExpression': {
+      const newExpr = expr as NodePath<t.NewExpression>;
+      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;
     }

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

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;
+}

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<any>): Set<any> {
   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<any>): Iterator<any> {
   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;
 }
 

scripts/rollup/build.js

@@ -453,7 +453,8 @@ function getPlugins(
             globalName,
             filename,
             moduleType,
-            bundle.wrapWithModuleBoundaries
+            bundle.wrapWithModuleBoundaries,
+            bundle.wrapWithNodeDevGuard
           );
         },
       },

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',

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}.`);
   }