From 125581ed9ab2c6a6c45936e4000ed904758f3e5b Mon Sep 17 00:00:00 2001
From: Mofei Zhang <feifei0@meta.com>
Date: Fri, 15 Nov 2024 12:02:15 -0500
Subject: [PATCH 1/5] [compiler] Fix: ref.current now correctly reactive

We were previously filtering out `ref.current` dependencies in propagateScopeDependencies:checkValidDependency`. This is incorrect.

Instead, we now always take a dependency on ref values (the outer box) as they may be reactive. Pruning is done in pruneNonReactiveDependencies.

This PR includes a small patch to `collectReactiveIdentifier`. Prior to this, we conservatively assumed that pruned scopes always produced reactive declarations. This assumption fixed a bug with non-reactivity, but some of these declarations are `useRef` calls. Now we have special handling for this case
```js
// This often produces a pruned scope
React.useRef(1);
```
---
 .../src/HIR/PropagateScopeDependenciesHIR.ts  |  18 ++--
 .../CollectReactiveIdentifiers.ts             |  14 ++-
 .../compiler/bug-nonreactive-ref.expect.md    |  89 ---------------
 .../fixtures/compiler/bug-nonreactive-ref.tsx |  33 ------
 .../compiler/reactive-ref-param.expect.md     | 102 ++++++++++++++++++
 .../fixtures/compiler/reactive-ref-param.tsx  |  29 +++++
 .../fixtures/compiler/reactive-ref.expect.md  |  79 ++++++++++++++
 .../fixtures/compiler/reactive-ref.tsx        |  22 ++++
 .../ref-parameter-mutate-in-effect.expect.md  |  30 +++---
 .../ref-parameter-mutate-in-effect.js         |   2 +-
 .../packages/snap/src/SproutTodoFilter.ts     |   1 -
 11 files changed, 272 insertions(+), 147 deletions(-)
 delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-nonreactive-ref.expect.md
 delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-nonreactive-ref.tsx
 create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-ref-param.expect.md
 create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-ref-param.tsx
 create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-ref.expect.md
 create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-ref.tsx

diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts
index bbec25a57cc0c..7fd44c29dccf6 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts
+++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts
@@ -440,14 +440,6 @@ class Context {
 
   // Checks if identifier is a valid dependency in the current scope
   #checkValidDependency(maybeDependency: ReactiveScopeDependency): boolean {
-    // ref.current access is not a valid dep
-    if (
-      isUseRefType(maybeDependency.identifier) &&
-      maybeDependency.path.at(0)?.property === 'current'
-    ) {
-      return false;
-    }
-
     // ref value is not a valid dep
     if (isRefValueType(maybeDependency.identifier)) {
       return false;
@@ -549,6 +541,16 @@ class Context {
       });
     }
 
+    // ref.current access is not a valid dep
+    if (
+      isUseRefType(maybeDependency.identifier) &&
+      maybeDependency.path.at(0)?.property === 'current'
+    ) {
+      maybeDependency = {
+        identifier: maybeDependency.identifier,
+        path: [],
+      };
+    }
     if (this.#checkValidDependency(maybeDependency)) {
       this.#dependencies.value!.push(maybeDependency);
     }
diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CollectReactiveIdentifiers.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CollectReactiveIdentifiers.ts
index 610e93965fc1a..385123400581f 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CollectReactiveIdentifiers.ts
+++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CollectReactiveIdentifiers.ts
@@ -12,6 +12,8 @@ import {
   PrunedReactiveScopeBlock,
   ReactiveFunction,
   isPrimitiveType,
+  isUseRefType,
+  Identifier,
 } from '../HIR/HIR';
 import {ReactiveFunctionVisitor, visitReactiveFunction} from './visitors';
 
@@ -50,13 +52,21 @@ class Visitor extends ReactiveFunctionVisitor<Set<IdentifierId>> {
     this.traversePrunedScope(scopeBlock, state);
 
     for (const [id, decl] of scopeBlock.scope.declarations) {
-      if (!isPrimitiveType(decl.identifier)) {
+      if (
+        !isPrimitiveType(decl.identifier) &&
+        !isStableRefType(decl.identifier, state)
+      ) {
         state.add(id);
       }
     }
   }
 }
-
+function isStableRefType(
+  identifier: Identifier,
+  reactiveIdentifiers: Set<IdentifierId>,
+): boolean {
+  return isUseRefType(identifier) && !reactiveIdentifiers.has(identifier.id);
+}
 /*
  * Computes a set of identifiers which are reactive, using the analysis previously performed
  * in `InferReactivePlaces`.
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-nonreactive-ref.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-nonreactive-ref.expect.md
deleted file mode 100644
index f79df15d52492..0000000000000
--- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-nonreactive-ref.expect.md
+++ /dev/null
@@ -1,89 +0,0 @@
-
-## Input
-
-```javascript
-import {useRef} from 'react';
-import {Stringify} from 'shared-runtime';
-
-/**
- * Bug: we're currently filtering out `ref.current` dependencies in
- * `propagateScopeDependencies:checkValidDependency`. This is incorrect.
- * Instead, we should always take a dependency on ref values (the outer box) as
- * they may be reactive. Pruning should be done in
- * `pruneNonReactiveDependencies`
- *
- * Found differences in evaluator results
- * Non-forget (expected):
- *   (kind: ok)
- *   <div>{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
- *   <div>{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}</div>
- * Forget:
- *   (kind: ok)
- *   <div>{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
- *   <div>{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
- */
-function Component({cond}) {
-  const ref1 = useRef(1);
-  const ref2 = useRef(2);
-  const ref = cond ? ref1 : ref2;
-  const cb = () => ref.current;
-  return <Stringify cb={cb} shouldInvokeFns={true} />;
-}
-
-export const FIXTURE_ENTRYPOINT = {
-  fn: Component,
-  params: [{cond: true}],
-  sequentialRenders: [{cond: true}, {cond: false}],
-};
-
-```
-
-## Code
-
-```javascript
-import { c as _c } from "react/compiler-runtime";
-import { useRef } from "react";
-import { Stringify } from "shared-runtime";
-
-/**
- * Bug: we're currently filtering out `ref.current` dependencies in
- * `propagateScopeDependencies:checkValidDependency`. This is incorrect.
- * Instead, we should always take a dependency on ref values (the outer box) as
- * they may be reactive. Pruning should be done in
- * `pruneNonReactiveDependencies`
- *
- * Found differences in evaluator results
- * Non-forget (expected):
- *   (kind: ok)
- *   <div>{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
- *   <div>{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}</div>
- * Forget:
- *   (kind: ok)
- *   <div>{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
- *   <div>{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
- */
-function Component(t0) {
-  const $ = _c(1);
-  const { cond } = t0;
-  const ref1 = useRef(1);
-  const ref2 = useRef(2);
-  const ref = cond ? ref1 : ref2;
-  let t1;
-  if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
-    const cb = () => ref.current;
-    t1 = <Stringify cb={cb} shouldInvokeFns={true} />;
-    $[0] = t1;
-  } else {
-    t1 = $[0];
-  }
-  return t1;
-}
-
-export const FIXTURE_ENTRYPOINT = {
-  fn: Component,
-  params: [{ cond: true }],
-  sequentialRenders: [{ cond: true }, { cond: false }],
-};
-
-```
-      
\ No newline at end of file
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-nonreactive-ref.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-nonreactive-ref.tsx
deleted file mode 100644
index a0115f2df3cfb..0000000000000
--- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-nonreactive-ref.tsx
+++ /dev/null
@@ -1,33 +0,0 @@
-import {useRef} from 'react';
-import {Stringify} from 'shared-runtime';
-
-/**
- * Bug: we're currently filtering out `ref.current` dependencies in
- * `propagateScopeDependencies:checkValidDependency`. This is incorrect.
- * Instead, we should always take a dependency on ref values (the outer box) as
- * they may be reactive. Pruning should be done in
- * `pruneNonReactiveDependencies`
- *
- * Found differences in evaluator results
- * Non-forget (expected):
- *   (kind: ok)
- *   <div>{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
- *   <div>{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}</div>
- * Forget:
- *   (kind: ok)
- *   <div>{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
- *   <div>{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
- */
-function Component({cond}) {
-  const ref1 = useRef(1);
-  const ref2 = useRef(2);
-  const ref = cond ? ref1 : ref2;
-  const cb = () => ref.current;
-  return <Stringify cb={cb} shouldInvokeFns={true} />;
-}
-
-export const FIXTURE_ENTRYPOINT = {
-  fn: Component,
-  params: [{cond: true}],
-  sequentialRenders: [{cond: true}, {cond: false}],
-};
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-ref-param.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-ref-param.expect.md
new file mode 100644
index 0000000000000..c8922ab0c46fa
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-ref-param.expect.md
@@ -0,0 +1,102 @@
+
+## Input
+
+```javascript
+import {useRef, forwardRef} from 'react';
+import {Stringify} from 'shared-runtime';
+
+/**
+ * Fixture showing that Ref types may be reactive.
+ * We should always take a dependency on ref values (the outer box) as
+ * they may be reactive. Pruning should be done in
+ * `pruneNonReactiveDependencies`
+ */
+
+function Parent({cond}) {
+  const ref1 = useRef(1);
+  const ref2 = useRef(2);
+  const ref = cond ? ref1 : ref2;
+  return <Child ref={ref} />;
+}
+
+function ChildImpl(_props, ref) {
+  const cb = () => ref.current;
+  return <Stringify cb={cb} shouldInvokeFns={true} />;
+}
+
+const Child = forwardRef(ChildImpl);
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Parent,
+  params: [{cond: true}],
+  sequentialRenders: [{cond: true}, {cond: false}],
+};
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime";
+import { useRef, forwardRef } from "react";
+import { Stringify } from "shared-runtime";
+
+/**
+ * Fixture showing that Ref types may be reactive.
+ * We should always take a dependency on ref values (the outer box) as
+ * they may be reactive. Pruning should be done in
+ * `pruneNonReactiveDependencies`
+ */
+
+function Parent(t0) {
+  const $ = _c(2);
+  const { cond } = t0;
+  const ref1 = useRef(1);
+  const ref2 = useRef(2);
+  const ref = cond ? ref1 : ref2;
+  let t1;
+  if ($[0] !== ref) {
+    t1 = <Child ref={ref} />;
+    $[0] = ref;
+    $[1] = t1;
+  } else {
+    t1 = $[1];
+  }
+  return t1;
+}
+
+function ChildImpl(_props, ref) {
+  const $ = _c(4);
+  let t0;
+  if ($[0] !== ref) {
+    t0 = () => ref.current;
+    $[0] = ref;
+    $[1] = t0;
+  } else {
+    t0 = $[1];
+  }
+  const cb = t0;
+  let t1;
+  if ($[2] !== cb) {
+    t1 = <Stringify cb={cb} shouldInvokeFns={true} />;
+    $[2] = cb;
+    $[3] = t1;
+  } else {
+    t1 = $[3];
+  }
+  return t1;
+}
+
+const Child = forwardRef(ChildImpl);
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Parent,
+  params: [{ cond: true }],
+  sequentialRenders: [{ cond: true }, { cond: false }],
+};
+
+```
+      
+### Eval output
+(kind: ok) <div>{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
+<div>{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}</div>
\ No newline at end of file
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-ref-param.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-ref-param.tsx
new file mode 100644
index 0000000000000..c3eea7a24379f
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-ref-param.tsx
@@ -0,0 +1,29 @@
+import {useRef, forwardRef} from 'react';
+import {Stringify} from 'shared-runtime';
+
+/**
+ * Fixture showing that Ref types may be reactive.
+ * We should always take a dependency on ref values (the outer box) as
+ * they may be reactive. Pruning should be done in
+ * `pruneNonReactiveDependencies`
+ */
+
+function Parent({cond}) {
+  const ref1 = useRef(1);
+  const ref2 = useRef(2);
+  const ref = cond ? ref1 : ref2;
+  return <Child ref={ref} />;
+}
+
+function ChildImpl(_props, ref) {
+  const cb = () => ref.current;
+  return <Stringify cb={cb} shouldInvokeFns={true} />;
+}
+
+const Child = forwardRef(ChildImpl);
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Parent,
+  params: [{cond: true}],
+  sequentialRenders: [{cond: true}, {cond: false}],
+};
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-ref.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-ref.expect.md
new file mode 100644
index 0000000000000..c3876c22924fe
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-ref.expect.md
@@ -0,0 +1,79 @@
+
+## Input
+
+```javascript
+import {useRef} from 'react';
+import {Stringify} from 'shared-runtime';
+
+/**
+ * Fixture showing that Ref types may be reactive.
+ * We should always take a dependency on ref values (the outer box) as
+ * they may be reactive. Pruning should be done in
+ * `pruneNonReactiveDependencies`
+ */
+function Component({cond}) {
+  const ref1 = useRef(1);
+  const ref2 = useRef(2);
+  const ref = cond ? ref1 : ref2;
+  const cb = () => ref.current;
+  return <Stringify cb={cb} shouldInvokeFns={true} />;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{cond: true}],
+  sequentialRenders: [{cond: true}, {cond: false}],
+};
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime";
+import { useRef } from "react";
+import { Stringify } from "shared-runtime";
+
+/**
+ * Fixture showing that Ref types may be reactive.
+ * We should always take a dependency on ref values (the outer box) as
+ * they may be reactive. Pruning should be done in
+ * `pruneNonReactiveDependencies`
+ */
+function Component(t0) {
+  const $ = _c(4);
+  const { cond } = t0;
+  const ref1 = useRef(1);
+  const ref2 = useRef(2);
+  const ref = cond ? ref1 : ref2;
+  let t1;
+  if ($[0] !== ref) {
+    t1 = () => ref.current;
+    $[0] = ref;
+    $[1] = t1;
+  } else {
+    t1 = $[1];
+  }
+  const cb = t1;
+  let t2;
+  if ($[2] !== cb) {
+    t2 = <Stringify cb={cb} shouldInvokeFns={true} />;
+    $[2] = cb;
+    $[3] = t2;
+  } else {
+    t2 = $[3];
+  }
+  return t2;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{ cond: true }],
+  sequentialRenders: [{ cond: true }, { cond: false }],
+};
+
+```
+      
+### Eval output
+(kind: ok) <div>{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
+<div>{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}</div>
\ No newline at end of file
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-ref.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-ref.tsx
new file mode 100644
index 0000000000000..db95c5c204a3a
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-ref.tsx
@@ -0,0 +1,22 @@
+import {useRef} from 'react';
+import {Stringify} from 'shared-runtime';
+
+/**
+ * Fixture showing that Ref types may be reactive.
+ * We should always take a dependency on ref values (the outer box) as
+ * they may be reactive. Pruning should be done in
+ * `pruneNonReactiveDependencies`
+ */
+function Component({cond}) {
+  const ref1 = useRef(1);
+  const ref2 = useRef(2);
+  const ref = cond ? ref1 : ref2;
+  const cb = () => ref.current;
+  return <Stringify cb={cb} shouldInvokeFns={true} />;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{cond: true}],
+  sequentialRenders: [{cond: true}, {cond: false}],
+};
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-parameter-mutate-in-effect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-parameter-mutate-in-effect.expect.md
index 8b5a2eb1a0b35..7a10cfe48e2e4 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-parameter-mutate-in-effect.expect.md
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-parameter-mutate-in-effect.expect.md
@@ -13,7 +13,7 @@ function Foo(props, ref) {
 
 export const FIXTURE_ENTRYPOINT = {
   fn: Foo,
-  params: [{bar: 'foo'}, {ref: {cuurrent: 1}}],
+  params: [{bar: 'foo'}, {ref: {current: 1}}],
   isComponent: true,
 };
 
@@ -26,35 +26,39 @@ import { c as _c } from "react/compiler-runtime";
 import { useEffect } from "react";
 
 function Foo(props, ref) {
-  const $ = _c(4);
+  const $ = _c(5);
   let t0;
-  let t1;
-  if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
+  if ($[0] !== ref) {
     t0 = () => {
       ref.current = 2;
     };
+    $[0] = ref;
+    $[1] = t0;
+  } else {
+    t0 = $[1];
+  }
+  let t1;
+  if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
     t1 = [];
-    $[0] = t0;
-    $[1] = t1;
+    $[2] = t1;
   } else {
-    t0 = $[0];
-    t1 = $[1];
+    t1 = $[2];
   }
   useEffect(t0, t1);
   let t2;
-  if ($[2] !== props.bar) {
+  if ($[3] !== props.bar) {
     t2 = <div>{props.bar}</div>;
-    $[2] = props.bar;
-    $[3] = t2;
+    $[3] = props.bar;
+    $[4] = t2;
   } else {
-    t2 = $[3];
+    t2 = $[4];
   }
   return t2;
 }
 
 export const FIXTURE_ENTRYPOINT = {
   fn: Foo,
-  params: [{ bar: "foo" }, { ref: { cuurrent: 1 } }],
+  params: [{ bar: "foo" }, { ref: { current: 1 } }],
   isComponent: true,
 };
 
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-parameter-mutate-in-effect.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-parameter-mutate-in-effect.js
index c60141e8d5d81..ffd9eae08d66f 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-parameter-mutate-in-effect.js
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ref-parameter-mutate-in-effect.js
@@ -9,6 +9,6 @@ function Foo(props, ref) {
 
 export const FIXTURE_ENTRYPOINT = {
   fn: Foo,
-  params: [{bar: 'foo'}, {ref: {cuurrent: 1}}],
+  params: [{bar: 'foo'}, {ref: {current: 1}}],
   isComponent: true,
 };
diff --git a/compiler/packages/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts
index 6c136f6310928..1971fe0d33161 100644
--- a/compiler/packages/snap/src/SproutTodoFilter.ts
+++ b/compiler/packages/snap/src/SproutTodoFilter.ts
@@ -484,7 +484,6 @@ const skipFilter = new Set([
   'bug-aliased-capture-mutate',
   'bug-functiondecl-hoisting',
   'bug-try-catch-maybe-null-dependency',
-  'bug-nonreactive-ref',
   'reduce-reactive-deps/bug-infer-function-cond-access-not-hoisted',
   'bug-invalid-phi-as-dependency',
   'reduce-reactive-deps/bug-merge-uncond-optional-chain-and-cond',

From 6483117723e785c47bf1356140ec6419f64b5b44 Mon Sep 17 00:00:00 2001
From: Mofei Zhang <feifei0@meta.com>
Date: Fri, 15 Nov 2024 12:02:15 -0500
Subject: [PATCH 2/5] [compiler] Stop using function `dependencies` in
 propagateScopeDeps

Recursively visit inner function instructions to extract dependencies instead of using `LoweredFunction.dependencies` directly.

This is currently gated by enableFunctionDependencyRewrite, which needs to be removed before we delete `LoweredFunction.dependencies` altogether (#31204).

Some nice side effects
- optional-chaining deps for inner functions
- full DCE and outlining for inner functions (see #31202)
- fewer extraneous instructions (see #31204)

-
---
 .../src/HIR/Environment.ts                    |  2 +
 .../src/HIR/PropagateScopeDependenciesHIR.ts  | 67 +++++++++------
 .../capturing-func-mutate-2.expect.md         | 21 ++---
 ...ures-reassigned-context-property.expect.md | 53 ++++++++++++
 ...k-captures-reassigned-context-property.tsx | 32 ++++++++
 ...less-specific-conditional-access.expect.md |  2 -
 ...ures-reassigned-context-property.expect.md | 81 -------------------
 ...k-captures-reassigned-context-property.tsx | 21 -----
 ...back-captures-reassigned-context.expect.md | 16 ++--
 ...llback-extended-contextvar-scope.expect.md | 26 +++---
 ...unction-uncond-optionals-hoisted.expect.md |  4 +-
 ...unction-uncond-optionals-hoisted.expect.md |  4 +-
 12 files changed, 159 insertions(+), 170 deletions(-)
 create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-reassigned-context-property.expect.md
 create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-reassigned-context-property.tsx
 delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context-property.expect.md
 delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context-property.tsx

diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts
index 1d2e155848802..855bc039abf37 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts
+++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts
@@ -231,6 +231,8 @@ const EnvironmentConfigSchema = z.object({
    */
   enableUseTypeAnnotations: z.boolean().default(false),
 
+  enableFunctionDependencyRewrite: z.boolean().default(true),
+
   /**
    * Enables inlining ReactElement object literals in place of JSX
    * An alternative to the standard JSX transform which replaces JSX with React's jsxProd() runtime
diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts
index 7fd44c29dccf6..8aed17f8ee847 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts
+++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts
@@ -663,35 +663,54 @@ function collectDependencies(
 
   const scopeTraversal = new ScopeBlockTraversal();
 
-  for (const [blockId, block] of fn.body.blocks) {
-    scopeTraversal.recordScopes(block);
-    const scopeBlockInfo = scopeTraversal.blockInfos.get(blockId);
-    if (scopeBlockInfo?.kind === 'begin') {
-      context.enterScope(scopeBlockInfo.scope);
-    } else if (scopeBlockInfo?.kind === 'end') {
-      context.exitScope(scopeBlockInfo.scope, scopeBlockInfo?.pruned);
-    }
-
-    // Record referenced optional chains in phis
-    for (const phi of block.phis) {
-      for (const operand of phi.operands) {
-        const maybeOptionalChain = temporaries.get(operand[1].identifier.id);
-        if (maybeOptionalChain) {
-          context.visitDependency(maybeOptionalChain);
+  const handleFunction = (fn: HIRFunction): void => {
+    for (const [blockId, block] of fn.body.blocks) {
+      scopeTraversal.recordScopes(block);
+      const scopeBlockInfo = scopeTraversal.blockInfos.get(blockId);
+      if (scopeBlockInfo?.kind === 'begin') {
+        context.enterScope(scopeBlockInfo.scope);
+      } else if (scopeBlockInfo?.kind === 'end') {
+        context.exitScope(scopeBlockInfo.scope, scopeBlockInfo.pruned);
+      }
+      // Record referenced optional chains in phis
+      for (const phi of block.phis) {
+        for (const operand of phi.operands) {
+          const maybeOptionalChain = temporaries.get(operand[1].identifier.id);
+          if (maybeOptionalChain) {
+            context.visitDependency(maybeOptionalChain);
+          }
         }
       }
-    }
-    for (const instr of block.instructions) {
-      if (!processedInstrsInOptional.has(instr)) {
-        handleInstruction(instr, context);
+      for (const instr of block.instructions) {
+        if (
+          fn.env.config.enableFunctionDependencyRewrite &&
+          (instr.value.kind === 'FunctionExpression' ||
+            instr.value.kind === 'ObjectMethod')
+        ) {
+          context.declare(instr.lvalue.identifier, {
+            id: instr.id,
+            scope: context.currentScope,
+          });
+          /**
+           * Recursively visit the inner function to extract dependencies there
+           */
+          const wasInInnerFn = context.inInnerFn;
+          context.inInnerFn = true;
+          handleFunction(instr.value.loweredFunc.func);
+          context.inInnerFn = wasInInnerFn;
+        } else if (!processedInstrsInOptional.has(instr)) {
+          handleInstruction(instr, context);
+        }
       }
-    }
 
-    if (!processedInstrsInOptional.has(block.terminal)) {
-      for (const place of eachTerminalOperand(block.terminal)) {
-        context.visitOperand(place);
+      if (!processedInstrsInOptional.has(block.terminal)) {
+        for (const place of eachTerminalOperand(block.terminal)) {
+          context.visitOperand(place);
+        }
       }
     }
-  }
+  };
+
+  handleFunction(fn);
   return context.deps;
 }
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-mutate-2.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-mutate-2.expect.md
index b31a16da90e3f..c071d5d20ed99 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-mutate-2.expect.md
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-mutate-2.expect.md
@@ -26,29 +26,20 @@ export const FIXTURE_ENTRYPOINT = {
 ```javascript
 import { c as _c } from "react/compiler-runtime";
 function component(a, b) {
-  const $ = _c(5);
-  let t0;
-  if ($[0] !== b) {
-    t0 = { b };
-    $[0] = b;
-    $[1] = t0;
-  } else {
-    t0 = $[1];
-  }
-  const y = t0;
+  const $ = _c(2);
+  const y = { b };
   let z;
-  if ($[2] !== a || $[3] !== y) {
+  if ($[0] !== a) {
     z = { a };
     const x = function () {
       z.a = 2;
     };
 
     x();
-    $[2] = a;
-    $[3] = y;
-    $[4] = z;
+    $[0] = a;
+    $[1] = z;
   } else {
-    z = $[4];
+    z = $[1];
   }
   return z;
 }
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-reassigned-context-property.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-reassigned-context-property.expect.md
new file mode 100644
index 0000000000000..ae44f27912293
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-reassigned-context-property.expect.md
@@ -0,0 +1,53 @@
+
+## Input
+
+```javascript
+// @validatePreserveExistingMemoizationGuarantees
+import {useCallback} from 'react';
+import {Stringify} from 'shared-runtime';
+
+/**
+ * TODO: we're currently bailing out because `contextVar` is a context variable
+ * and not recorded into the PropagateScopeDeps LoadLocal / PropertyLoad
+ * sidemap. Previously, we were able to avoid this as `BuildHIR` hoisted
+ * `LoadContext` and `PropertyLoad` instructions into the outer function, which
+ * we took as eligible dependencies.
+ *
+ * One solution is to simply record `LoadContext` identifiers into the
+ * temporaries sidemap when the instruction occurs *after* the context
+ * variable's mutable range.
+ */
+function Foo(props) {
+  let contextVar;
+  if (props.cond) {
+    contextVar = {val: 2};
+  } else {
+    contextVar = {};
+  }
+
+  const cb = useCallback(() => [contextVar.val], [contextVar.val]);
+
+  return <Stringify cb={cb} shouldInvokeFns={true} />;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Foo,
+  params: [{cond: true}],
+};
+
+```
+
+
+## Error
+
+```
+  22 |   }
+  23 |
+> 24 |   const cb = useCallback(() => [contextVar.val], [contextVar.val]);
+     |                          ^^^^^^^^^^^^^^^^^^^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected (24:24)
+  25 |
+  26 |   return <Stringify cb={cb} shouldInvokeFns={true} />;
+  27 | }
+```
+          
+      
\ No newline at end of file
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-reassigned-context-property.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-reassigned-context-property.tsx
new file mode 100644
index 0000000000000..8447e3960dc51
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-reassigned-context-property.tsx
@@ -0,0 +1,32 @@
+// @validatePreserveExistingMemoizationGuarantees
+import {useCallback} from 'react';
+import {Stringify} from 'shared-runtime';
+
+/**
+ * TODO: we're currently bailing out because `contextVar` is a context variable
+ * and not recorded into the PropagateScopeDeps LoadLocal / PropertyLoad
+ * sidemap. Previously, we were able to avoid this as `BuildHIR` hoisted
+ * `LoadContext` and `PropertyLoad` instructions into the outer function, which
+ * we took as eligible dependencies.
+ *
+ * One solution is to simply record `LoadContext` identifiers into the
+ * temporaries sidemap when the instruction occurs *after* the context
+ * variable's mutable range.
+ */
+function Foo(props) {
+  let contextVar;
+  if (props.cond) {
+    contextVar = {val: 2};
+  } else {
+    contextVar = {};
+  }
+
+  const cb = useCallback(() => [contextVar.val], [contextVar.val]);
+
+  return <Stringify cb={cb} shouldInvokeFns={true} />;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Foo,
+  params: [{cond: true}],
+};
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useCallback-infer-less-specific-conditional-access.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useCallback-infer-less-specific-conditional-access.expect.md
index 955d391f912cf..940b3975c160d 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useCallback-infer-less-specific-conditional-access.expect.md
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useCallback-infer-less-specific-conditional-access.expect.md
@@ -44,8 +44,6 @@ function Component({propA, propB}) {
      | ^^^^^^^^^^^^^^^^^
 > 14 |   }, [propA?.a, propB.x.y]);
      | ^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected (6:14)
