react-compiler: declare what is outlined from a nested function in the scope that holds it

src/js_parser/p.rs

@@ -457,9 +457,12 @@ pub struct P<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> {
     pub(crate) react_compiler_result: Option<bun_react_compiler::CompileResult>,
     /// Visiting a function that is handed to the React Compiler, which re-declares its locals as new symbols.
     pub(crate) react_compiler_may_replace_body: bool,
-    /// Functions the compiler outlined from a function that isn't at the top level. `visit_stmts` puts them after the
-    /// statement it is visiting.
+    /// Functions outlined from a nested function, until the `visit_stmts` call that visits its statement takes them.
     pub(crate) react_compiler_outlined_from_nested: Vec<Stmt>,
+    /// The scope of the statement list that the innermost `visit_stmts` call visits.
+    pub(crate) react_compiler_list_scope: js_ast::StoreRef<js_ast::Scope>,
+    /// Whether the source has the word `eval`, once a nested function in a block has asked.
+    pub(crate) react_compiler_source_has_eval: Option<bool>,
 
     /// only applicable when `.options.features.server_components` is
     /// configured to wrap exports. populated before visit pass starts.
@@ -3435,7 +3438,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
         self.runtime_imports.put(b"__require", ref_);
     }
 
-    fn will_use_renamer(&self) -> bool {
+    pub(crate) fn will_use_renamer(&self) -> bool {
         self.options.bundle || self.options.features.minify_identifiers
     }
 
@@ -9798,6 +9801,8 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
             react_compiler_result: None,
             react_compiler_may_replace_body: false,
             react_compiler_outlined_from_nested: Vec::new(),
+            react_compiler_list_scope: scope,
+            react_compiler_source_has_eval: None,
             server_components_wrap_ref: Ref::NONE,
             jest: Jest::default(),
             imported_mock_apis: Default::default(),

src/js_parser/react_compiler_host.rs

@@ -8,6 +8,7 @@ use bun_ast as js_ast;
 use bun_collections::VecExt;
 
 use crate::JSXImport;
+use crate::lexer::is_identifier_continue;
 use crate::p::P;
 
 /// What [`P::enter_react_compiler_candidate`] decided about a function.
@@ -22,6 +23,20 @@ pub(crate) struct ReactCompilerCandidate {
     pub(crate) is_top_level: bool,
 }
 
+/// Where what the compiler outlines from a function is declared ([`P::react_compiler_place_of_outlined`]).
+#[derive(Clone, Copy)]
+pub(crate) enum OutlinedPlace {
+    /// In the part that `bun_react_compiler::finish` makes, which declares each name at the top level of the module.
+    ModuleEnd,
+    /// At the end of the statement list of this scope.
+    List(js_ast::StoreRef<js_ast::Scope>),
+    /// In the body of the function itself, whose scope lists its locals from `first_local` on.
+    OwnBody {
+        first_local: usize,
+        own_name: Option<js_ast::Ref>,
+    },
+}
+
 pub struct ReactCompilerHost<'p, 'a, const TS: bool, const SCAN_ONLY: bool> {
     p: &'p mut P<'a, TS, SCAN_ONLY>,
 }
@@ -191,7 +206,189 @@ impl<'a, const TS: bool, const SCAN_ONLY: bool> bun_react_compiler::Host
     }
 }
 
