react_compiler: fix stale values for a let reassigned inside a memo block

.claude/skills/sync-react-compiler.md

@@ -44,7 +44,10 @@ the porting reference for the AST-boundary files.
 2. **Apply whole-crate diffs mechanically.** For each hunk under the
    whole-crate section, apply it to the corresponding `src/react_compiler/<dir>/`
    file. The only systematic edit is import paths (`react_compiler_hir::` →
-   `crate::hir::`, etc.); everything else lands verbatim.
+   `crate::hir::`, etc.); everything else lands verbatim. A few sites fix
+   miscompiles that upstream still has. They carry a "Not in upstream" comment,
+   and the script lists them at the end of its output. DESIGN.md says why each
+   exists, under "Behaviour that differs from upstream". Keep them.
 
 3. **Re-port AST-boundary diffs by hand.** For each hunk under the
    AST-boundary section, re-port it into the named Bun file using the
@@ -54,7 +57,8 @@ the porting reference for the AST-boundary files.
    `react_compiler_ast::statements::Statement::Foo { … }`, the Bun port calls
    `Stmt::alloc(S::Foo { … }, loc)`. Keep control flow, pass ordering,
    variable names, and comments 1:1 with upstream — only the AST reads/writes
-   change.
+   change. The "Not in upstream" rule from step 2 applies here too: `codegen.rs`
+   has such a site (`codegen_reactive_scope`). Keep it.
 
    For large diffs, fan out one agent per file with the upstream diff + the Bun
    port + DESIGN.md as context, then adversarially review each port.

scripts/sync-react-compiler.sh

@@ -129,4 +129,10 @@ git -C "$tmp" grep -l '\breact_compiler_ast\b' -- 'compiler/crates/*/src/*.rs' \
   || echo "(none)"
 echo
 
+# See "Behaviour that differs from upstream" in src/react_compiler/DESIGN.md.
+echo "### sites that fix what upstream still gets wrong (keep them) ###"
+git -C "$repo_root" grep -n 'Not in upstream' -- src/react_compiler ':!src/react_compiler/DESIGN.md' \
+  || echo "(none)"
+echo
+
 echo "When porting is done: echo $new > $ported_file"

src/react_compiler/DESIGN.md

@@ -66,6 +66,75 @@ Keep the diff between upstream and the port as small as the type substitution
 allows — the `/sync-react-compiler` skill re-ports upstream changes hunk by
 hunk, so gratuitous restructuring makes that harder.
 
+### Behaviour that differs from upstream
+
+A few sites fix miscompiles that upstream still has. Each one carries a
+one-line comment that starts with "Not in upstream":
+
+```sh
+git grep -n "Not in upstream" src/react_compiler
+```
+
+Keep them on a sync. `react-compiler-fixtures.test.ts` cannot tell whether they
+survived: it compiles the upstream fixtures in `infer` mode, which skips most
+of their functions, and none of these sites changes what Bun emits for one. The
+tests in `test/bundler/transpiler/react-compiler.test.ts` that run compiled
+components can. When upstream fixes the same case, run Bun's test for it
+against upstream's fix before you drop Bun's.
+
+The code comments are short on purpose. The reasons for a group of sites go
+here.
+
+#### A `let` declared before a reactive scope and reassigned inside it
+
+facebook/react#37224. The scope stores the variable after the computation and
+restores it when the cache hits, so the variable is an output of the scope.
+`react-compiler/ReassignedLocalDeclaredBeforeMemoBlock` runs a component for
+each point below.
+
+- `visit_phi` (`propagate_scope_dependencies_hir.rs`). A path that does not
+  reassign the variable leaves it with the value it had on entry. Only a phi
+  reads that value, and upstream visits instruction operands only. So the value
+  is not a dependency, and a cache hit restores the value of an older render.
+  `visit_phi` runs in the block loop that already walks the phis. It makes an
+  operand a dependency when `reassignments` dates it before the current scope.
+  An operand that `reassignments` does not hold yet comes from a loop back
+  edge, so its definition is later in the scope.
+- `visit_phi` also records the phi itself in `reassignments`, dated just before
+  its block, with an empty scope stack (nothing reads the scope stack of a
+  `reassignments` entry). Upstream never records a phi, so
+  `check_valid_dependency` dates a read of one by the first declaration of its
+  variable, and a read of a phi inside the scope becomes a dependency. Codegen
+  reads a dependency by name before the scope, where the variable still holds
+  the value on entry. That is the wrong value, and it throws when the read is
+  `u.profile.name` and the value on entry is null.
+- `apply_reactive_flags_replay` (`infer_reactive_places.rs`) flags every phi
+  operand whose identifier is reactive. Upstream stops at the first reactive
+  operand of a phi, so the flag of a later operand can be unset. `visit_phi`
+  copies that flag into the dependency.
+- `prune_non_reactive_dependencies.rs`, `|| dep.reactive`. The value on entry
+  can be a parameter, or a phi that only another phi reads. No place in the
+  reactive function carries that identifier, so the set the pass builds never
+  holds it.
+- `exit_scope`. Upstream records a reassignment on the innermost scope. When the
+  cache of an enclosing scope hits, the inner scope does not run, so the
+  enclosing scope has to restore the variable too.
+- `handle_instruction`, `PrefixUpdate` and `PostfixUpdate`. Upstream visits only
+  `value`, so `x++` is not a reassignment and the scope does not restore `x`.
+- `codegen_reactive_scope` (`codegen.rs`). The cache stores run after the
+  computation, so a dependency on a variable the scope reassigns stored the new
+  value, and the next comparison tested the value on entry against it. The
+  dependency is copied to a `const` before the scope, and the comparison and
+  the store use the copy. `rename_variables` has run by then, so
+  `fresh_temporary_name` picks the first unused `t<n>`.
+
+The open upstream fix, facebook/react#37273, adds the same dependency. It
+differs in three ways. It tests `operand.reactive` without the change to
+`apply_reactive_flags_replay`. It stores the dependency before the computation,
+which breaks the rule that the cache never holds inputs without outputs (React
+shares the cache between render attempts). It does not pass a reassignment to
+the enclosing scope.
+
 ### Type mapping (input: lowering)
 
 | upstream `react_compiler_ast`                                            | `bun_ast`                                                                      |