-
-CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected (6:14)
   15 | }
   16 |
 ```
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context-property.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context-property.expect.md
deleted file mode 100644
index db69bc2821b12..0000000000000
--- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context-property.expect.md
+++ /dev/null
@@ -1,81 +0,0 @@
-
-## Input
-
-```javascript
-// @validatePreserveExistingMemoizationGuarantees
-import {useCallback} from 'react';
-import {Stringify} from 'shared-runtime';
-
-function Foo(props) {
-  let contextVar;
-  if (props.cond) {
-    contextVar = {val: 2};
-  } else {
-    contextVar = {};
-  }
-
-  const cb = useCallback(() => [contextVar.val], [contextVar.val]);
-
-  return <Stringify cb={cb} shouldInvokeFns={true} />;
-}
-
-export const FIXTURE_ENTRYPOINT = {
-  fn: Foo,
-  params: [{cond: true}],
-};
-
-```
-
-## Code
-
-```javascript
-import { c as _c } from "react/compiler-runtime"; // @validatePreserveExistingMemoizationGuarantees
-import { useCallback } from "react";
-import { Stringify } from "shared-runtime";
-
-function Foo(props) {
-  const $ = _c(6);
-  let contextVar;
-  if ($[0] !== props.cond) {
-    if (props.cond) {
-      contextVar = { val: 2 };
-    } else {
-      contextVar = {};
-    }
-    $[0] = props.cond;
-    $[1] = contextVar;
-  } else {
-    contextVar = $[1];
-  }
-
-  const t0 = contextVar;
-  let t1;
-  if ($[2] !== t0.val) {
-    t1 = () => [contextVar.val];
-    $[2] = t0.val;
-    $[3] = t1;
-  } else {
-    t1 = $[3];
-  }
-  contextVar;
-  const cb = t1;
-  let t2;
-  if ($[4] !== cb) {
-    t2 = <Stringify cb={cb} shouldInvokeFns={true} />;
-    $[4] = cb;
-    $[5] = t2;
-  } else {
-    t2 = $[5];
-  }
-  return t2;
-}
-
-export const FIXTURE_ENTRYPOINT = {
-  fn: Foo,
-  params: [{ cond: true }],
-};
-
-```
-      
-### Eval output
-(kind: ok) <div>{"cb":{"kind":"Function","result":[2]},"shouldInvokeFns":true}</div>
\ No newline at end of file
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context-property.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context-property.tsx
deleted file mode 100644
index cb6f65a9f4f52..0000000000000
--- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context-property.tsx
+++ /dev/null
@@ -1,21 +0,0 @@
-// @validatePreserveExistingMemoizationGuarantees
-import {useCallback} from 'react';
-import {Stringify} from 'shared-runtime';
-
-function Foo(props) {
-  let contextVar;
-  if (props.cond) {
-    contextVar = {val: 2};
-  } else {
-    contextVar = {};
-  }
-
-  const cb = useCallback(() => [contextVar.val], [contextVar.val]);
-
-  return <Stringify cb={cb} shouldInvokeFns={true} />;
-}
-
-export const FIXTURE_ENTRYPOINT = {
-  fn: Foo,
-  params: [{cond: true}],
-};
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context.expect.md
index b66661fbcaa08..41994e1e56e44 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context.expect.md
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context.expect.md
@@ -45,18 +45,16 @@ function Foo(props) {
   } else {
     x = $[1];
   }
-
-  const t0 = x;
-  let t1;
-  if ($[2] !== t0) {
-    t1 = () => [x];
-    $[2] = t0;
-    $[3] = t1;
+  let t0;
+  if ($[2] !== x) {
+    t0 = () => [x];
+    $[2] = x;
+    $[3] = t0;
   } else {
-    t1 = $[3];
+    t0 = $[3];
   }
   x;
-  const cb = t1;
+  const cb = t0;
   return cb;
 }
 
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-extended-contextvar-scope.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-extended-contextvar-scope.expect.md
index b141c27614d24..9ce4a62e710d4 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-extended-contextvar-scope.expect.md
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-extended-contextvar-scope.expect.md
@@ -70,28 +70,26 @@ function useBar(t0, cond) {
   if (cond) {
     x = b;
   }
-
-  const t2 = x;
-  let t3;
-  if ($[1] !== a || $[2] !== t2) {
-    t3 = () => [a, x];
+  let t2;
+  if ($[1] !== a || $[2] !== x) {
+    t2 = () => [a, x];
     $[1] = a;
-    $[2] = t2;
-    $[3] = t3;
+    $[2] = x;
+    $[3] = t2;
   } else {
-    t3 = $[3];
+    t2 = $[3];
   }
   x;
-  const cb = t3;
-  let t4;
+  const cb = t2;
+  let t3;
   if ($[4] !== cb) {
-    t4 = <Stringify cb={cb} shouldInvoke={true} />;
+    t3 = <Stringify cb={cb} shouldInvoke={true} />;
     $[4] = cb;
-    $[5] = t4;
+    $[5] = t3;
   } else {
-    t4 = $[5];
+    t3 = $[5];
   }
-  return t4;
+  return t3;
 }
 
 export const FIXTURE_ENTRYPOINT = {
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/todo-infer-function-uncond-optionals-hoisted.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/todo-infer-function-uncond-optionals-hoisted.expect.md
index 02e60eff91d61..ed56ff068113b 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/todo-infer-function-uncond-optionals-hoisted.expect.md
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/todo-infer-function-uncond-optionals-hoisted.expect.md
@@ -34,9 +34,9 @@ function useFoo(t0) {
   const $ = _c(2);
   const { a } = t0;
   let t1;
-  if ($[0] !== a.b) {
+  if ($[0] !== a.b?.c.d?.e) {
     t1 = <Stringify fn={() => a.b?.c.d?.e} shouldInvokeFns={true} />;
-    $[0] = a.b;
+    $[0] = a.b?.c.d?.e;
     $[1] = t1;
   } else {
     t1 = $[1];
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/todo-infer-function-uncond-optionals-hoisted.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/todo-infer-function-uncond-optionals-hoisted.expect.md
index 157e2de81a1ff..bb99a5d90fe2b 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/todo-infer-function-uncond-optionals-hoisted.expect.md
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/todo-infer-function-uncond-optionals-hoisted.expect.md
@@ -31,9 +31,9 @@ function useFoo(t0) {
   const $ = _c(2);
   const { a } = t0;
   let t1;
-  if ($[0] !== a.b) {
+  if ($[0] !== a.b?.c.d?.e) {
     t1 = <Stringify fn={() => a.b?.c.d?.e} shouldInvokeFns={true} />;
-    $[0] = a.b;
+    $[0] = a.b?.c.d?.e;
     $[1] = t1;
   } else {
     t1 = $[1];

From 6a143e5a4379741c8f80264201d682e3926f01eb Mon Sep 17 00:00:00 2001
From: Mofei Zhang <feifei0@meta.com>
Date: Fri, 15 Nov 2024 12:02:15 -0500
Subject: [PATCH 3/5] [compiler] Lower JSXMemberExpression with LoadLocal

`JSXMemberExpression` is currently the only instruction (that I know of) that directly references identifier lvalues without a corresponding `LoadLocal`.

This has some side effects:
- deadcode elimination and constant propagation now reach JSXMemberExpressions
- we can delete `LoweredFunction.dependencies` without dangling references (previously, the only reference to JSXMemberExpression objects in HIR was in function dependencies)
- JSXMemberExpression now is consistent with all other instructions (e.g. has a rvalue-producing LoadLocal)

'
---
 .../src/HIR/BuildHIR.ts                       |  8 +-
 .../invalid-jsx-lowercase-localvar.expect.md  | 75 +++++++++++++++++++
 .../invalid-jsx-lowercase-localvar.jsx        | 29 +++++++
 ...local-memberexpr-tag-conditional.expect.md |  3 +-
 .../jsx-local-memberexpr-tag.expect.md        |  3 +-
 ...se-localvar-memberexpr-in-lambda.expect.md | 59 +++++++++++++++
 ...owercase-localvar-memberexpr-in-lambda.jsx | 12 +++
 ...sx-lowercase-localvar-memberexpr.expect.md | 45 +++++++++++
 .../jsx-lowercase-localvar-memberexpr.jsx     | 10 +++
 .../jsx-lowercase-memberexpr.expect.md        | 44 +++++++++++
 .../compiler/jsx-lowercase-memberexpr.jsx     |  9 +++
 .../jsx-memberexpr-tag-in-lambda.expect.md    |  3 +-
 .../packages/snap/src/SproutTodoFilter.ts     |  3 +
 .../snap/src/sprout/shared-runtime.ts         |  3 +
 14 files changed, 299 insertions(+), 7 deletions(-)
 create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-jsx-lowercase-localvar.expect.md
 create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-jsx-lowercase-localvar.jsx
 create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-lowercase-localvar-memberexpr-in-lambda.expect.md
 create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-lowercase-localvar-memberexpr-in-lambda.jsx
 create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-lowercase-localvar-memberexpr.expect.md
 create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-lowercase-localvar-memberexpr.jsx
 create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-lowercase-memberexpr.expect.md
 create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-lowercase-memberexpr.jsx

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 9494436d1f463..ecc22365dd03c 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts
+++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts
@@ -3186,7 +3186,13 @@ function lowerJsxMemberExpression(
       loc: object.node.loc ?? null,
       suggestions: null,
     });
-    objectPlace = lowerIdentifier(builder, object);
+
+    const kind = getLoadKind(builder, object);
+    objectPlace = lowerValueToTemporary(builder, {
+      kind: kind,
+      place: lowerIdentifier(builder, object),
+      loc: exprPath.node.loc ?? GeneratedSource,
+    });
   }
   const property = exprPath.get('property').node.name;
   return lowerValueToTemporary(builder, {
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-jsx-lowercase-localvar.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-jsx-lowercase-localvar.expect.md
new file mode 100644
index 0000000000000..925346225c3e8
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-jsx-lowercase-localvar.expect.md
@@ -0,0 +1,75 @@
+
+## Input
+
+```javascript
+import {Throw} from 'shared-runtime';
+
+/**
+ * Note: this is disabled in the evaluator due to different devmode errors.
+ * Found differences in evaluator results
+ *  Non-forget (expected):
+ *  (kind: ok) <invalidtag val="[object Object]"></invalidtag>
+ *  logs: ['Warning: <%s /> is using incorrect casing. Use PascalCase for React components, or lowercase for HTML elements.%s','invalidTag']
+ *
+ *  Forget:
+ *  (kind: ok) <invalidtag val="[object Object]"></invalidtag>
+ *  logs: [
+ *   'Warning: <%s /> is using incorrect casing. Use PascalCase for React components, or lowercase for HTML elements.%s','invalidTag',
+ *   'Warning: The tag <%s> is unrecognized in this browser. If you meant to render a React component, start its name with an uppercase letter.%s','invalidTag',
+ *  ]
+ */
+function useFoo() {
+  const invalidTag = Throw;
+  /**
+   * Need to be careful to not parse `invalidTag` as a localVar (i.e. render
+   * Throw). Note that the jsx transform turns this into a string tag:
+   * `jsx("invalidTag"...
+   */
+  return <invalidTag val={{val: 2}} />;
+}
+export const FIXTURE_ENTRYPOINT = {
+  fn: useFoo,
+  params: [],
+};
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime";
+import { Throw } from "shared-runtime";
+
+/**
+ * Note: this is disabled in the evaluator due to different devmode errors.
+ * Found differences in evaluator results
+ *  Non-forget (expected):
+ *  (kind: ok) <invalidtag val="[object Object]"></invalidtag>
+ *  logs: ['Warning: <%s /> is using incorrect casing. Use PascalCase for React components, or lowercase for HTML elements.%s','invalidTag']
+ *
+ *  Forget:
+ *  (kind: ok) <invalidtag val="[object Object]"></invalidtag>
+ *  logs: [
+ *   'Warning: <%s /> is using incorrect casing. Use PascalCase for React components, or lowercase for HTML elements.%s','invalidTag',
+ *   'Warning: The tag <%s> is unrecognized in this browser. If you meant to render a React component, start its name with an uppercase letter.%s','invalidTag',
+ *  ]
+ */
+function useFoo() {
+  const $ = _c(1);
+  let t0;
+  if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
+    t0 = <invalidTag val={{ val: 2 }} />;
+    $[0] = t0;
+  } else {
+    t0 = $[0];
+  }
+  return t0;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: useFoo,
+  params: [],
+};
+
+```
+      
\ No newline at end of file
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-jsx-lowercase-localvar.jsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-jsx-lowercase-localvar.jsx
new file mode 100644
index 0000000000000..1e62eb011765e
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-jsx-lowercase-localvar.jsx
@@ -0,0 +1,29 @@
+import {Throw} from 'shared-runtime';
+
+/**
+ * Note: this is disabled in the evaluator due to different devmode errors.
+ * Found differences in evaluator results
+ *  Non-forget (expected):
+ *  (kind: ok) <invalidtag val="[object Object]"></invalidtag>
+ *  logs: ['Warning: <%s /> is using incorrect casing. Use PascalCase for React components, or lowercase for HTML elements.%s','invalidTag']
+ *
+ *  Forget:
+ *  (kind: ok) <invalidtag val="[object Object]"></invalidtag>
+ *  logs: [
+ *   'Warning: <%s /> is using incorrect casing. Use PascalCase for React components, or lowercase for HTML elements.%s','invalidTag',
+ *   'Warning: The tag <%s> is unrecognized in this browser. If you meant to render a React component, start its name with an uppercase letter.%s','invalidTag',
+ *  ]
+ */
+function useFoo() {
+  const invalidTag = Throw;
+  /**
+   * Need to be careful to not parse `invalidTag` as a localVar (i.e. render
+   * Throw). Note that the jsx transform turns this into a string tag:
+   * `jsx("invalidTag"...
+   */
+  return <invalidTag val={{val: 2}} />;
+}
+export const FIXTURE_ENTRYPOINT = {
+  fn: useFoo,
+  params: [],
+};
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-local-memberexpr-tag-conditional.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-local-memberexpr-tag-conditional.expect.md
index 0cb821459cd03..f13d3a05981d4 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-local-memberexpr-tag-conditional.expect.md
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-local-memberexpr-tag-conditional.expect.md
@@ -27,11 +27,10 @@ import * as SharedRuntime from "shared-runtime";
 function useFoo(t0) {
   const $ = _c(1);
   const { cond } = t0;
-  const MyLocal = SharedRuntime;
   if (cond) {
     let t1;
     if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
-      t1 = <MyLocal.Text value={4} />;
+      t1 = <SharedRuntime.Text value={4} />;
       $[0] = t1;
     } else {
       t1 = $[0];
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-local-memberexpr-tag.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-local-memberexpr-tag.expect.md
index ab11ddedb8489..f24e7a754dbe7 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-local-memberexpr-tag.expect.md
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-local-memberexpr-tag.expect.md
@@ -22,10 +22,9 @@ import { c as _c } from "react/compiler-runtime";
 import * as SharedRuntime from "shared-runtime";
 function useFoo() {
   const $ = _c(1);
-  const MyLocal = SharedRuntime;
   let t0;
   if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
-    t0 = <MyLocal.Text value={4} />;
+    t0 = <SharedRuntime.Text value={4} />;
     $[0] = t0;
   } else {
     t0 = $[0];
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-lowercase-localvar-memberexpr-in-lambda.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-lowercase-localvar-memberexpr-in-lambda.expect.md
new file mode 100644
index 0000000000000..248234793931e
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-lowercase-localvar-memberexpr-in-lambda.expect.md
@@ -0,0 +1,59 @@
+
+## Input
+
+```javascript
+import * as SharedRuntime from 'shared-runtime';
+import {invoke} from 'shared-runtime';
+function useComponentFactory({name}) {
+  const localVar = SharedRuntime;
+  const cb = () => <localVar.Stringify>hello world {name}</localVar.Stringify>;
+  return invoke(cb);
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: useComponentFactory,
+  params: [{name: 'sathya'}],
+};
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime";
+import * as SharedRuntime from "shared-runtime";
+import { invoke } from "shared-runtime";
+function useComponentFactory(t0) {
+  const $ = _c(4);
+  const { name } = t0;
+  let t1;
+  if ($[0] !== name) {
+    t1 = () => (
+      <SharedRuntime.Stringify>hello world {name}</SharedRuntime.Stringify>
+    );
+    $[0] = name;
+    $[1] = t1;
+  } else {
+    t1 = $[1];
+  }
+  const cb = t1;
+  let t2;
+  if ($[2] !== cb) {
+    t2 = invoke(cb);
+    $[2] = cb;
+    $[3] = t2;
+  } else {
+    t2 = $[3];
+  }
+  return t2;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: useComponentFactory,
+  params: [{ name: "sathya" }],
+};
+
+```
+      
+### Eval output
+(kind: ok) <div>{"children":["hello world ","sathya"]}</div>
\ No newline at end of file
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-lowercase-localvar-memberexpr-in-lambda.jsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-lowercase-localvar-memberexpr-in-lambda.jsx
new file mode 100644
index 0000000000000..534490d5d4ef5
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-lowercase-localvar-memberexpr-in-lambda.jsx
@@ -0,0 +1,12 @@
+import * as SharedRuntime from 'shared-runtime';
+import {invoke} from 'shared-runtime';
+function useComponentFactory({name}) {
+  const localVar = SharedRuntime;
+  const cb = () => <localVar.Stringify>hello world {name}</localVar.Stringify>;
+  return invoke(cb);
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: useComponentFactory,
+  params: [{name: 'sathya'}],
+};
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-lowercase-localvar-memberexpr.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-lowercase-localvar-memberexpr.expect.md
new file mode 100644
index 0000000000000..5778bf599f88f
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-lowercase-localvar-memberexpr.expect.md
@@ -0,0 +1,45 @@
+
+## Input
+
+```javascript
+import * as SharedRuntime from 'shared-runtime';
+function Component({name}) {
+  const localVar = SharedRuntime;
+  return <localVar.Stringify>hello world {name}</localVar.Stringify>;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{name: 'sathya'}],
+};
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime";
+import * as SharedRuntime from "shared-runtime";
+function Component(t0) {
+  const $ = _c(2);
+  const { name } = t0;
+  let t1;
+  if ($[0] !== name) {
+    t1 = <SharedRuntime.Stringify>hello world {name}</SharedRuntime.Stringify>;
+    $[0] = name;
+    $[1] = t1;
+  } else {
+    t1 = $[1];
+  }
+  return t1;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{ name: "sathya" }],
+};
+
+```
+      
+### Eval output
+(kind: ok) <div>{"children":["hello world ","sathya"]}</div>
\ No newline at end of file
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-lowercase-localvar-memberexpr.jsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-lowercase-localvar-memberexpr.jsx
new file mode 100644
index 0000000000000..d55037fca039e
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-lowercase-localvar-memberexpr.jsx
@@ -0,0 +1,10 @@
+import * as SharedRuntime from 'shared-runtime';
+function Component({name}) {
+  const localVar = SharedRuntime;
+  return <localVar.Stringify>hello world {name}</localVar.Stringify>;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{name: 'sathya'}],
+};
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-lowercase-memberexpr.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-lowercase-memberexpr.expect.md
new file mode 100644
index 0000000000000..f5f7b3727e99c
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-lowercase-memberexpr.expect.md
@@ -0,0 +1,44 @@
+
+## Input
+
+```javascript
+import * as SharedRuntime from 'shared-runtime';
+function Component({name}) {
+  return <SharedRuntime.Stringify>hello world {name}</SharedRuntime.Stringify>;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{name: 'sathya'}],
+};
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime";
+import * as SharedRuntime from "shared-runtime";
+function Component(t0) {
+  const $ = _c(2);
+  const { name } = t0;
+  let t1;
+  if ($[0] !== name) {
+    t1 = <SharedRuntime.Stringify>hello world {name}</SharedRuntime.Stringify>;
+    $[0] = name;
+    $[1] = t1;
+  } else {
+    t1 = $[1];
+  }
+  return t1;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{ name: "sathya" }],
+};
+
+```
+      
+### Eval output
+(kind: ok) <div>{"children":["hello world ","sathya"]}</div>
\ No newline at end of file
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-lowercase-memberexpr.jsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-lowercase-memberexpr.jsx
new file mode 100644
index 0000000000000..992cbecebe38e
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-lowercase-memberexpr.jsx
@@ -0,0 +1,9 @@
+import * as SharedRuntime from 'shared-runtime';
+function Component({name}) {
+  return <SharedRuntime.Stringify>hello world {name}</SharedRuntime.Stringify>;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{name: 'sathya'}],
+};
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-memberexpr-tag-in-lambda.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-memberexpr-tag-in-lambda.expect.md
index 363f82d12c6a6..22fa3b2e2a2e3 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-memberexpr-tag-in-lambda.expect.md
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-memberexpr-tag-in-lambda.expect.md
@@ -25,10 +25,9 @@ import { c as _c } from "react/compiler-runtime";
 import * as SharedRuntime from "shared-runtime";
 function useFoo() {
   const $ = _c(1);
-  const MyLocal = SharedRuntime;
   let t0;
   if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
-    const callback = () => <MyLocal.Text value={4} />;
+    const callback = () => <SharedRuntime.Text value={4} />;
 
     t0 = callback();
     $[0] = t0;
diff --git a/compiler/packages/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts
index 1971fe0d33161..9a629ed8bb033 100644
--- a/compiler/packages/snap/src/SproutTodoFilter.ts
+++ b/compiler/packages/snap/src/SproutTodoFilter.ts
@@ -475,6 +475,9 @@ const skipFilter = new Set([
   'rules-of-hooks/rules-of-hooks-93dc5d5e538a',
   'rules-of-hooks/rules-of-hooks-69521d94fa03',
 
+  // false positives
+  'invalid-jsx-lowercase-localvar',
+
   // bugs
   'fbt/bug-fbt-plural-multiple-function-calls',
   'fbt/bug-fbt-plural-multiple-mixed-call-tag',
diff --git a/compiler/packages/snap/src/sprout/shared-runtime.ts b/compiler/packages/snap/src/sprout/shared-runtime.ts
index 0f3e09b12e127..e6e82d6b77e05 100644
--- a/compiler/packages/snap/src/sprout/shared-runtime.ts
+++ b/compiler/packages/snap/src/sprout/shared-runtime.ts
@@ -252,6 +252,9 @@ export function Stringify(props: any): React.ReactElement {
     toJSON(props, props?.shouldInvokeFns),
   );
 }
+export function Throw() {
+  throw new Error();
+}
 
 export function ValidateMemoization({
   inputs,

From 83a96d32c4606ff693f77a802aefca1c3262c3ad Mon Sep 17 00:00:00 2001
From: Mofei Zhang <feifei0@meta.com>
Date: Fri, 15 Nov 2024 12:02:15 -0500
Subject: [PATCH 4/5] [compiler][be] Patch test fixtures for evaluator

Add more `FIXTURE_ENTRYPOINT`s

'
---
 ...uring-func-alias-captured-mutate.expect.md | 46 +++++++++---
 .../capturing-func-alias-captured-mutate.js   | 20 ++++--
 .../compiler/capturing-func-mutate.expect.md  | 59 ++++++++++-----
 .../compiler/capturing-func-mutate.js         | 21 ++++--
 .../capturing-func-no-mutate.expect.md        | 72 +++++++++++++++++++
 .../compiler/capturing-func-no-mutate.js      | 21 ++++++
 .../packages/snap/src/SproutTodoFilter.ts     |  2 -
 7 files changed, 200 insertions(+), 41 deletions(-)
 create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-no-mutate.expect.md
 create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-no-mutate.js

diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-alias-captured-mutate.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-alias-captured-mutate.expect.md
index 14532562fb153..9a98f76b97170 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-alias-captured-mutate.expect.md
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-alias-captured-mutate.expect.md
@@ -2,39 +2,55 @@
 ## Input
 
 ```javascript
-function component(foo, bar) {
+import {mutate} from 'shared-runtime';
+
+function Component({foo, bar}) {
   let x = {foo};
   let y = {bar};
   const f0 = function () {
-    let a = {y};
+    let a = [y];
     let b = x;
-    a.x = b;
+    // this writes y.x = x
+    a[0].x = b;
   };
   f0();
-  mutate(y);
+  mutate(y.x);
   return y;
 }
 
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{foo: 3, bar: 4}],
+  sequentialRenders: [
+    {foo: 3, bar: 4},
+    {foo: 3, bar: 5},
+  ],
+};
+
 ```
 
 ## Code
 
 ```javascript
 import { c as _c } from "react/compiler-runtime";
-function component(foo, bar) {
+import { mutate } from "shared-runtime";
+
+function Component(t0) {
   const $ = _c(3);
+  const { foo, bar } = t0;
   let y;
   if ($[0] !== bar || $[1] !== foo) {
     const x = { foo };
     y = { bar };
     const f0 = function () {
-      const a = { y };
+      const a = [y];
       const b = x;
-      a.x = b;
+
+      a[0].x = b;
     };
 
     f0();
-    mutate(y);
+    mutate(y.x);
     $[0] = bar;
     $[1] = foo;
     $[2] = y;
@@ -44,5 +60,17 @@ function component(foo, bar) {
   return y;
 }
 
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{ foo: 3, bar: 4 }],
+  sequentialRenders: [
+    { foo: 3, bar: 4 },
+    { foo: 3, bar: 5 },
+  ],
+};
+
 ```
-      
\ No newline at end of file
+      
+### Eval output
+(kind: ok) {"bar":4,"x":{"foo":3,"wat0":"joe"}}
+{"bar":5,"x":{"foo":3,"wat0":"joe"}}
\ No newline at end of file
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-alias-captured-mutate.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-alias-captured-mutate.js
index ed4e097b66ace..b88ad567184fe 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-alias-captured-mutate.js
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-alias-captured-mutate.js
@@ -1,12 +1,24 @@
-function component(foo, bar) {
+import {mutate} from 'shared-runtime';
+
+function Component({foo, bar}) {
   let x = {foo};
   let y = {bar};
   const f0 = function () {
-    let a = {y};
+    let a = [y];
     let b = x;
-    a.x = b;
+    // this writes y.x = x
+    a[0].x = b;
   };
   f0();
-  mutate(y);
+  mutate(y.x);
   return y;
 }
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{foo: 3, bar: 4}],
+  sequentialRenders: [
+    {foo: 3, bar: 4},
+    {foo: 3, bar: 5},
+  ],
+};
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-mutate.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-mutate.expect.md
index 7ad5c47da75f8..fcde7d675c846 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-mutate.expect.md
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-mutate.expect.md
@@ -2,21 +2,28 @@
 ## Input
 
 ```javascript
-function component(a, b) {
+import {mutate} from 'shared-runtime';
+
+function Component({a, b}) {
   let z = {a};
-  let y = {b};
+  let y = {b: {b}};
   let x = function () {
     z.a = 2;
-    console.log(y.b);
+    mutate(y.b);
   };
   x();
-  return z;
+  return [y, z];
 }
 
 export const FIXTURE_ENTRYPOINT = {
-  fn: component,
-  params: ['TodoAdd'],
-  isComponent: 'TodoAdd',
+  fn: Component,
+  params: [{a: 2, b: 3}],
+  sequentialRenders: [
+    {a: 2, b: 3},
+    {a: 2, b: 3},
+    {a: 4, b: 3},
+    {a: 4, b: 5},
+  ],
 };
 
 ```
@@ -25,32 +32,46 @@ export const FIXTURE_ENTRYPOINT = {
 
 ```javascript
 import { c as _c } from "react/compiler-runtime";
-function component(a, b) {
+import { mutate } from "shared-runtime";
+
+function Component(t0) {
   const $ = _c(3);
-  let z;
+  const { a, b } = t0;
+  let t1;
   if ($[0] !== a || $[1] !== b) {
-    z = { a };
-    const y = { b };
+    const z = { a };
+    const y = { b: { b } };
     const x = function () {
       z.a = 2;
-      console.log(y.b);
+      mutate(y.b);
     };
 
     x();
+    t1 = [y, z];
     $[0] = a;
     $[1] = b;
-    $[2] = z;
+    $[2] = t1;
   } else {
-    z = $[2];
+    t1 = $[2];
   }
-  return z;
+  return t1;
 }
 
 export const FIXTURE_ENTRYPOINT = {
-  fn: component,
-  params: ["TodoAdd"],
-  isComponent: "TodoAdd",
+  fn: Component,
+  params: [{ a: 2, b: 3 }],
+  sequentialRenders: [
+    { a: 2, b: 3 },
+    { a: 2, b: 3 },
+    { a: 4, b: 3 },
+    { a: 4, b: 5 },
+  ],
 };
 
 ```
-      
\ No newline at end of file
+      
+### Eval output
+(kind: ok) [{"b":{"b":3,"wat0":"joe"}},{"a":2}]
+[{"b":{"b":3,"wat0":"joe"}},{"a":2}]
+[{"b":{"b":3,"wat0":"joe"}},{"a":2}]
+[{"b":{"b":5,"wat0":"joe"}},{"a":2}]
\ No newline at end of file
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-mutate.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-mutate.js
index 62014ee084eae..2ec7bcbe86cfd 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-mutate.js
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-mutate.js
@@ -1,16 +1,23 @@
-function component(a, b) {
+import {mutate} from 'shared-runtime';
+
+function Component({a, b}) {
   let z = {a};
-  let y = {b};
+  let y = {b: {b}};
   let x = function () {
     z.a = 2;
-    console.log(y.b);
+    mutate(y.b);
   };
   x();
-  return z;
+  return [y, z];
 }
 
 export const FIXTURE_ENTRYPOINT = {
-  fn: component,
-  params: ['TodoAdd'],
-  isComponent: 'TodoAdd',
+  fn: Component,
+  params: [{a: 2, b: 3}],
+  sequentialRenders: [
+    {a: 2, b: 3},
+    {a: 2, b: 3},
+    {a: 4, b: 3},
+    {a: 4, b: 5},
+  ],
 };
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-no-mutate.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-no-mutate.expect.md
new file mode 100644
index 0000000000000..aa32b3260ef50
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-no-mutate.expect.md
@@ -0,0 +1,72 @@
+
+## Input
+
+```javascript
+function Component({a, b}) {
+  let z = {a};
+  let y = {b};
+  let x = function () {
+    z.a = 2;
+    return Math.max(y.b, 0);
+  };
+  x();
+  return z;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{a: 2, b: 3}],
+  sequentialRenders: [
+    {a: 2, b: 3},
+    {a: 2, b: 3},
+    {a: 4, b: 3},
+    {a: 4, b: 5},
+  ],
+};
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime";
+function Component(t0) {
+  const $ = _c(3);
+  const { a, b } = t0;
+  let z;
+  if ($[0] !== a || $[1] !== b) {
+    z = { a };
+    const y = { b };
+    const x = function () {
+      z.a = 2;
+      return Math.max(y.b, 0);
+    };
+
+    x();
+    $[0] = a;
+    $[1] = b;
+    $[2] = z;
+  } else {
+    z = $[2];
+  }
+  return z;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{ a: 2, b: 3 }],
+  sequentialRenders: [
+    { a: 2, b: 3 },
+    { a: 2, b: 3 },
+    { a: 4, b: 3 },
+    { a: 4, b: 5 },
+  ],
+};
+
+```
+      
+### Eval output
+(kind: ok) {"a":2}
+{"a":2}
+{"a":2}
+{"a":2}
\ No newline at end of file
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-no-mutate.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-no-mutate.js
new file mode 100644
index 0000000000000..8fe3bb3db52f6
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-func-no-mutate.js
@@ -0,0 +1,21 @@
+function Component({a, b}) {
+  let z = {a};
+  let y = {b};
+  let x = function () {
+    z.a = 2;
+    return Math.max(y.b, 0);
+  };
+  x();
+  return z;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{a: 2, b: 3}],
+  sequentialRenders: [
+    {a: 2, b: 3},
+    {a: 2, b: 3},
+    {a: 4, b: 3},
+    {a: 4, b: 5},
+  ],
+};
diff --git a/compiler/packages/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts
index 9a629ed8bb033..f1f5ef0f6b3c7 100644
--- a/compiler/packages/snap/src/SproutTodoFilter.ts
+++ b/compiler/packages/snap/src/SproutTodoFilter.ts
@@ -34,7 +34,6 @@ const skipFilter = new Set([
   'capturing-arrow-function-1',
   'capturing-func-mutate-3',
   'capturing-func-mutate-nested',
-  'capturing-func-mutate',
   'capturing-function-1',
   'capturing-function-alias-computed-load',
   'capturing-function-decl',
@@ -236,7 +235,6 @@ const skipFilter = new Set([
   'capturing-fun-alias-captured-mutate-2',
   'capturing-fun-alias-captured-mutate-arr-2',
   'capturing-func-alias-captured-mutate-arr',
-  'capturing-func-alias-captured-mutate',
   'capturing-func-alias-computed-mutate',
   'capturing-func-alias-mutate',
   'capturing-func-alias-receiver-computed-mutate',

From 66179075578e244d52f147775396ef2043a8c5e2 Mon Sep 17 00:00:00 2001
From: Mofei Zhang <feifei0@meta.com>
Date: Fri, 15 Nov 2024 12:02:15 -0500
Subject: [PATCH 5/5] [compiler][be] Clean up nested function context in DCE

Now that we rely on function context exclusively, let's clean up `HIRFunction.context` after DCE. This PR is in preparation of #31204, which would otherwise have unnecessary declarations (of context values that become entirely DCE'd)

'
---
 .../src/Optimization/DeadCodeElimination.ts   |  8 ++++
 .../compiler/arrow-expr-directive.expect.md   |  5 ++-
 .../compiler/capture-param-mutate.expect.md   |  9 ++--
 .../function-expr-directive.expect.md         |  5 ++-
 .../compiler/merge-scopes-callback.expect.md  |  5 ++-
 ...reactive-scope-with-early-return.expect.md | 42 ++++++++-----------
 ...react-hooks-based-on-import-name.expect.md |  5 ++-
 7 files changed, 46 insertions(+), 33 deletions(-)

diff --git a/compiler/packages/babel-plugin-react-compiler/src/Optimization/DeadCodeElimination.ts b/compiler/packages/babel-plugin-react-compiler/src/Optimization/DeadCodeElimination.ts
index 885ec2b3ab2eb..0202d3ecf043f 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/Optimization/DeadCodeElimination.ts
+++ b/compiler/packages/babel-plugin-react-compiler/src/Optimization/DeadCodeElimination.ts
@@ -58,6 +58,14 @@ export function deadCodeElimination(fn: HIRFunction): void {
       }
     }
   }
+
+  /**
+   * Constant propagation and DCE may have deleted or rewritten instructions
+   * that reference context variables.
+   */
+  retainWhere(fn.context, contextVar =>
+    state.isIdOrNameUsed(contextVar.identifier),
+  );
 }
 
 class State {
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/arrow-expr-directive.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/arrow-expr-directive.expect.md
index 4586bfb103cbf..93eb2bd28ac6c 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/arrow-expr-directive.expect.md
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/arrow-expr-directive.expect.md
@@ -28,7 +28,7 @@ function Component() {
     t0 = () => {
       "worklet";
 
-      setCount((count_0) => count_0 + 1);
+      setCount(_temp);
     };
     $[0] = t0;
   } else {
@@ -45,6 +45,9 @@ function Component() {
   }
   return t1;
 }
+function _temp(count_0) {
+  return count_0 + 1;
+}
 
 ```
       
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capture-param-mutate.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capture-param-mutate.expect.md
index c9c197345c2c6..9e4709616d5ec 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capture-param-mutate.expect.md
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capture-param-mutate.expect.md
@@ -55,11 +55,7 @@ function getNativeLogFunction(level) {
       if (arguments.length === 1 && typeof arguments[0] === "string") {
         str = arguments[0];
       } else {
-        str = Array.prototype.map
-          .call(arguments, function (arg) {
-            return inspect(arg, { depth: 10 });
-          })
-          .join(", ");
+        str = Array.prototype.map.call(arguments, _temp).join(", ");
       }
 
       const firstArg = arguments[0];
@@ -92,6 +88,9 @@ function getNativeLogFunction(level) {
   }
   return t0;
 }
+function _temp(arg) {
+  return inspect(arg, { depth: 10 });
+}
 
 ```
       
\ No newline at end of file
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/function-expr-directive.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/function-expr-directive.expect.md
index 3980434bde07b..8c4aa612e8ca7 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/function-expr-directive.expect.md
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/function-expr-directive.expect.md
@@ -34,7 +34,7 @@ function Component() {
     t0 = function update() {
       "worklet";
 
-      setCount((count_0) => count_0 + 1);
+      setCount(_temp);
     };
     $[0] = t0;
   } else {
@@ -51,6 +51,9 @@ function Component() {
   }
   return t1;
 }
+function _temp(count_0) {
+  return count_0 + 1;
+}
 
 export const FIXTURE_ENTRYPOINT = {
   fn: Component,
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/merge-scopes-callback.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/merge-scopes-callback.expect.md
index edf748de5c270..0ff9773f76e0b 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/merge-scopes-callback.expect.md
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/merge-scopes-callback.expect.md
@@ -32,7 +32,7 @@ function Component() {
   let t0;
   if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
     t0 = () => {
-      setState((s) => s + 1);
+      setState(_temp);
     };
     $[0] = t0;
   } else {
@@ -61,6 +61,9 @@ function Component() {
   }
   return t2;
 }
+function _temp(s) {
+  return s + 1;
+}
 
 ```
       
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-no-declarations-in-reactive-scope-with-early-return.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-no-declarations-in-reactive-scope-with-early-return.expect.md
index 0c1bf1cd701b3..506e4ca713301 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-no-declarations-in-reactive-scope-with-early-return.expect.md
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-no-declarations-in-reactive-scope-with-early-return.expect.md
@@ -39,7 +39,7 @@ function Component() {
 ```javascript
 import { c as _c } from "react/compiler-runtime"; // @enableAssumeHooksFollowRulesOfReact @enableTransitivelyFreezeFunctionExpressions
 function Component() {
-  const $ = _c(8);
+  const $ = _c(7);
   const items = useItems();
   let t0;
   let t1;
@@ -47,35 +47,25 @@ function Component() {
   if ($[0] !== items) {
     t2 = Symbol.for("react.early_return_sentinel");
     bb0: {
-      let t3;
-      if ($[4] === Symbol.for("react.memo_cache_sentinel")) {
-        t3 = (t4) => {
-          const [item] = t4;
-          return item.name != null;
-        };
-        $[4] = t3;
-      } else {
-        t3 = $[4];
-      }
-      t0 = items.filter(t3);
+      t0 = items.filter(_temp);
       const filteredItems = t0;
       if (filteredItems.length === 0) {
-        let t4;
-        if ($[5] === Symbol.for("react.memo_cache_sentinel")) {
-          t4 = (
+        let t3;
+        if ($[4] === Symbol.for("react.memo_cache_sentinel")) {
+          t3 = (
             <div>
               <span />
             </div>
           );
-          $[5] = t4;
+          $[4] = t3;
         } else {
-          t4 = $[5];
+          t3 = $[4];
         }
-        t2 = t4;
+        t2 = t3;
         break bb0;
       }
 
-      t1 = filteredItems.map(_temp);
+      t1 = filteredItems.map(_temp2);
     }
     $[0] = items;
     $[1] = t1;
@@ -90,19 +80,23 @@ function Component() {
     return t2;
   }
   let t3;
-  if ($[6] !== t1) {
+  if ($[5] !== t1) {
     t3 = <>{t1}</>;
-    $[6] = t1;
-    $[7] = t3;
+    $[5] = t1;
+    $[6] = t3;
   } else {
-    t3 = $[7];
+    t3 = $[6];
   }
   return t3;
 }
-function _temp(t0) {
+function _temp2(t0) {
   const [item_0] = t0;
   return <Stringify item={item_0} />;
 }
+function _temp(t0) {
+  const [item] = t0;
+  return item.name != null;
+}
 
 ```
       
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/resolve-react-hooks-based-on-import-name.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/resolve-react-hooks-based-on-import-name.expect.md
index dc3081321e90d..496d61df9d7a7 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/resolve-react-hooks-based-on-import-name.expect.md
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/resolve-react-hooks-based-on-import-name.expect.md
@@ -38,7 +38,7 @@ function Component() {
   let t0;
   if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
     t0 = () => {
-      setState((s) => s + 1);
+      setState(_temp);
     };
     $[0] = t0;
   } else {
@@ -67,6 +67,9 @@ function Component() {
   }
   return t2;
 }
+function _temp(s) {
+  return s + 1;
+}
 
 export const FIXTURE_ENTRYPOINT = {
   fn: Component,