+/// Whether `eval` stands in `source` as a name of its own. In `evaluate` and `retrieval` it is a part of a longer name.
+fn has_the_word_eval(source: &[u8]) -> bool {
+    use bun_core::lexer::{char_and_size, last_char};
+    let mut searched = 0;
+    while let Some(found) = bun_core::strings::index_of(&source[searched..], b"eval") {
+        let start = searched + found;
+        searched = start + b"eval".len();
+        // The character in front of the word is four bytes at most.
+        let (before, _) = last_char(&source[start.saturating_sub(4)..start]);
+        let (after, _) = char_and_size(source, searched);
+        if !is_identifier_continue(before) && !is_identifier_continue(after) {
+            return true;
+        }
+    }
+    false
+}
+
 impl<'a, const TS: bool, const SCAN_ONLY: bool> P<'a, TS, SCAN_ONLY> {
+    /// `current_scope` is the body of the function being compiled, and its parent holds the arguments.
+    fn scope_around_compiled_function(&self) -> Option<js_ast::StoreRef<js_ast::Scope>> {
+        self.current_scope().parent.and_then(|args| args.parent)
+    }
+
+    /// Where what is outlined from the function being compiled is declared. src/react_compiler/DESIGN.md has the rule.
+    pub(crate) fn react_compiler_place_of_outlined(
+        &mut self,
+        pending: &bun_react_compiler::PendingCompile,
+    ) -> OutlinedPlace {
+        use js_ast::scope::Kind;
+        if pending.is_top_level {
+            return OutlinedPlace::ModuleEnd;
+        }
+        let mut around = self.scope_around_compiled_function();
+        let mut is_in_list = false;
+        while let Some(scope) = around {
+            is_in_list |= scope == self.react_compiler_list_scope;
+            if is_in_list {
+                // Only a part can declare a name of the module.
+                if scope == self.module_scope {
+                    return OutlinedPlace::ModuleEnd;
+                }
+                // A block around the statement of a label would take the label from a loop.
+                if scope.kind != Kind::Label {
+                    // A `case` can call a declaration that its block keeps, and skip the `let` of its callback.
+                    if !scope.kind_stops_hoisting()
+                        && (Self::is_kept_in_a_block(pending)
+                            || self.may_hold_a_direct_eval(&scope))
+                    {
+                        break;
+                    }
+                    return OutlinedPlace::List(scope);
+                }
+            } else if !matches!(scope.kind, Kind::Block | Kind::Label) || scope.members.count() > 0
+            {
+                break;
+            }
+            around = scope.parent;
+        }
+        OutlinedPlace::OwnBody {
+            first_local: self.current_scope().generated.len(),
+            own_name: pending.own_name,
+        }
+    }
+
+    /// Beside a direct `eval` a block keeps every function declaration. The `eval` marks its scope only once visited.
+    fn may_hold_a_direct_eval(&mut self, scope: &js_ast::Scope) -> bool {
+        if scope.contains_direct_eval {
+            return true;
+        }
+        if self.react_compiler_source_has_eval.is_none() {
+            let has_eval = has_the_word_eval(&self.source.contents);
+            self.react_compiler_source_has_eval = Some(has_eval);
+        }
+        self.react_compiler_source_has_eval == Some(true)
+    }
+
+    /// A block makes a `let` of a function declaration, but not of one that is async or a generator.
+    fn is_kept_in_a_block(pending: &bun_react_compiler::PendingCompile) -> bool {
+        use js_ast::flags::Function;
+        pending.own_name.is_some()
+            && pending.binding == pending.own_name
+            && (pending.flags.contains(Function::IsAsync)
+                || pending.flags.contains(Function::IsGenerator))
+    }
+
+    fn is_declared_around_compiled_function(&self, name: &[u8]) -> bool {
+        let mut around = self.scope_around_compiled_function();
+        while let Some(scope) = around {
+            if scope.members.get(name).is_some() {
+                return true;
+            }
+            around = scope.parent;
+        }
+        false
+    }
+
+    /// With no renamer, a local would hide a variable of the same name that a function outlined into this body reads.
+    fn rename_locals_named_like_a_variable_around(
+        &mut self,
+        first_local: usize,
+        own_name: Option<js_ast::Ref>,
+    ) {
+        if self.will_use_renamer() {
+            return;
+        }
+        let own_name = own_name.map(|own| self.load_name_from_ref(own));
+        for at in first_local..self.current_scope().generated.len() {
+            let local = self.current_scope().generated[at];
+            let name = self.load_name_from_ref(local);
+            // A read of the function's own name is a local that nothing declares: it keeps its spelling.
+            if own_name != Some(name) && self.is_declared_around_compiled_function(name) {
+                let renamed = self.temp_var_name(name);
+                self.symbols[local.inner_index() as usize].original_name =
+                    js_ast::StoreStr::new(renamed);
+            }
+        }
+    }
+
+    /// Declares in `place` what the nested function that was just compiled outlined. `body` is that function's own.
+    #[cold]
+    #[inline(never)]
+    pub(crate) fn declare_outlined_from_nested(
+        &mut self,
+        rc: &mut bun_react_compiler::ReactCompilerState,
+        outlined_len: usize,
+        place: OutlinedPlace,
+        body: &mut bun_alloc::ArenaVec<'a, js_ast::Stmt>,
+    ) {
+        let (is_in_list, mut declared_in) = match place {
+            OutlinedPlace::ModuleEnd => return,
+            OutlinedPlace::List(scope) => (true, scope),
+            OutlinedPlace::OwnBody {
+                first_local,
+                own_name,
+            } => {
+                self.rename_locals_named_like_a_variable_around(first_local, own_name);
+                (false, self.current_scope)
+            }
+        };
+        for stmt in rc.take_outlined_from(outlined_len) {
+            if let js_ast::StmtData::SFunction(function) = stmt.data
+                && let Some(name) = function.func.name
+            {
+                // The renamer numbers a name with the scope that lists it: take it out of the module scope.
+                let in_module_scope = &mut self.module_scope_mut().generated;
+                if let Some(at) = in_module_scope
+                    .iter()
+                    .rposition(|listed| *listed == name.ref_)
+                {
+                    in_module_scope.remove(at);
+                }
+                VecExt::append(&mut declared_in.generated, name.ref_);
+            }
+            if is_in_list {
+                self.react_compiler_outlined_from_nested.push(stmt);
+            } else {
+                body.push(stmt);
+            }
+        }
+    }
+
+    /// Moves what was outlined since `start` into the visited list. A block gets each as a `let`, through `before`.
+    #[cold]
+    #[inline(never)]
+    pub(crate) fn append_outlined_from_nested(
+        &mut self,
+        start: usize,
+        visited: &mut bun_alloc::ArenaVec<'a, js_ast::Stmt>,
+        before: &mut bun_alloc::ArenaVec<'a, js_ast::Stmt>,
+    ) {
+        let scope = self.current_scope();
+        // The list around the label takes them.
+        if scope.kind == js_ast::scope::Kind::Label {
+            return;
+        }
+        let list = if scope.kind_stops_hoisting() {
+            visited
+        } else {
+            before
+        };
+        list.extend(self.react_compiler_outlined_from_nested.drain(start..));
+    }
+
     /// Drops the replaced body's symbols, or the renamer prints a new local `count` as `count2`.
     pub(crate) fn drop_symbols_of_replaced_function(&mut self, name: Option<js_ast::Ref>) {
         let mut body = self.current_scope;

src/js_parser/visit/mod.rs

@@ -13,6 +13,7 @@ use crate::parser::{
     Ref, RelocateVarsMode, ScopeOrder, StmtsKind, StrictModeFeature, StringVoidMap, VisitArgsOpts,
     VisitDeclOpts, is_eval_or_arguments,
 };
+use crate::react_compiler_host::OutlinedPlace;
 use bun_alloc::{ArenaVec as BumpVec, ArenaVecExt as _};
 use bun_ast as js_ast;
 use bun_ast::G::{Decl, PropertyKind};
@@ -1572,6 +1573,9 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
             // `&mut`) so the pointer shares the local's base tag and survives the
             // direct `&mut before` reborrows in the loop below (Stacked Borrows).
             p.nearest_stmt_list = NonNull::new(core::ptr::addr_of_mut!(before));
+            let prev_list_scope =
+                core::mem::replace(&mut p.react_compiler_list_scope, p.current_scope);
+            let outlined_start = p.react_compiler_outlined_from_nested.len();
 
             let mut preprocessed_enum_i: usize = 0;
 
@@ -1613,8 +1617,11 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
                     break 'list_getter &mut visited;
                 };
                 p.visit_and_append_stmt(list, stmt)?;
-                // Outlined functions can use variables from this scope, so they can't go at the end of the module.
-                visited.extend(p.react_compiler_outlined_from_nested.drain(..));
+            }
+            p.react_compiler_list_scope = prev_list_scope;
+            // Outlined functions can use variables from this scope: they go to the end of its list.
+            if p.react_compiler_outlined_from_nested.len() > outlined_start {
+                p.append_outlined_from_nested(outlined_start, &mut visited, &mut before);
             }
 
             // Transform block-level function declarations into variable declarations
