From 1af5b5725e8faa184b0f8db3ec16be568e5bf9db Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 11 Sep 2026 22:45:14 +0000 Subject: [PATCH 1/3] react_compiler: return PruneNonEscapingScopes invariants as errors `(m = p.f()) ? 1 : 0` puts the assignment to `m` in the test of a conditional. CollectDependenciesVisitor does not visit that test, so the reactive scope that reassigns `m` gets no scope node, but visit_scope still adds the scope to the node of `m`. When `m` is then force-memoized, force_memoize_scope_dependencies finds no node. Upstream raises `Invariant: Expected a node for all scopes` here and leaves that one function uncompiled. The port had `.expect()`, which aborts the whole build because Bun builds with panic=abort. The four upstream invariants in this pass now return `cold_invariant` errors. The three in the visitor callbacks, which return `()`, record the first error in CollectState, the same way assert_scope_instructions_within_scopes does. --- .../prune_non_escaping_scopes.rs | 104 +++++++++++------- .../bundler/transpiler/react-compiler.test.ts | 85 ++++++++++++++ 2 files changed, 149 insertions(+), 40 deletions(-) diff --git a/src/react_compiler/reactive_scopes/prune_non_escaping_scopes.rs b/src/react_compiler/reactive_scopes/prune_non_escaping_scopes.rs index 491076bd79f0..abf1032f484c 100644 --- a/src/react_compiler/reactive_scopes/prune_non_escaping_scopes.rs +++ b/src/react_compiler/reactive_scopes/prune_non_escaping_scopes.rs @@ -12,6 +12,9 @@ use std::collections::HashSet; use crate::collections::IdMap; use crate::collections::IndexSet; +use crate::diagnostics::CompilerError; +use crate::diagnostics::SourceLocation; +use crate::diagnostics::cold_invariant; use crate::hir::ArrayPatternElement; use crate::hir::DeclarationId; use crate::hir::Effect; @@ -52,7 +55,7 @@ use crate::reactive_scopes::visitors::visit_reactive_function; pub(crate) fn prune_non_escaping_scopes( func: &mut ReactiveFunction, env: &mut Environment, -) -> Result<(), crate::diagnostics::CompilerError> { +) -> Result<(), CompilerError> { // First build up a map of which instructions are involved in creating which values, // and which values are returned. let mut state = CollectState::new(); @@ -64,10 +67,13 @@ pub(crate) fn prune_non_escaping_scopes( let visitor = CollectDependenciesVisitor::new(env); let mut visitor_state = (state, Vec::::new()); visit_reactive_function(func, &visitor, &mut visitor_state); - let (state, _) = visitor_state; + let (mut state, _) = visitor_state; + if let Some(err) = state.error.take() { + return Err(err); + } // Then walk outward from the returned values and find all captured operands. - let memoized = compute_memoized_identifiers(&state); + let memoized = compute_memoized_identifiers(&state)?; // Prune scopes that do not declare/reassign any escaping values let mut transform = PruneScopesTransform { @@ -139,6 +145,9 @@ struct CollectState { identifiers: IdMap, scopes: IdMap, escaping_values: IndexSet, + /// The first invariant violation. The visitor callbacks return `()`, so the + /// entry point reads it back once the walk ends. + error: Option, } impl CollectState { @@ -148,9 +157,17 @@ impl CollectState { identifiers: IdMap::new(), scopes: IdMap::new(), escaping_values: IndexSet::new(), + error: None, } } + /// TS: `CompilerError.invariant(identifierNode !== undefined, ...)` + fn identifier_not_initialized(&mut self, loc: Option) { + self.error.get_or_insert_with(|| { + cold_invariant("Expected identifier to be initialized", None, loc) + }); + } + /// Declare a new identifier, used for function id and params. fn declare(&mut self, id: DeclarationId) { self.identifiers.insert( @@ -189,10 +206,10 @@ impl CollectState { }); // Avoid unused variable warning — we needed the entry to exist let _ = node; - let identifier_node = self - .identifiers - .get_mut(identifier) - .expect("Expected identifier to be initialized"); + let Some(identifier_node) = self.identifiers.get_mut(identifier) else { + self.identifier_not_initialized(place.loc); + return; + }; identifier_node.scopes.insert(scope_id); } } @@ -1027,17 +1044,16 @@ impl<'a> ReactiveFunctionVisitor for CollectDependenciesVisitor<'a> { self.traverse_terminal(stmt, state); // Handle return terminals - if let ReactiveTerminal::Return { value, .. } = &stmt.terminal { + if let ReactiveTerminal::Return { value, loc, .. } = &stmt.terminal { let env = self.env; let decl = env.identifiers[value.identifier.0 as usize].declaration_id; state.0.escaping_values.insert(decl); // If the return is within a scope, associate those scopes with the returned value - let identifier_node = state - .0 - .identifiers - .get_mut(decl) - .expect("Expected identifier to be initialized"); + let Some(identifier_node) = state.0.identifiers.get_mut(decl) else { + state.0.identifier_not_initialized(*loc); + return; + }; for scope_id in &state.1 { identifier_node.scopes.insert(*scope_id); } @@ -1052,12 +1068,12 @@ impl<'a> ReactiveFunctionVisitor for CollectDependenciesVisitor<'a> { // If a scope reassigns any variables, set the chain of active scopes as a dependency // of those variables. for reassignment_id in &scope_data.reassignments { - let decl = env.identifiers[reassignment_id.0 as usize].declaration_id; - let identifier_node = state - .0 - .identifiers - .get_mut(decl) - .expect("Expected identifier to be initialized"); + let reassignment = &env.identifiers[reassignment_id.0 as usize]; + let Some(identifier_node) = state.0.identifiers.get_mut(reassignment.declaration_id) + else { + state.0.identifier_not_initialized(reassignment.loc); + return; + }; for s in &state.1 { identifier_node.scopes.insert(*s); } @@ -1083,7 +1099,9 @@ type IdentNodeTuple = ( bool, ); -fn compute_memoized_identifiers(state: &CollectState) -> HashSet { +fn compute_memoized_identifiers( + state: &CollectState, +) -> Result, CompilerError> { let mut memoized = HashSet::new(); // We need mutable access to the nodes, so we clone the state into mutable structures @@ -1112,12 +1130,12 @@ fn compute_memoized_identifiers(state: &CollectState) -> HashSet identifier_nodes: &mut IdMap, scope_nodes: &mut IdMap, bool)>, memoized: &mut HashSet, - ) -> bool { + ) -> Result { let Some(&(level, _, _, _, seen)) = identifier_nodes.get(id) else { - return false; + return Ok(false); }; if seen { - return identifier_nodes.get(id).unwrap().1; + return Ok(identifier_nodes.get(id).unwrap().1); } // Mark as seen, temporarily mark as non-memoized @@ -1134,7 +1152,7 @@ fn compute_memoized_identifiers(state: &CollectState) -> HashSet .collect(); let mut has_memoized_dependency = false; for dep in deps { - let is_dep_memoized = visit(dep, false, identifier_nodes, scope_nodes, memoized); + let is_dep_memoized = visit(dep, false, identifier_nodes, scope_nodes, memoized)?; has_memoized_dependency |= is_dep_memoized; } @@ -1153,10 +1171,15 @@ fn compute_memoized_identifiers(state: &CollectState) -> HashSet .copied() .collect(); for scope_id in scopes { - force_memoize_scope_dependencies(scope_id, identifier_nodes, scope_nodes, memoized); + force_memoize_scope_dependencies( + scope_id, + identifier_nodes, + scope_nodes, + memoized, + )?; } } - identifier_nodes.get(id).unwrap().1 + Ok(identifier_nodes.get(id).unwrap().1) } fn force_memoize_scope_dependencies( @@ -1164,20 +1187,21 @@ fn compute_memoized_identifiers(state: &CollectState) -> HashSet identifier_nodes: &mut IdMap, scope_nodes: &mut IdMap, bool)>, memoized: &mut HashSet, - ) { - let seen = scope_nodes - .get(id) - .expect("Expected a node for all scopes") - .1; - if seen { - return; + ) -> Result<(), CompilerError> { + // TS: CompilerError.invariant(node !== undefined, ...) + let Some(node) = scope_nodes.get_mut(id) else { + return Err(cold_invariant("Expected a node for all scopes", None, None)); + }; + if node.1 { + return Ok(()); } - scope_nodes.get_mut(id).unwrap().1 = true; // seen = true + node.1 = true; // seen = true - let deps: Vec = scope_nodes.get(id).unwrap().0.clone(); + let deps: Vec = node.0.clone(); for dep in deps { - visit(dep, true, identifier_nodes, scope_nodes, memoized); + visit(dep, true, identifier_nodes, scope_nodes, memoized)?; } + Ok(()) } // Walk from the "roots" aka returned/escaping identifiers @@ -1189,10 +1213,10 @@ fn compute_memoized_identifiers(state: &CollectState) -> HashSet &mut identifier_nodes, &mut scope_nodes, &mut memoized, - ); + )?; } - memoized + Ok(memoized) } // ============================================================================= @@ -1216,7 +1240,7 @@ impl<'a> ReactiveFunctionTransform for PruneScopesTransform<'a> { &mut self, scope: &mut ReactiveScopeBlock, state: &mut HashSet, - ) -> Result, crate::diagnostics::CompilerError> { + ) -> Result, CompilerError> { self.visit_scope(scope, state)?; let scope_id = scope.scope; @@ -1252,7 +1276,7 @@ impl<'a> ReactiveFunctionTransform for PruneScopesTransform<'a> { &mut self, instruction: &mut ReactiveInstruction, state: &mut HashSet, - ) -> Result, crate::diagnostics::CompilerError> { + ) -> Result, CompilerError> { self.traverse_instruction(instruction, state)?; match &mut instruction.value { diff --git a/test/bundler/transpiler/react-compiler.test.ts b/test/bundler/transpiler/react-compiler.test.ts index 64c75ba6b411..56a2f6399990 100644 --- a/test/bundler/transpiler/react-compiler.test.ts +++ b/test/bundler/transpiler/react-compiler.test.ts @@ -1265,6 +1265,91 @@ describe("bundler", () => { }, }); + // prune_non_escaping_scopes does not visit the test of a `?:`. A reactive + // scope that only the test reaches gets no node, but the local that scope + // reassigns still refers to it. Once that local has to be memoized, Babel + // raises `Invariant: Expected a node for all scopes` and skips that one + // function. The last two functions are the controls. + // + // A compiled function calls the fake `c` once per render, so the second + // number of each pair is 1 for a compiled function and 0 for a skipped one. + itBundled("react-compiler/AssignmentInConditionalTestSkipsOnlyThatFunction", { + files: { + "/entry.jsx": /* jsx */ ` + import { calls } from "react/compiler-runtime"; + + function CallInTest(p) { + let m; + const r = (m = p.f()) ? 1 : 0; + return
; + } + function ArrayInTest(p) { + let m; + const r = (m = [p.a]) ? 1 : 0; + return
; + } + function ObjectInTest(p) { + let m = null; + const r = (m = { a: p.a }) ? p.b : p.c; + return
; + } + function useInHook(p) { + "use memo"; + let m; + const r = ((m = p.f()) ? 1 : 0) + 1; + return [m, r]; + } + function AssignedLocalNotMemoized(p) { + let m; + const r = (m = p.f()) ? 1 : 0; + return
; + } + function Plain({ name }) { + return
Hello {name}
; + } + + function render(fn, props) { + const before = calls(); + const result = fn(props); + return [result.props ?? result, calls() - before]; + } + console.log(JSON.stringify({ + CallInTest: render(CallInTest, { f: () => "x" }), + ArrayInTest: render(ArrayInTest, { a: 1 }), + ObjectInTest: render(ObjectInTest, { a: 1, b: 2, c: 3 }), + useInHook: render(useInHook, { f: () => "y" }), + AssignedLocalNotMemoized: render(AssignedLocalNotMemoized, { f: () => "z" }), + Plain: render(Plain, { name: "bun" }), + })); + `, + "/node_modules/react/package.json": `{"name":"react","main":"./index.js"}`, + "/node_modules/react/index.js": `exports.createElement = () => null;`, + "/node_modules/react/jsx-runtime.js": `exports.jsx = exports.jsxs = (type, props) => ({ type, props });`, + "/node_modules/react/jsx-dev-runtime.js": `exports.jsxDEV = (type, props) => ({ type, props });`, + "/node_modules/react/compiler-runtime.js": ` + let count = 0; + exports.c = size => { + count++; + return new Array(size).fill(Symbol.for("react.memo_cache_sentinel")); + }; + exports.calls = () => count; + `, + }, + reactCompiler: true, + target: "browser", + backend: "cli", + run: { + stdout: JSON.stringify({ + CallInTest: [{ "data-v": ["x", 1] }, 0], + ArrayInTest: [{ "data-v": [[1]], "data-r": 1 }, 0], + ObjectInTest: [{ "data-v": { m: { a: 1 }, r: 2 } }, 0], + useInHook: [["y", 2], 0], + AssignedLocalNotMemoized: [{ "data-m": "z", "data-r": 1 }, 1], + Plain: [{ children: ["Hello ", "bun"] }, 1], + }), + }, + }); + // A 0-arg call to an unknown import is non-reactive in InferReactivePlaces // (no operand is reactive, callee isn't a hook), so its scope's deps prune // to empty and it becomes a sentinel-only block. Babel does the same; this From 00605ddc87eb388e8032b5fe78bea5bb02b327a5 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 11 Sep 2026 23:17:48 +0000 Subject: [PATCH 2/3] react_compiler: shorten the CollectState::error comment to one line --- .../reactive_scopes/prune_non_escaping_scopes.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/react_compiler/reactive_scopes/prune_non_escaping_scopes.rs b/src/react_compiler/reactive_scopes/prune_non_escaping_scopes.rs index abf1032f484c..0706b3125366 100644 --- a/src/react_compiler/reactive_scopes/prune_non_escaping_scopes.rs +++ b/src/react_compiler/reactive_scopes/prune_non_escaping_scopes.rs @@ -145,8 +145,7 @@ struct CollectState { identifiers: IdMap, scopes: IdMap, escaping_values: IndexSet, - /// The first invariant violation. The visitor callbacks return `()`, so the - /// entry point reads it back once the walk ends. + /// First invariant violation. The `()` visitor callbacks cannot return it. error: Option, } From b9caeb0346884512924b074aad68675ec807a9c4 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 12 Sep 2026 04:50:39 +0000 Subject: [PATCH 3/3] test: cover the conditional in a try in an IIFE (facebook/react#37228) --- test/bundler/transpiler/react-compiler.test.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/bundler/transpiler/react-compiler.test.ts b/test/bundler/transpiler/react-compiler.test.ts index 56a2f6399990..f30a89138612 100644 --- a/test/bundler/transpiler/react-compiler.test.ts +++ b/test/bundler/transpiler/react-compiler.test.ts @@ -1299,6 +1299,22 @@ describe("bundler", () => { const r = ((m = p.f()) ? 1 : 0) + 1; return [m, r]; } + // https://github.com/facebook/react/issues/37228: the same invariant + // with no assignment, from a \`?:\` in a \`try\` in an inlined IIFE. + const check = v => v.ok; + function useValue() { + return { ok: true }; + } + function ConditionalInTryInIife() { + const raw = useValue(); + return (() => { + try { + return check(raw) ? raw : null; + } catch { + return null; + } + })(); + } function AssignedLocalNotMemoized(p) { let m; const r = (m = p.f()) ? 1 : 0; @@ -1318,6 +1334,7 @@ describe("bundler", () => { ArrayInTest: render(ArrayInTest, { a: 1 }), ObjectInTest: render(ObjectInTest, { a: 1, b: 2, c: 3 }), useInHook: render(useInHook, { f: () => "y" }), + ConditionalInTryInIife: render(ConditionalInTryInIife), AssignedLocalNotMemoized: render(AssignedLocalNotMemoized, { f: () => "z" }), Plain: render(Plain, { name: "bun" }), })); @@ -1344,6 +1361,7 @@ describe("bundler", () => { ArrayInTest: [{ "data-v": [[1]], "data-r": 1 }, 0], ObjectInTest: [{ "data-v": { m: { a: 1 }, r: 2 } }, 0], useInHook: [["y", 2], 0], + ConditionalInTryInIife: [{ ok: true }, 0], AssignedLocalNotMemoized: [{ "data-m": "z", "data-r": 1 }, 1], Plain: [{ children: ["Hello ", "bun"] }, 1], }),