bundler: do not fold an unwrapped require() declaration when its variable is rebound

src/ast/e.rs

@@ -2305,26 +2305,9 @@ pub struct If {
     pub no: ExprNodeIndex,
 }
 
-pub enum UnwrappedRequireMarker {}
-/// Index into the parser's `imports_to_convert_from_require`.
-pub type UnwrappedRequireIndex = bun_core::GenericIndex<u32, UnwrappedRequireMarker>;
-pub type UnwrappedRequireIndexOptional =
-    bun_core::GenericIndexOptional<u32, UnwrappedRequireMarker>;
-
-#[derive(Clone, Copy)]
+#[derive(Clone, Copy, Default)]
 pub struct RequireString {
     pub import_record_index: u32,
-
-    /// Set when `unwrap_commonjs_to_esm` turned this `require()` into an import.
-    pub unwrapped_id: UnwrappedRequireIndexOptional,
-}
-impl Default for RequireString {
-    fn default() -> Self {
-        Self {
-            import_record_index: 0,
-            unwrapped_id: UnwrappedRequireIndexOptional::NONE,
-        }
-    }
 }
 
 #[derive(Clone, Copy)]

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 parser linked another declaration of this variable to this symbol.
+        const HAS_REDECLARATION = 1 << 8;
     }
 }
 
@@ -159,10 +162,16 @@ 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;
+    has_redeclaration, set_has_redeclaration => HAS_REDECLARATION;
 }
 
 const _: () = assert!(core::mem::size_of::<Option<bun_alloc::AstBox<G::NamespaceAlias>>>() == 8);
 const _: () = assert!(core::mem::size_of::<Symbol>() <= 48);
+// Two flags on one bit alias with no other diagnostic.
+const _: () = assert!(
+    SymbolFlags::all().bits().count_ones() as usize
+        == <SymbolFlags as bitflags::Flags>::FLAGS.len()
+);
 
 const INVALID_CHUNK_INDEX: u32 = u32::MAX;
 pub const INVALID_NESTED_SCOPE_SLOT: u32 = u32::MAX;
@@ -240,6 +249,12 @@ impl Symbol {
         self.link.get().is_valid()
     }
 
+    /// In the parser: another symbol also declares this variable, with uses and a declaration of its own.
+    #[inline]
+    pub fn shares_variable(&self) -> bool {
+        self.has_link() || self.has_redeclaration()
+    }
+
     /// An import item the linker merged into the export it names. A local
     /// that a hoisting merge links has no import item status.
     #[inline]

src/bun_core/util.rs

@@ -3307,13 +3307,6 @@ impl<I: GenericIndexInt, M> GenericIndex<I, M> {
         GenericIndexOptional(self.0, core::marker::PhantomData)
     }
 }
-impl<I: GenericIndexInt, M> GenericIndexOptional<I, M> {
-    #[inline]
-    pub fn is_some(self) -> bool {
-        !self.is_none()
-    }
-}
-
 /// `GenericIndex::Optional` — `MAX` is `none`.
 #[repr(transparent)]
 pub struct GenericIndexOptional<I, M = ()>(I, core::marker::PhantomData<M>);