@@ -1798,6 +1805,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
             // `Host::new_local` appends the locals of the compiled function here.
             let generated_len = p.current_scope().generated.len();
             let outlined_len = rc.outlined_len();
+            let outlined_place = p.react_compiler_place_of_outlined(&pending);
             let compiled = {
                 let host = &mut crate::react_compiler_host::ReactCompilerHost::new(p);
                 bun_react_compiler::maybe_compile_pending(
@@ -1808,19 +1816,20 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
                     name,
                 )
             };
-            if !pending.is_top_level {
-                p.react_compiler_outlined_from_nested
-                    .extend(rc.take_outlined_from(outlined_len));
-            }
-            p.react_compiler = Some(rc);
             if let Some((new_body, result)) = compiled {
                 stmts.clear();
                 stmts.extend(new_body);
+                if !matches!(outlined_place, OutlinedPlace::ModuleEnd)
+                    && rc.outlined_len() > outlined_len
+                {
+                    p.declare_outlined_from_nested(&mut rc, outlined_len, outlined_place, stmts);
+                }
                 p.react_compiler_result = Some(result);
                 p.drop_symbols_of_replaced_function(pending.own_name);
             } else {
                 p.current_scope_mut().generated.truncate(generated_len);
             }
+            p.react_compiler = Some(rc);
         }
 
         if !p.should_remove_dead_statements() || !p.options.features.dead_code_elimination {

src/jsc/RuntimeTranspilerCache.rs

@@ -85,7 +85,8 @@ bun_core::declare_scope!(cache, visible);
 /// Version 50: The body of a class with lowered class decorators names the class by the
 /// binding that holds what the decorators return.
 /// Version 51: A `bun:` specifier that names no builtin is printed as it is written.
-const EXPECTED_VERSION: u32 = 51;
+/// Version 52: The React Compiler declares what it outlines from a nested function in the scope that holds it.
+const EXPECTED_VERSION: u32 = 52;
 
 /// Source files smaller than this are not written to / read from the on-disk
 /// transpiler cache. Originally 50 KiB, which excluded almost every file in a

src/react_compiler/DESIGN.md

@@ -128,6 +128,40 @@ function expression). The bundler's renamer then numbers the new symbols like
 the locals of any other function. `Host::new_generated` is for a name declared
 at module level, such as an outlined `_temp`.
 
