From 2cb08e65b39d1bd184803cc8e4d1081f9c801f69 Mon Sep 17 00:00:00 2001
From: Joseph Savona <6425824+josephsavona@users.noreply.github.com>
Date: Fri, 5 Dec 2025 11:29:06 -0800
Subject: [PATCH] [compiler] Fix bug w functions depending on hoisted
 primitives (#35284)

Fixes an edge case where a function expression would fail to take a
dependency if it referenced a hoisted `const` inferred as a primitive
value. We were incorrectly skipping primitve-typed operands when
determing scopes for merging in InferReactiveScopeVariables.

This was super tricky to debug, for posterity the trick is that Context
variables (StoreContext etc) are modeled just like a mutable object,
where assignment to the variable is equivalent to `object.value = ...`
and reading the variable is equivalent to `object.value` property
access. Comparing to an equivalent version of the repro case replaced
with an object and property read/writes showed that everything was
exactly right, except that InferReactiveScopeVariables wasn't merging
the scopes of the function and the context variable, which led me right
to the problematic line.

Closes #35122
---
 .../InferReactiveScopeVariables.ts            |  8 --
 ...-stale-closure-forward-reference.expect.md | 94 +++++++++++++++++++
 .../repro-stale-closure-forward-reference.js  | 32 +++++++
 3 files changed, 126 insertions(+), 8 deletions(-)
 create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-stale-closure-forward-reference.expect.md
 create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-stale-closure-forward-reference.js

diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts
index 300115c04e4..b034ee893d0 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts
+++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts
@@ -389,14 +389,6 @@ export function findDisjointMutableValues(
              */
             operand.identifier.mutableRange.start > 0
           ) {
-            if (
-              instr.value.kind === 'FunctionExpression' ||
-              instr.value.kind === 'ObjectMethod'
-            ) {
-              if (operand.identifier.type.kind === 'Primitive') {
-                continue;
-              }
-            }
             operands.push(operand.identifier);
           }
         }
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-stale-closure-forward-reference.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-stale-closure-forward-reference.expect.md
new file mode 100644
index 00000000000..01d1fe688ff
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-stale-closure-forward-reference.expect.md
@@ -0,0 +1,94 @@
+
+## Input
+
+```javascript
+import {useState} from 'react';
+
+/**
+ * Repro for https://github.com/facebook/react/issues/35122
+ *
+ * InferReactiveScopeVariables was excluding primitive operands
+ * when considering operands for merging. We previously did not
+ * infer types for context variables (StoreContext etc), but later
+ * started inferring types in cases of `const` context variables,
+ * since the type cannot change.
+ *
+ * In this example, this meant that we skipped the `isExpired`
+ * operand of the onClick function expression when considering
+ * scopes to merge.
+ */
+function Test1() {
+  const [expire, setExpire] = useState(5);
+
+  const onClick = () => {
+    // Reference to isExpired prior to declaration
+    console.log('isExpired', isExpired);
+  };
+
+  const isExpired = expire === 0;
+
+  return <div onClick={onClick}>{expire}</div>;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Test1,
+  params: [{}],
+};
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime";
+import { useState } from "react";
+
+/**
+ * Repro for https://github.com/facebook/react/issues/35122
+ *
+ * InferReactiveScopeVariables was excluding primitive operands
+ * when considering operands for merging. We previously did not
+ * infer types for context variables (StoreContext etc), but later
+ * started inferring types in cases of `const` context variables,
+ * since the type cannot change.
+ *
+ * In this example, this meant that we skipped the `isExpired`
+ * operand of the onClick function expression when considering
+ * scopes to merge.
+ */
+function Test1() {
+  const $ = _c(5);
+  const [expire] = useState(5);
+  let onClick;
+  if ($[0] !== expire) {
+    onClick = () => {
+      console.log("isExpired", isExpired);
+    };
+
+    const isExpired = expire === 0;
+    $[0] = expire;
+    $[1] = onClick;
+  } else {
+    onClick = $[1];
+  }
+  let t0;
+  if ($[2] !== expire || $[3] !== onClick) {
+    t0 = <div onClick={onClick}>{expire}</div>;
+    $[2] = expire;
+    $[3] = onClick;
+    $[4] = t0;
+  } else {
+    t0 = $[4];
+  }
+  return t0;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Test1,
+  params: [{}],
+};
+
+```
+      
+### Eval output
+(kind: ok) <div>5</div>
\ No newline at end of file
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-stale-closure-forward-reference.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-stale-closure-forward-reference.js
new file mode 100644
index 00000000000..035640fb6ba
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-stale-closure-forward-reference.js
@@ -0,0 +1,32 @@
+import {useState} from 'react';
+
+/**
+ * Repro for https://github.com/facebook/react/issues/35122
+ *
+ * InferReactiveScopeVariables was excluding primitive operands
+ * when considering operands for merging. We previously did not
+ * infer types for context variables (StoreContext etc), but later
+ * started inferring types in cases of `const` context variables,
+ * since the type cannot change.
+ *
+ * In this example, this meant that we skipped the `isExpired`
+ * operand of the onClick function expression when considering
+ * scopes to merge.
+ */
+function Test1() {
+  const [expire, setExpire] = useState(5);
+
+  const onClick = () => {
+    // Reference to isExpired prior to declaration
+    console.log('isExpired', isExpired);
+  };
+
+  const isExpired = expire === 0;
+
+  return <div onClick={onClick}>{expire}</div>;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Test1,
+  params: [{}],
+};