src/react_compiler/codegen.rs

@@ -452,6 +452,22 @@ impl<'a, 'h> Context<'a, 'h> {
         validated
     }
 
+    /// The first `t<n>` that no identifier uses, for a temporary that only codegen creates.
+    fn fresh_temporary_name(&mut self) -> String {
+        let mut itoa = bun_core::fmt::ItoaBuf::new();
+        let mut name = String::new();
+        for index in 0u32.. {
+            name.clear();
+            name.push('t');
+            name.push_str(itoa.format(index));
+            if !self.unique_identifiers.contains(&name) {
+                break;
+            }
+        }
+        self.unique_identifiers.insert(name.clone());
+        name
+    }
+
     fn record_error(&mut self, detail: CompilerErrorDetail) -> Result<(), CompilerError> {
         self.env.record_error(detail)
     }
@@ -680,17 +696,49 @@ fn codegen_reactive_scope(
 
     for dep in &deps {
         let index = cx.alloc_cache_index();
+        let dep_decl = cx.env.identifiers[dep.identifier.0 as usize].declaration_id;
+        // Not in upstream: the stores run last, so a dependency the scope reassigns is copied first.
+        let value_on_entry = if scope_reassignments
+            .iter()
+            .any(|id| cx.env.identifiers[id.0 as usize].declaration_id == dep_decl)
+        {
+            let name = store_str(cx.fresh_temporary_name().as_bytes());
+            let copy_ref = cx.cg.ref_for_local(name);
+            statements.push(Stmt::alloc(
+                S::Local {
+                    kind: S::Kind::KConst,
+                    decls: decl_list([G::Decl {
+                        binding: Binding::alloc(
+                            cx.cg.arena,
+                            b::Identifier { r#ref: copy_ref },
+                            loc,
+                        ),
+                        value: Some(codegen_dependency(cx, dep)?),
+                    }]),
+                    ..Default::default()
+                },
+                loc,
+            ));
+            Some(name)
+        } else {
+            None
+        };
+        let dep_expr = |cx: &mut Context| match value_on_entry {
+            Some(name) => Ok(Expr::init_identifier(cx.cg.ref_for_local(name), loc)),
+            None => codegen_dependency(cx, dep),
+        };
+
         let comparison = Expr::init(
             E::Binary {
                 op: OpCode::BinStrictNe,
                 left: cache_slot(index),
-                right: codegen_dependency(cx, dep)?,
+                right: dep_expr(cx)?,
             },
             loc,
         );
         change_exprs.push(comparison);
 
-        let dep_value = codegen_dependency(cx, dep)?;
+        let dep_value = dep_expr(cx)?;
         cache_store_exprs.push(Expr::init(
             E::Binary {
                 op: OpCode::BinAssign,

src/react_compiler/inference/infer_reactive_places.rs

@@ -581,6 +581,10 @@ fn apply_reactive_flags_replay(
                         operand.reactive = true;
                     }
                 }
+                // Not in upstream: the fixpoint stops at the first reactive operand. Flag the rest.
+                if reactive_ids[operand.identifier.0 as usize] {
+                    operand.reactive = true;
+                }
             }
         }
 

src/react_compiler/inference/propagate_scope_dependencies_hir.rs

@@ -1781,11 +1781,72 @@ impl<'a> DependencyCollectionContext<'a> {
             }
         }
 
+        // Not in upstream: the enclosing scope has to restore what this scope reassigns.
+        if let Some(parent) = self.current_scope() {
+            let parent_start = env.scopes[parent.0 as usize].range.start;
+            for index in 0..env.scopes[scope_id.0 as usize].reassignments.len() {
+                let id = env.scopes[scope_id.0 as usize].reassignments[index];
+                let decl_id = env.identifiers[id.0 as usize].declaration_id;
+                let declared_before_parent = self
+                    .declarations
+                    .get(decl_id)
+                    .is_some_and(|decl| decl.id < parent_start);
+                let already = env.scopes[parent.0 as usize]
+                    .reassignments
+                    .iter()
+                    .any(|other| env.identifiers[other.0 as usize].declaration_id == decl_id);
+                if declared_before_parent && !already {
+                    env.scopes[parent.0 as usize].reassignments.push(id);
+                }
+            }
+        }
+
         if !pruned {
             self.deps.insert(scope_id, scoped_deps);
         }
     }
 
+    /// Not in upstream: a value from before the scope that reaches a phi in it is a dependency.
+    fn visit_phi(
+        &mut self,
+        phi: &crate::hir::Phi,
+        before_block: EvaluationOrder,
+        env: &mut Environment,
+    ) {
+        for (_pred_id, operand) in &phi.operands {
+            if let Some(maybe_optional_chain) = self.temporaries.get(operand.identifier) {
+                self.visit_dependency(maybe_optional_chain.clone(), env);
+                continue;
+            }
+            // An operand that is not declared yet comes from a loop back edge, later in the scope.
+            let (Some(scope), Some(decl)) = (
+                self.current_scope(),
+                self.reassignments.get(operand.identifier),
+            ) else {
+                continue;
+            };
+            if decl.id < env.scopes[scope.0 as usize].range.start {
+                self.visit_dependency(
+                    ReactiveScopeDependency {
+                        identifier: operand.identifier,
+                        reactive: operand.reactive,
+                        path: hir_vec![],
+                        loc: operand.loc,
+                    },
+                    env,
+                );
+            }
+        }
+        // A read of this phi inside the scope it sits in is then not a dependency of that scope.
+        self.reassignments.insert(
+            phi.place.identifier,
+            Decl {
+                id: before_block,
+                scope_stack: Vec::new(),
+            },
+        );
+    }
+
     fn current_scope(&self) -> Option<ScopeId> {
         self.scope_stack.last().copied()
     }
@@ -2134,6 +2195,25 @@ fn handle_instruction(
             ctx.visit_operand(&lvalue.place, env);
             ctx.visit_operand(val, env);
         }
+        // Not in upstream: `x++` reassigns `x`, so the scope has to restore it on a cache hit.
+        InstructionValue::PrefixUpdate {
+            lvalue, value: val, ..
+        }
+        | InstructionValue::PostfixUpdate {
+            lvalue, value: val, ..
+        } => {
+            ctx.visit_operand(val, env);
+            ctx.visit_reassignment(lvalue, env);
+            let scope_stack_copy = ctx.scope_stack.clone();
+            ctx.declare(
+                lvalue.identifier,
+                Decl {
+                    id,
+                    scope_stack: scope_stack_copy,
+                },
+                env,
+            );
+        }
         _ => {
             // Visit all value operands
             let operands = visitors::each_instruction_value_operand(&instr.value, env);
@@ -2207,11 +2287,13 @@ fn handle_function_deps(
         }
 
         // Record phi operands
-        for phi in &block.phis {
-            for (_pred_id, operand) in &phi.operands {
-                if let Some(maybe_optional_chain) = ctx.temporaries.get(operand.identifier) {
-                    ctx.visit_dependency(maybe_optional_chain.clone(), env);
-                }
+        if !block.phis.is_empty() {
+            let first = match block.instructions.first() {
+                Some(instr_id) => func.instructions[instr_id.0 as usize].id,
+                None => block.terminal.evaluation_order(),
+            };
+            for phi in &block.phis {
+                ctx.visit_phi(phi, EvaluationOrder(first.0.saturating_sub(1)), env);
             }
         }
 

src/react_compiler/reactive_scopes/prune_non_reactive_dependencies.rs

@@ -214,9 +214,10 @@ impl<'a> ReactiveFunctionTransform for PruneVisitor<'a> {
         let scope_data = &mut self.env.scopes[scope_id.0 as usize];
 
         // Remove non-reactive dependencies
+        // Not in upstream: `|| dep.reactive`, for a parameter or a phi that only a phi reads.
         scope_data
             .dependencies
-            .retain(|dep| state.contains(&dep.identifier));
+            .retain(|dep| state.contains(&dep.identifier) || dep.reactive);
 
         // If any deps remain, mark all declarations and reassignments as reactive
         if !scope_data.dependencies.is_empty() {