refactor(transformer): add more specific methods to ModuleImportsStore (#6560)

Make `ImportKind` private implementation detail of `ModuleImports` common transform. Provide `add_default_import` and `add_named_import` methods instead.

crates/oxc_transformer/src/common/helper_loader.rs

@@ -73,7 +73,6 @@ use oxc_traverse::{BoundIdentifier, Traverse, TraverseCtx};
 use rustc_hash::FxHashMap;
 use serde::Deserialize;
 
-use super::module_imports::ImportKind;
 use crate::TransformCtx;
 
 /// Defines the mode for loading helper functions.
@@ -150,7 +149,7 @@ impl<'a, 'ctx> HelperLoader<'a, 'ctx> {
     fn add_imports(&self) {
         self.ctx.helper_loader.loaded_helpers.borrow_mut().drain().for_each(
             |(_, (source, import))| {
-                self.ctx.module_imports.add_import(source, ImportKind::new_default(import), false);
+                self.ctx.module_imports.add_default_import(source, import, false);
             },
         );
     }

crates/oxc_transformer/src/common/module_imports.rs

@@ -62,27 +62,16 @@ impl<'a, 'ctx> Traverse<'a> for ModuleImports<'a, 'ctx> {
     }
 }
 
-#[derive(Clone)]
-pub struct NamedImport<'a> {
+struct NamedImport<'a> {
     imported: Atom<'a>,
     local: BoundIdentifier<'a>,
 }
 
-pub enum ImportKind<'a> {
+enum ImportKind<'a> {
     Named(NamedImport<'a>),
     Default(BoundIdentifier<'a>),
 }
 
-impl<'a> ImportKind<'a> {
-    pub fn new_named(imported: Atom<'a>, local: BoundIdentifier<'a>) -> Self {
-        Self::Named(NamedImport { imported, local })
-    }
-
-    pub fn new_default(local: BoundIdentifier<'a>) -> Self {
-        Self::Default(local)
-    }
-}
-
 /// Store for `import` / `require` statements to be added at top of program.
 ///
 /// TODO(improve-on-babel): Insertion order does not matter. We only have to use `IndexMap`
@@ -99,6 +88,43 @@ impl<'a> ModuleImportsStore<'a> {
         Self { imports: RefCell::new(IndexMap::default()) }
     }
 
+    /// Add default `import` or `require` to top of program.
+    ///
+    /// Which it will be depends on the source type.
+    ///
+    /// * `import named_import from 'source';` or
+    /// * `var named_import = require('source');`
+    ///
+    /// If `front` is `true`, `import`/`require` is added to front of the `import`s/`require`s.
+    pub fn add_default_import(&self, source: Atom<'a>, local: BoundIdentifier<'a>, front: bool) {
+        self.add_import(source, ImportKind::Default(local), front);
+    }
+
+    /// Add named `import` to top of program.
+    ///
+    /// `import { named_import } from 'source';`
+    ///
+    /// If `front` is `true`, `import` is added to front of the `import`s.
+    ///
+    /// Adding named `require`s is not supported, and will cause a panic later on.
+    pub fn add_named_import(
+        &self,
+        source: Atom<'a>,
+        imported: Atom<'a>,
+        local: BoundIdentifier<'a>,
+        front: bool,
+    ) {
+        self.add_import(source, ImportKind::Named(NamedImport { imported, local }), front);
+    }
+
+    /// Returns `true` if no imports have been scheduled for insertion.
+    pub fn is_empty(&self) -> bool {
+        self.imports.borrow().is_empty()
+    }
+}
+
+// Internal methods
+impl<'a> ModuleImportsStore<'a> {
     /// Add `import` or `require` to top of program.
     ///
     /// Which it will be depends on the source type.
@@ -106,10 +132,12 @@ impl<'a> ModuleImportsStore<'a> {
     /// * `import { named_import } from 'source';` or
     /// * `var named_import = require('source');`
     ///
+    /// Adding a named `require` is not supported, and will cause a panic later on.
+    ///
     /// If `front` is `true`, `import`/`require` is added to front of the `import`s/`require`s.
     /// TODO(improve-on-babel): `front` option is only required to pass one of Babel's tests. Output
     /// without it is still valid. Remove this once our output doesn't need to match Babel exactly.
-    pub fn add_import(&self, source: Atom<'a>, import: ImportKind<'a>, front: bool) {
+    fn add_import(&self, source: Atom<'a>, import: ImportKind<'a>, front: bool) {
         match self.imports.borrow_mut().entry(source) {
             IndexMapEntry::Occupied(mut entry) => {
                 entry.get_mut().push(import);
@@ -128,14 +156,6 @@ impl<'a> ModuleImportsStore<'a> {
         }
     }
 
-    /// Returns `true` if no imports have been scheduled for insertion.
-    pub fn is_empty(&self) -> bool {
-        self.imports.borrow().is_empty()
-    }
-}
-
-// Internal methods
-impl<'a> ModuleImportsStore<'a> {
     /// Insert `import` / `require` statements at top of program.
     fn insert_into_program(&self, transform_ctx: &TransformCtx<'a>, ctx: &mut TraverseCtx<'a>) {
         if transform_ctx.source_type.is_script() {

crates/oxc_transformer/src/react/jsx.rs

@@ -100,7 +100,7 @@ use oxc_syntax::{
 };
 use oxc_traverse::{BoundIdentifier, Traverse, TraverseCtx};
 
-use crate::{common::module_imports::ImportKind, TransformCtx};
+use crate::TransformCtx;
 
 use super::diagnostics;
 pub use super::{
@@ -198,8 +198,7 @@ impl<'a, 'ctx> AutomaticScriptBindings<'a, 'ctx> {
     ) -> BoundIdentifier<'a> {
         let binding =
             ctx.generate_uid_in_root_scope(variable_name, SymbolFlags::FunctionScopedVariable);
-        let import = ImportKind::new_default(binding.clone());
-        self.ctx.module_imports.add_import(source, import, front);
+        self.ctx.module_imports.add_default_import(source, binding.clone(), front);
         binding
     }
 }
@@ -297,8 +296,7 @@ impl<'a, 'ctx> AutomaticModuleBindings<'a, 'ctx> {
         ctx: &mut TraverseCtx<'a>,
     ) -> BoundIdentifier<'a> {
         let binding = ctx.generate_uid_in_root_scope(name, SymbolFlags::Import);
-        let import = ImportKind::new_named(Atom::from(name), binding.clone());
-        self.ctx.module_imports.add_import(source, import, false);
+        self.ctx.module_imports.add_named_import(source, Atom::from(name), binding.clone(), false);
         binding
     }
 }