From d9b3841ca64f292e3450edcd0a87eca05ac4c5f9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= <sebastian@calyptus.eu>
Date: Tue, 12 Nov 2024 12:46:55 -0500
Subject: [PATCH 1/4] Revert "Performance tracks are sorted by start time" hack
 (#31518)

This reverts commit d3bf32a95806b6d583ef041b8d83781cd686cfd8 which was
part of #30983

When you have very deep trees this trick can cause the top levels to
skew way too much from the real numbers. Creating unbalanced trees.

The bug should have been fixed in Chrome Canary now so that entries
added later are sorted to go first which should've addressed this issue.
---
 packages/react-reconciler/src/ReactProfilerTimer.js | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/packages/react-reconciler/src/ReactProfilerTimer.js b/packages/react-reconciler/src/ReactProfilerTimer.js
index 0df3427fe8..e59b5cb8e5 100644
--- a/packages/react-reconciler/src/ReactProfilerTimer.js
+++ b/packages/react-reconciler/src/ReactProfilerTimer.js
@@ -193,13 +193,8 @@ export function popComponentEffectStart(prevEffectStart: number): void {
   if (!enableProfilerTimer || !enableProfilerCommitHooks) {
     return;
   }
-  if (prevEffectStart < 0) {
-    // If the parent component didn't have a start time, we use the start
-    // of the child as the parent's start time. We subtrack a minimal amount of
-    // time to ensure that the parent's start time is before the child to ensure
-    // that the performance tracks line up in the right order.
-    componentEffectStartTime -= 0.001;
-  } else {
+  // If the parent component didn't have a start time, we let this current time persist.
+  if (prevEffectStart >= 0) {
     // Otherwise, we restore the previous parent's start time.
     componentEffectStartTime = prevEffectStart;
   }

From 3770c11011c78d227c7013468787e0c34ae91dc8 Mon Sep 17 00:00:00 2001
From: mofeiZ <34200447+mofeiZ@users.noreply.github.com>
Date: Tue, 12 Nov 2024 14:04:54 -0500
Subject: [PATCH 2/4] [compiler] repro for reactive ref.current accesses
 (#31519)

See test fixture
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31519).
* #31521
* __->__ #31519
---
 .../compiler/bug-nonreactive-ref.expect.md    | 89 +++++++++++++++++++
 .../fixtures/compiler/bug-nonreactive-ref.tsx | 33 +++++++
 .../packages/snap/src/SproutTodoFilter.ts     |  1 +
 3 files changed, 123 insertions(+)
 create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-nonreactive-ref.expect.md
 create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-nonreactive-ref.tsx

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
new file mode 100644
index 0000000000..f79df15d52
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-nonreactive-ref.expect.md
@@ -0,0 +1,89 @@
+
+## 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
new file mode 100644
index 0000000000..a0115f2df3
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-nonreactive-ref.tsx
@@ -0,0 +1,33 @@
+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/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts
index 1971fe0d33..6c136f6310 100644
--- a/compiler/packages/snap/src/SproutTodoFilter.ts
+++ b/compiler/packages/snap/src/SproutTodoFilter.ts
@@ -484,6 +484,7 @@ 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 366938753f68b0085a89309dc04ce4451b197993 Mon Sep 17 00:00:00 2001
From: Mofei Zhang <feifei0@meta.com>
Date: Tue, 12 Nov 2024 14:05:24 -0500
Subject: [PATCH 3/4] [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  |  32 +++---
 .../ref-parameter-mutate-in-effect.js         |   2 +-
 .../packages/snap/src/SproutTodoFilter.ts     |   1 -
 11 files changed, 273 insertions(+), 148 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 bbec25a57c..7fd44c29dc 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 610e93965f..3851234005 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 f79df15d52..0000000000
--- 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 a0115f2df3..0000000000
--- 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 0000000000..c8922ab0c4
--- /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 0000000000..c3eea7a243
--- /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 0000000000..c3876c2292
--- /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 0000000000..db95c5c204
--- /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 8b5a2eb1a0..7a10cfe48e 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;
     };
-    t1 = [];
-    $[0] = t0;
-    $[1] = t1;
+    $[0] = ref;
+    $[1] = t0;
   } else {
-    t0 = $[0];
-    t1 = $[1];
+    t0 = $[1];
+  }
+  let t1;
+  if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
+    t1 = [];
+    $[2] = t1;
+  } else {
+    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 c60141e8d5..ffd9eae08d 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 6c136f6310..1971fe0d33 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 781222610f3eee098dbd6e4ba548e552ba4717cd Mon Sep 17 00:00:00 2001
From: Mofei Zhang <feifei0@meta.com>
Date: Tue, 12 Nov 2024 14:05:31 -0500
Subject: [PATCH 4/4] [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  | 69 ++++++++++------
 .../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, 160 insertions(+), 171 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 1d2e155848..855bc039ab 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 7fd44c29dc..8aed17f8ee 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);
-    }
+  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 (
+          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);
+        }
+      }
 
-    // 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);
+      if (!processedInstrsInOptional.has(block.terminal)) {
+        for (const place of eachTerminalOperand(block.terminal)) {
+          context.visitOperand(place);
         }
       }
     }
-    for (const instr of block.instructions) {
-      if (!processedInstrsInOptional.has(instr)) {
-        handleInstruction(instr, context);
-      }
-    }
+  };
 
-    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 b31a16da90..c071d5d20e 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 0000000000..ae44f27912
--- /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 0000000000..8447e3960d
--- /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 955d391f91..940b3975c1 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 db69bc2821..0000000000
--- 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 cb6f65a9f4..0000000000
--- 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 b66661fbca..41994e1e56 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 b141c27614..9ce4a62e71 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 02e60eff91..ed56ff0681 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 157e2de81a..bb99a5d90f 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];