[compiler] Don't outline functions that reference unbound module-local names

compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineFunctions.ts

@@ -27,7 +27,8 @@ export function outlineFunctions(
         value.loweredFunc.func.context.length === 0 &&
         // TODO: handle outlining named functions
         value.loweredFunc.func.id === null &&
-        !fbtOperands.has(lvalue.identifier.id)
+        !fbtOperands.has(lvalue.identifier.id) &&
+        !referencesUnboundModuleLocal(value.loweredFunc.func)
       ) {
         const loweredFunc = value.loweredFunc.func;
 
@@ -49,3 +50,40 @@ export function outlineFunctions(
     }
   }
 }
+
+/**
+ * Returns true if `fn` contains a `LoadGlobal(ModuleLocal)` whose name does
+ * not actually resolve at module/program scope.
+ *
+ * This guards against an upstream misclassification: when the outermost
+ * function being compiled is itself nested inside another function (e.g. a
+ * factory or HOC, as in `infer` compilation mode), `HIRBuilder` resolves
+ * identifiers relative to `parentFunction.scope.parent`, which is the
+ * enclosing function's scope rather than the program scope. As a result,
+ * references to the enclosing function's locals are tagged as `ModuleLocal`
+ * and surface here as `LoadGlobal` instructions with `context.length === 0`,
+ * making the function look outlineable. Hoisting it to module scope would
+ * leave the referenced name undefined at runtime.
+ *
+ * A proper fix lives in `HIRBuilder`/`BuildHIR` — classifying the binding
+ * correctly and threading factory-scope variables through the capture
+ * machinery so they appear in `fn.context`. That is a substantially larger
+ * change that affects how nested functions are lowered in general; this pass
+ * keeps the function inline as a targeted, conservative workaround.
+ */
+function referencesUnboundModuleLocal(fn: HIRFunction): boolean {
+  const programScope = fn.env.parentFunction.scope.getProgramParent();
+  for (const [, block] of fn.body.blocks) {
+    for (const instr of block.instructions) {
+      const {value} = instr;
+      if (
+        value.kind === 'LoadGlobal' &&
+        value.binding.kind === 'ModuleLocal' &&
+        programScope.getBinding(value.binding.name) == null
+      ) {
+        return true;
+      }
+    }
+  }
+  return false;
+}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/factory-function-component-captures-outer-scope.expect.md

@@ -0,0 +1,85 @@
+
+## Input
+
+```javascript
+// @compilationMode:"infer"
+/**
+ * Regression test for https://github.com/facebook/react/issues/34901
+ *
+ * When `infer` mode compiles a component defined inside an enclosing
+ * function (e.g. a factory or HOC), inner callbacks that capture
+ * variables from the enclosing scope must NOT be hoisted to module
+ * scope by `outlineFunctions`. The enclosing scope's locals are
+ * mis-tagged as `ModuleLocal` by `HIRBuilder` (it uses
+ * `parentFunction.scope.parent` as a proxy for the module scope),
+ * so `getCount`'s context appeared empty and outlining would emit a
+ * top-level `_temp` function referencing an undefined `counter` at
+ * runtime.
+ */
+function makeCounter(initialValue) {
+  const counter = {value: initialValue};
+
+  const Counter = () => {
+    const getCount = () => counter.value;
+    return <div>{getCount()}</div>;
+  };
+
+  return Counter;
+}
+
+const Counter = makeCounter(42);
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Counter,
+  params: [],
+};
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime"; // @compilationMode:"infer"
+/**
+ * Regression test for https://github.com/facebook/react/issues/34901
+ *
+ * When `infer` mode compiles a component defined inside an enclosing
+ * function (e.g. a factory or HOC), inner callbacks that capture
+ * variables from the enclosing scope must NOT be hoisted to module
+ * scope by `outlineFunctions`. The enclosing scope's locals are
+ * mis-tagged as `ModuleLocal` by `HIRBuilder` (it uses
+ * `parentFunction.scope.parent` as a proxy for the module scope),
+ * so `getCount`'s context appeared empty and outlining would emit a
+ * top-level `_temp` function referencing an undefined `counter` at
+ * runtime.
+ */
+function makeCounter(initialValue) {
+  const counter = { value: initialValue };
+
+  const Counter = () => {
+    const $ = _c(1);
+    let t0;
+    if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
+      const getCount = () => counter.value;
+      t0 = <div>{getCount()}</div>;
+      $[0] = t0;
+    } else {
+      t0 = $[0];
+    }
+    return t0;
+  };
+
+  return Counter;
+}
+
+const Counter = makeCounter(42);
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Counter,
+  params: [],
+};
+
+```
+
+### Eval output
+(kind: ok) <div>42</div>

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/factory-function-component-captures-outer-scope.js

@@ -0,0 +1,31 @@
+// @compilationMode:"infer"
+/**
+ * Regression test for https://github.com/facebook/react/issues/34901
+ *
+ * When `infer` mode compiles a component defined inside an enclosing
+ * function (e.g. a factory or HOC), inner callbacks that capture
+ * variables from the enclosing scope must NOT be hoisted to module
+ * scope by `outlineFunctions`. The enclosing scope's locals are
+ * mis-tagged as `ModuleLocal` by `HIRBuilder` (it uses
+ * `parentFunction.scope.parent` as a proxy for the module scope),
+ * so `getCount`'s context appeared empty and outlining would emit a
+ * top-level `_temp` function referencing an undefined `counter` at
+ * runtime.
+ */
+function makeCounter(initialValue) {
+  const counter = {value: initialValue};
+
+  const Counter = () => {
+    const getCount = () => counter.value;
+    return <div>{getCount()}</div>;
+  };
+
+  return Counter;
+}
+
+const Counter = makeCounter(42);
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Counter,
+  params: [],
+};