+What is outlined from a function that is nested in another one can read that
+function's variables: `Host::is_module_level` is true for every binding outside
+the compiled function. So the parser declares it where the nested function can
+reach it and where it can reach those variables:
+
+- At the end of the statement list that holds the nested function. In a block
+  it is a `let`, like the block's own function declarations. A label, the head
+  of a `for` that declares nothing and a `switch` with no `let` count as the
+  list around them. When that list is the module's, it goes to the end of the
+  module like what is outlined from a top-level function: only a part can
+  declare a name of the module.
+- In the nested function's own body when it stands in a head that declares a
+  name: a parameter list, a `catch` binding, a `for` that declares, the test of
+  a `case` in a `switch` that declares, the initializer of an enum member. Also
+  in a block of a file that has the word `eval`: beside a direct `eval` a block
+  keeps a function declaration as written, which is a `var` too where the code
+  is not strict, and a `let` in a `case` is not set when another `case` calls
+  the function. A direct `eval` marks its scope only when it is visited, so
+  the parser looks for the word in the text of the file: a comment that has it
+  counts, a longer name such as `evaluate` does not. Also when it is an async
+  or generator function declaration in a block, which a block always keeps. It
+  is then a new function at each call.
+  With no renamer, no local of the compiled function takes a name that a scope
+  around it declares.
+
+The parser lists the name in the scope of that declaration, and the renamer
+numbers it there. The `--aot` loader places these functions itself (`Places` in
+`src/aot/load/react_compiler/writer.rs`), and not always in the same place:
+what is outlined from the test of a `case` goes in front of the statements of
+that `case`. Upstream puts the outlined function after a nested function
+declaration. For a nested arrow function or function expression it puts it at
+the end of the program, where it throws if it reads a variable of an enclosing
+function (facebook/react#34901).
+
 ### The compiler as a linter
 
 `bun lint` has the validations as rules (`react/refs`, `react-hooks/refs`, ...). `lint.rs` is upstream's

src/react_compiler/program.rs

@@ -127,7 +127,7 @@ pub trait Host {
     /// resolved from the current scope the way the visit pass resolves it.
     fn jsx_classic_factory(&mut self, loc: Loc) -> Expr;
 
-    /// A new symbol for a module-level name, such as an outlined function.
+    /// A new symbol for a name that is not a local of the compiled function, such as an outlined function.
     fn new_generated(&mut self, name: &[u8]) -> Ref;
 
     /// A new symbol for a name that the compiled function (or one outlined from it) declares.