js_parser: relocate module-level vars from brace-less bodies for the bundler and the REPL

src/ast/symbol.rs

@@ -108,7 +108,7 @@ pub struct Symbol {
 
 bitflags::bitflags! {
     #[derive(Copy, Clone, Eq, PartialEq, Default, Debug)]
-    pub struct SymbolFlags: u8 {
+    pub struct SymbolFlags: u16 {
         const MUST_START_WITH_CAPITAL_LETTER_FOR_JSX = 1 << 1;
 
         /// Certain symbols must not be renamed or minified. For example, the
@@ -131,6 +131,9 @@ bitflags::bitflags! {
 
         /// A call of this function declaration or lifted export ignores `this`.
         const CALL_IGNORES_THIS = 1 << 7;
+
+        /// The `__esm` wrapper stores into this `var`, so no store of it is lifted above the wrapper. Set on the root of the link chain.
+        const VAR_STORES_STAY_IN_WRAPPER = 1 << 8;
     }
 }
 
@@ -159,6 +162,7 @@ symbol_flag_accessors! {
     has_been_assigned_to, set_has_been_assigned_to => HAS_BEEN_ASSIGNED_TO;
     called_as_method, set_called_as_method => CALLED_AS_METHOD;
     call_ignores_this, set_call_ignores_this => CALL_IGNORES_THIS;
+    var_stores_stay_in_wrapper, set_var_stores_stay_in_wrapper => VAR_STORES_STAY_IN_WRAPPER;
 }
 
 const _: () = assert!(core::mem::size_of::<Option<bun_alloc::AstBox<G::NamespaceAlias>>>() == 8);

src/bundler/linker_context/generateCodeForFileInChunkJS.rs

@@ -677,6 +677,13 @@ pub fn generate_code_for_file_in_chunk_js<'r, 'src>(
                         Expr::init_identifier(ref_, loc)
                     }
 
+                    fn var_stores_stay_in_wrapper(&self, ref_: Ref) -> bool {
+                        let symbols = self.symbols.get();
+                        symbols
+                            .get_const(symbols.follow(ref_))
+                            .is_some_and(|symbol| symbol.var_stores_stay_in_wrapper())
+                    }
+
                     /// The pattern without the locals the linker bound to exports
                     /// (`const { a } = await import(…)`): assigning one would
                     /// overwrite the export.
@@ -762,7 +769,8 @@ pub fn generate_code_for_file_in_chunk_js<'r, 'src>(
                                     if let Some(initializer) = decl.value {
                                         // Keep in sync with `needs_wrapper_ref` in the parser.
                                         if initializer.can_be_moved()
-                                            && matches!(decl.binding.data, B::B::BIdentifier(_))
+                                            && let B::B::BIdentifier(id) = decl.binding.data
+                                            && !hoist.var_stores_stay_in_wrapper(id.get().r#ref)
                                         {
                                             // ie `const main = class { static main() {} }` => `var main = class { static main() {} }`
                                             hoist.decls.push(G::Decl {

src/js_parser/fold.rs

@@ -56,7 +56,8 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
     ) -> RelocateVars {
         let p = self;
         // Only do this when the scope is not already top-level and when we're not inside a function.
-        if p.current_scope == p.module_scope {
+        // A brace-less body opens no scope, so there the scope does not say "top-level".
+        if p.current_scope == p.module_scope && !p.relocates_single_stmt_body_decls() {
             return RelocateVars {
                 ok: false,
                 ..Default::default()
@@ -79,6 +80,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
             };
         }
 
+        let first_relocated = p.relocated_top_level_vars.len();
         let mut value: Expr = Expr::EMPTY;
         for decl in decls {
             // Derive `*mut P` from the live `&mut Self` so the trampoline's
@@ -92,6 +94,9 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
                 value = Expr::join_with_comma(value, binding);
             }
         }
+        if p.current_scope == p.module_scope {
+            p.keep_var_stores_in_wrapper(first_relocated);
+        }
 
         if matches!(value.data, js_ast::ExprData::EMissing(_)) {
             return RelocateVars {

src/js_parser/p.rs

@@ -645,6 +645,7 @@ pub struct P<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> {
     // The old "var" statements are turned into regular assignments instead. This
     // makes it easier to quickly scan the top-level statements for "var" locals
     // with the guarantee that all will be found.
+    // Not all yet: see `relocates_single_stmt_body_decls`, and a lowered top-level `using` keeps them in its `try` (#38284).
     pub(crate) relocated_top_level_vars: List<'a, js_ast::LocRef>,
 
     // ArrowFunction is a special case in the grammar. Although it appears to be
@@ -6890,7 +6891,13 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
                 value: None,
             }]);
 
-            if self.current_scope == self.module_scope {
+            if self.current_scope == self.module_scope && self.relocates_single_stmt_body_decls() {
+                // `if (c) enum E {}`: `append_part` declares the "var" at the top level.
+                self.relocated_top_level_vars.push(LocRef {
+                    loc: name_loc,
+                    ref_: name_ref,
+                });
+            } else if self.current_scope == self.module_scope {
                 // Top-level namespace: "var"
                 stmts.push(self.s(
                     S::Local {
@@ -8025,6 +8032,40 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
         Expr::init_identifier(r#ref, loc)
     }
 
+    /// The linker's `__esm` wrapper and the REPL transform move top-level declarations out of their closure, and only those.
+    #[inline]
+    pub(crate) fn lifts_top_level_decls(&self) -> bool {
+        self.options.repl_mode
+            || (self.options.bundle && !self.options.features.hot_module_reloading)
+    }
+
+    /// A brace-less `if`/`else`/`while`/`do` body opens no scope, so `lifts_top_level_decls` needs its declarations relocated too.
+    #[inline]
+    pub(crate) fn relocates_single_stmt_body_decls(&self) -> bool {
+        self.is_inside_single_stmt_body
+            && self.lifts_top_level_decls()
+            // A bundle is module code, which rejects a top-level "var f" beside "function f" (#44001).
+            && !(self.options.bundle && self.has_top_level_function_merged_with_var)
+    }
+
+    /// The wrapper now reads and stores `relocated_top_level_vars[first..]`, so a store that the linker lifts above it would run first.
+    pub(crate) fn keep_var_stores_in_wrapper(&mut self, first: usize) {
+        for i in first..self.relocated_top_level_vars.len() {
+            let root = self.follow_symbol_links(self.relocated_top_level_vars[i].ref_);
+            self.symbols[root.inner_index() as usize].set_var_stores_stay_in_wrapper(true);
+        }
+    }
+
+    fn follow_symbol_links(&self, mut r#ref: Ref) -> Ref {
+        loop {
+            let symbol = &self.symbols[r#ref.inner_index() as usize];
+            if !symbol.has_link() {
+                return r#ref;
+            }
+            r#ref = symbol.link.get();
+        }
+    }
+
     // One statement could potentially expand to several statements
     pub(crate) fn stmts_to_single_stmt(
         &mut self,
@@ -9513,10 +9554,14 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
                                     // The linker keeps a pattern inside the wrapper.
                                     if !matches!(value.data, js_ast::ExprData::EMissing(_))
                                         && (!value.can_be_moved()
-                                            || !matches!(
-                                                decl.binding.data,
-                                                js_ast::b::B::BIdentifier(_)
-                                            ))
+                                            || match decl.binding.data {
+                                                js_ast::b::B::BIdentifier(id) => self.symbols[self
+                                                    .follow_symbol_links(id.r#ref)
+                                                    .inner_index()
+                                                    as usize]
+                                                    .var_stores_stay_in_wrapper(),
+                                                _ => true,
+                                            })
                                     {
                                         return true;
                                     }

src/js_parser/visit/visit_stmt.rs

@@ -1206,6 +1206,9 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
                 if let Some(new_stmt) = relocated.stmt {
                     stmts.push(new_stmt);
                 }
+                if p.options.features.react_fast_refresh && p.current_scope == p.module_scope {
+                    Self::register_react_refresh_decls(p, stmts, data.decls.slice())?;
+                }
 
                 return Ok(());
             }
@@ -1216,39 +1219,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
         stmts.push(*stmt);
 
         if p.options.features.react_fast_refresh && p.current_scope == p.module_scope {
-            for decl in data.decls.slice() {
-                'try_register: {
-                    let Some(val) = decl.value else {
-                        break 'try_register;
-                    };
-                    match val.data {
-                        // Assigning a component to a local.
-                        js_ast::ExprData::EArrow(_) | js_ast::ExprData::EFunction(_) => {}
-
-                        // A wrapped component.
-                        js_ast::ExprData::ECall(call) => match call.target.data {
-                            js_ast::ExprData::EIdentifier(id) => {
-                                if id.ref_ != p.react_refresh.latest_signature_ref {
-                                    break 'try_register;
-                                }
-                            }
-                            _ => break 'try_register,
-                        },
-                        _ => break 'try_register,
-                    }
-                    let id = match decl.binding.data {
-                        js_ast::binding::Data::BIdentifier(b) => b.r#ref,
-                        _ => break 'try_register,
-                    };
-                    let original_name = p.symbols[id.inner_index() as usize].original_name.slice();
-                    p.handle_react_refresh_register(
-                        stmts,
-                        original_name,
-                        id,
-                        ReactRefreshExportKind::Named,
-                    )?;
-                }
-            }
+            Self::register_react_refresh_decls(p, stmts, data.decls.slice())?;
         }
 
         if data.is_export && p.options.features.server_components.wraps_exports() {
@@ -1271,6 +1242,47 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
         Ok(())
     }
 
+    fn register_react_refresh_decls(
+        p: &mut Self,
+        stmts: &mut StmtList<'a>,
+        decls: &[G::Decl],
+    ) -> Result<(), Error> {
+        for decl in decls {
+            'try_register: {
+                let Some(val) = decl.value else {
+                    break 'try_register;
+                };
+                match val.data {
+                    // Assigning a component to a local.
+                    js_ast::ExprData::EArrow(_) | js_ast::ExprData::EFunction(_) => {}
+
+                    // A wrapped component.
+                    js_ast::ExprData::ECall(call) => match call.target.data {
+                        js_ast::ExprData::EIdentifier(id) => {
+                            if id.ref_ != p.react_refresh.latest_signature_ref {
+                                break 'try_register;
+                            }
+                        }
+                        _ => break 'try_register,
+                    },
+                    _ => break 'try_register,
+                }
+                let id = match decl.binding.data {
+                    js_ast::binding::Data::BIdentifier(b) => b.r#ref,
+                    _ => break 'try_register,
+                };
+                let original_name = p.symbols[id.inner_index() as usize].original_name.slice();
+                p.handle_react_refresh_register(
+                    stmts,
+                    original_name,
+                    id,
+                    ReactRefreshExportKind::Named,
+                )?;
+            }
+        }
+        Ok(())
+    }
+
     // ─── control-flow / scope visitors ──────────────────────────────────────
 
     fn s_break(
@@ -1903,12 +1915,17 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
                 // Potentially relocate "var" declarations to the top level. Note that this
                 // must be done inside the scope of the for loop or they won't be relocated.
                 if local.kind == S::Kind::KVar {
+                    let first_relocated = p.relocated_top_level_vars.len();
                     let relocate = p.maybe_relocate_vars_to_top_level(
                         local.decls.slice(),
                         RelocateVarsMode::Normal,
                     );
                     if let Some(relocated) = relocate.stmt {
                         data.init = Some(relocated);
+                    } else if relocate.ok && p.lifts_top_level_decls() {
+                        // `for (var i; ...)`: a "var" left here declares a second `i` inside the wrapper.
+                        data.init = None;
+                        p.keep_var_stores_in_wrapper(first_relocated);
                     }
                 }
             }