@@ -3337,18 +3330,7 @@ impl<I: core::fmt::Debug, M> core::fmt::Debug for GenericIndexOptional<I, M> {
     }
 }
 impl<I: GenericIndexInt, M> GenericIndexOptional<I, M> {
-    #[inline]
-    pub fn is_none(self) -> bool {
-        self.0 == I::NULL_VALUE
-    }
-
     pub const NONE: Self = Self(I::NULL_VALUE, core::marker::PhantomData);
-    /// Alias for `unwrap()` matching the local-newtype API that pre-existed in
-    /// `bun_bundler::output_file::IndexOptional`.
-    #[inline]
-    pub fn get(self) -> Option<GenericIndex<I, M>> {
-        self.unwrap()
-    }
     #[inline]
     pub fn unwrap(self) -> Option<GenericIndex<I, M>> {
         if self.0 == I::NULL_VALUE {

src/bundler/LinkerContext.rs

@@ -2269,7 +2269,6 @@ impl<'a> LinkerContext<'a> {
                             value: Some(Expr::init(
                                 E::RequireString {
                                     import_record_index,
-                                    ..Default::default()
                                 },
                                 loc,
                             )),
@@ -2314,7 +2313,6 @@ impl<'a> LinkerContext<'a> {
                                 value: Some(Expr::init(
                                     E::RequireString {
                                         import_record_index,
-                                        ..Default::default()
                                     },
                                     loc,
                                 )),
@@ -2540,26 +2538,17 @@ impl<'a> LinkerContext<'a> {
     pub(crate) fn require_or_import_meta_for_source(
         &mut self,
         source_index: crate::IndexInt,
-        was_unwrapped_require: bool,
     ) -> js_printer::RequireOrImportMeta {
         let flags = self.graph.meta.items_flags()[source_index as usize];
         let wrapper_ref = self.graph.ast.items_wrapper_ref()[source_index as usize];
         js_printer::RequireOrImportMeta {
-            exports_ref: if flags.wrap == WrapKind::Esm
-                || (was_unwrapped_require
-                    && self.graph.ast.items_flags()[source_index as usize]
-                        .contains(AstFlags::FORCE_CJS_TO_ESM))
-            {
+            exports_ref: if flags.wrap == WrapKind::Esm {
                 self.graph.ast.items_exports_ref()[source_index as usize]
             } else {
                 Ref::NONE
             },
             is_wrapper_async: flags.is_async_or_has_async_dependency && wrapper_ref.is_valid(),
             wrapper_ref,
-
-            was_unwrapped_require: was_unwrapped_require
-                && self.graph.ast.items_flags()[source_index as usize]
-                    .contains(AstFlags::FORCE_CJS_TO_ESM),
         }
     }
 
@@ -2830,12 +2819,8 @@ impl<'a> LinkerContext<'a> {
 /// can call back into `LinkerContext::require_or_import_meta_for_source`.
 impl<'a> js_printer::RequireOrImportMetaSource for LinkerContext<'a> {
     #[inline]
-    fn require_or_import_meta_for_source(
-        &mut self,
-        id: u32,
-        was_unwrapped_require: bool,
-    ) -> js_printer::RequireOrImportMeta {
-        LinkerContext::require_or_import_meta_for_source(self, id, was_unwrapped_require)
+    fn require_or_import_meta_for_source(&mut self, id: u32) -> js_printer::RequireOrImportMeta {
+        LinkerContext::require_or_import_meta_for_source(self, id)
     }
 }
 
@@ -4477,9 +4462,6 @@ impl<'a> LinkerContext<'a> {
             // A named import that holds a namespace (`export * as`) prints as
             // an import identifier.
             bun_ast::ExprData::EImportIdentifier(identifier) => identifier.ref_,
-            bun_ast::ExprData::ERequireString(require) => {
-                return require.unwrapped_id.get().is_some();
-            }
             _ => return false,
         };
         // A require() lifted into an import binds an ordinary local, so user

src/bundler/linker_context/convertStmtsForChunk.rs

@@ -313,7 +313,6 @@ pub(crate) fn convert_stmts_for_chunk(
                                 Expr::init(
                                     E::RequireString {
                                         import_record_index: s.import_record_index,
-                                        ..Default::default()
                                     },
                                     stmt.loc,
                                 )
@@ -343,7 +342,6 @@ pub(crate) fn convert_stmts_for_chunk(
                                 break 'brk Expr::init(
                                     E::RequireString {
                                         import_record_index: s.import_record_index,
-                                        ..Default::default()
                                     },
                                     stmt.loc,
                                 );

src/js_parser/fold.rs

@@ -208,6 +208,13 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
                     // module linking just to rewrite these EDot expressions.
                     if p.options.bundle {
                         if p.import_items_for_namespace.contains_key(&id.ref_) {
+                            let waiting_alias = p.require_alias_of(id.ref_);
+                            // Something rebinds the local already: this is a property of the variable.
+                            if let Some(alias) = &waiting_alias
+                                && !p.require_alias_holds(alias)
+                            {
+                                break 'sw;
+                            }
                             // Note: split into lookup → (maybe new_symbol) → re-borrow for borrowck.
                             let existing = p
                                 .import_items_for_namespace
@@ -272,6 +279,9 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
                             // under it.
                             if dynamic_record.is_some() {
                                 p.note_tracked_namespace_use(id.ref_);
+                            } else if let Some(alias) = waiting_alias {
+                                // The local stays counted: `settle_require_aliases` takes back one use for each of these.
+                                p.record_usage_impl(alias.namespace, false);
                             } else {
                                 p.ignore_usage(id.ref_);
                             }