diff --git a/crates/rolldown/src/bundler/bundle/bundle.rs b/crates/rolldown/src/bundler/bundle/bundle.rs index 4dd0ee71250..662a8d581ce 100644 --- a/crates/rolldown/src/bundler/bundle/bundle.rs +++ b/crates/rolldown/src/bundler/bundle/bundle.rs @@ -3,7 +3,7 @@ use std::{borrow::Cow, hash::BuildHasherDefault}; use super::asset::Asset; use crate::bundler::{ chunk::{ - chunk::{Chunk, CrossChunkImportItem}, + chunk::{Chunk, ChunkSymbolExporter, CrossChunkImportItem}, ChunkId, ChunksVec, }, chunk_graph::ChunkGraph, @@ -53,6 +53,8 @@ impl<'a> Bundle<'a> { }); } + // TODO(hyf0): refactor this function + #[allow(clippy::too_many_lines)] fn compute_cross_chunk_links(&mut self, chunk_graph: &mut ChunkGraph) { // Determine which symbols belong to which chunk let mut chunk_meta_imports_vec = @@ -111,16 +113,36 @@ impl<'a> Bundle<'a> { let chunk_meta_imports = &chunk_meta_imports_vec[chunk_id]; for import_ref in chunk_meta_imports.iter().copied() { let import_symbol = self.graph.symbols.get(import_ref); - let importee_chunk_id = import_symbol.chunk_id.unwrap_or_else(|| { - panic!("symbol {import_ref:?} {import_symbol:?} is not assigned to any chunk") - }); - if chunk_id != importee_chunk_id { + // Find out the import_ref whether comes from the chunk or external module. + + if let Some(importee_chunk_id) = import_symbol.chunk_id { + if chunk_id != importee_chunk_id { + chunk + .imports_from_other_chunks + .entry(ChunkSymbolExporter::Chunk(importee_chunk_id)) + .or_default() + .push(CrossChunkImportItem { + import_ref, + export_alias: None, + export_alias_is_star: false, + }); + chunk_meta_exports_vec[importee_chunk_id].insert(import_ref); + } + } else { + // The symbol is from an external module. + let canonical_ref = self.graph.symbols.canonical_ref_for(import_ref); + let symbol = self.graph.symbols.get(canonical_ref); + // The module must be an external module. + let importee = self.graph.modules[canonical_ref.owner].expect_external(); chunk .imports_from_other_chunks - .entry(importee_chunk_id) + .entry(ChunkSymbolExporter::ExternalModule(importee.id)) .or_default() - .push(CrossChunkImportItem { import_ref, export_alias: None }); - chunk_meta_exports_vec[importee_chunk_id].insert(import_ref); + .push(CrossChunkImportItem { + import_ref, + export_alias: symbol.exported_as.clone(), + export_alias_is_star: symbol.exported_as_star, + }); } } @@ -150,9 +172,9 @@ impl<'a> Bundle<'a> { } } for chunk_id in chunk_graph.chunks.indices() { - for (importee_chunk_id, import_items) in - &chunk_graph.chunks[chunk_id].imports_from_other_chunks + for (symbol_exporter, import_items) in &chunk_graph.chunks[chunk_id].imports_from_other_chunks { + let ChunkSymbolExporter::Chunk(importee_chunk_id) = symbol_exporter else { return }; for item in import_items { if let Some(alias) = chunk_graph.chunks[*importee_chunk_id].exports_to_other_chunks.get(&item.import_ref) diff --git a/crates/rolldown/src/bundler/chunk/chunk.rs b/crates/rolldown/src/bundler/chunk/chunk.rs index c5e8959cece..0a878330f9a 100644 --- a/crates/rolldown/src/bundler/chunk/chunk.rs +++ b/crates/rolldown/src/bundler/chunk/chunk.rs @@ -18,9 +18,16 @@ use crate::{ use super::ChunkId; +#[derive(Debug, Hash, PartialEq, Eq, Clone, Copy)] +pub enum ChunkSymbolExporter { + Chunk(ChunkId), + ExternalModule(ModuleId), +} + #[derive(Debug)] pub struct CrossChunkImportItem { pub export_alias: Option, + pub export_alias_is_star: bool, pub import_ref: SymbolRef, } @@ -32,7 +39,7 @@ pub struct Chunk { pub file_name: Option, pub canonical_names: FxHashMap, pub bits: BitSet, - pub imports_from_other_chunks: FxHashMap>, + pub imports_from_other_chunks: FxHashMap>, // meaningless if the chunk is an entrypoint pub exports_to_other_chunks: FxHashMap, } diff --git a/crates/rolldown/src/bundler/chunk/render_chunk_imports.rs b/crates/rolldown/src/bundler/chunk/render_chunk_imports.rs index 25f52443d43..50f6dde3861 100644 --- a/crates/rolldown/src/bundler/chunk/render_chunk_imports.rs +++ b/crates/rolldown/src/bundler/chunk/render_chunk_imports.rs @@ -11,30 +11,59 @@ impl Chunk { chunk_graph: &ChunkGraph, ) -> MagicString<'static> { let mut s = MagicString::new(""); - self.imports_from_other_chunks.iter().for_each(|(chunk_id, items)| { - let chunk = &chunk_graph.chunks[*chunk_id]; - let mut import_items = items - .iter() - .map(|item| { - let imported = chunk - .canonical_names - .get(&graph.symbols.par_canonical_ref_for(item.import_ref)) - .cloned() - .unwrap(); - let alias = item.export_alias.as_ref().unwrap(); - if imported == alias { - format!("{imported}") - } else { - format!("{imported} as {alias}") - } - }) - .collect::>(); - import_items.sort(); - s.append(format!( - "import {{ {} }} from \"./{}\";\n", - import_items.join(", "), - chunk.file_name.as_ref().unwrap() - )); + self.imports_from_other_chunks.iter().for_each(|(exporter_id, items)| match exporter_id { + super::chunk::ChunkSymbolExporter::Chunk(exporter_id) => { + let importee_chunk = &chunk_graph.chunks[*exporter_id]; + let mut import_items = items + .iter() + .map(|item| { + let imported = importee_chunk + .canonical_names + .get(&graph.symbols.par_canonical_ref_for(item.import_ref)) + .cloned() + .unwrap(); + let alias = item.export_alias.as_ref().unwrap(); + if imported == alias { + format!("{imported}") + } else { + format!("{imported} as {alias}") + } + }) + .collect::>(); + import_items.sort(); + s.append(format!( + "import {{ {} }} from \"./{}\";\n", + import_items.join(", "), + importee_chunk.file_name.as_ref().unwrap() + )); + } + super::chunk::ChunkSymbolExporter::ExternalModule(exporter_id) => { + let module = graph.modules[*exporter_id].expect_external(); + let mut import_items = items + .iter() + .filter_map(|item| { + let alias = graph.symbols.get_original_name(item.import_ref); + if item.export_alias_is_star { + s.append(format!("import * as {alias} from \"{}\";\n", module.resource_id.as_ref())); + return None; + } + let imported = item.export_alias.as_ref().unwrap(); + Some(if imported == alias { + format!("{imported}") + } else { + format!("{imported} as {alias}") + }) + }) + .collect::>(); + import_items.sort(); + if !import_items.is_empty() { + s.append(format!( + "import {{ {} }} from \"{}\";\n", + import_items.join(", "), + module.resource_id.as_ref() + )); + } + } }); s } diff --git a/crates/rolldown/src/bundler/graph/linker.rs b/crates/rolldown/src/bundler/graph/linker.rs index 4981ed0b364..c6bc735e125 100644 --- a/crates/rolldown/src/bundler/graph/linker.rs +++ b/crates/rolldown/src/bundler/graph/linker.rs @@ -124,7 +124,6 @@ impl<'graph> Linker<'graph> { self.mark_module_wrapped(&mut symbols, &mut linking_infos); - // Mark namespace symbol for namespace referenced // Create symbols for external module self.mark_extra_symbols(&mut symbols, &mut linking_infos); diff --git a/crates/rolldown/src/bundler/graph/symbols.rs b/crates/rolldown/src/bundler/graph/symbols.rs index 1faed0393ac..da9b86d58f9 100644 --- a/crates/rolldown/src/bundler/graph/symbols.rs +++ b/crates/rolldown/src/bundler/graph/symbols.rs @@ -26,6 +26,10 @@ pub struct Symbol { pub link: Option, /// The chunk that this symbol is defined in. pub chunk_id: Option, + // FIXME: should not place those fields here + // Only for external modules + pub exported_as: Option, + pub exported_as_star: bool, } #[derive(Debug, Default)] @@ -68,7 +72,14 @@ impl Symbols { table .names .into_iter() - .map(|name| Symbol { name, link: None, chunk_id: None, namespace_alias: None }) + .map(|name| Symbol { + name, + link: None, + chunk_id: None, + namespace_alias: None, + exported_as: None, + exported_as_star: false, + }) .collect() }) .collect(); @@ -77,8 +88,14 @@ impl Symbols { } pub fn create_symbol(&mut self, owner: ModuleId, name: Atom) -> SymbolRef { - let symbol_id = - self.inner[owner].push(Symbol { name, link: None, chunk_id: None, namespace_alias: None }); + let symbol_id = self.inner[owner].push(Symbol { + name, + link: None, + chunk_id: None, + namespace_alias: None, + exported_as: None, + exported_as_star: false, + }); SymbolRef { owner, symbol: symbol_id } } diff --git a/crates/rolldown/src/bundler/module/external_module.rs b/crates/rolldown/src/bundler/module/external_module.rs index 4a6bd0e91a6..cf67fbb7056 100644 --- a/crates/rolldown/src/bundler/module/external_module.rs +++ b/crates/rolldown/src/bundler/module/external_module.rs @@ -11,34 +11,53 @@ pub struct ExternalModule { pub exec_order: u32, pub resource_id: ResourceId, pub import_records: IndexVec, - pub symbols_imported_by_others: FxHashMap, - pub namespace_name: Atom, + pub exported_name_to_binding_ref: FxHashMap, + // FIXME: make this non-optional + pub namespace_ref: Option, } impl ExternalModule { pub fn new(id: ModuleId, resource_id: ResourceId) -> Self { - let namespace_name = format!("{}_ns", resource_id.generate_unique_name()).into(); Self { id, exec_order: u32::MAX, resource_id, import_records: IndexVec::default(), - symbols_imported_by_others: FxHashMap::default(), - namespace_name, + exported_name_to_binding_ref: FxHashMap::default(), + namespace_ref: None, } } #[allow(clippy::needless_pass_by_value)] pub fn add_export_symbol(&mut self, symbols: &mut Symbols, exported: Atom, is_star: bool) { - let symbol = if is_star { &self.namespace_name } else { &exported }; - self - .symbols_imported_by_others - .entry(symbol.clone()) - .or_insert_with(|| symbols.create_symbol(self.id, symbol.clone())); + // Don't worry about the user happen to use the same name as the name we generated for `default export` or `namespace export`. + // Since they have different `SymbolId`, so in de-conflict phase, they will be renamed to different names. + let symbol_ref = if is_star && self.namespace_ref.is_none() { + self.namespace_ref = Some(symbols.create_symbol( + self.id, + Atom::from(format!("{}_ns", self.resource_id.generate_unique_name())), + )); + self.namespace_ref.unwrap() + } else { + *self.exported_name_to_binding_ref.entry(exported.clone()).or_insert_with_key(|exported| { + let declared_name = if exported.as_ref() == "default" { + Atom::from(format!("{}_default", self.resource_id.generate_unique_name())) + } else { + exported.clone() + }; + symbols.create_symbol(self.id, declared_name) + }) + }; + let symbol = symbols.get_mut(symbol_ref); + symbol.exported_as = Some(exported); + symbol.exported_as_star = is_star; } pub fn resolve_export(&self, exported: &Atom, is_star: bool) -> SymbolRef { - let symbol = if is_star { &self.namespace_name } else { exported }; - *self.symbols_imported_by_others.get(symbol).expect("should have export symbol") + if is_star { + self.namespace_ref.expect("should have namespace ref") + } else { + *self.exported_name_to_binding_ref.get(exported).expect("should have export symbol") + } } } diff --git a/crates/rolldown/src/bundler/module/mod.rs b/crates/rolldown/src/bundler/module/mod.rs index 2855ed83852..e81bd27e4de 100644 --- a/crates/rolldown/src/bundler/module/mod.rs +++ b/crates/rolldown/src/bundler/module/mod.rs @@ -56,6 +56,13 @@ impl Module { } } + pub fn expect_external(&self) -> &ExternalModule { + match self { + Self::Normal(_) => unreachable!(), + Self::External(m) => m, + } + } + pub fn import_records(&self) -> &IndexVec { match self { Self::Normal(m) => &m.import_records, diff --git a/crates/rolldown/src/bundler/module_loader/module_loader.rs b/crates/rolldown/src/bundler/module_loader/module_loader.rs index 3a45e597aa0..86b8477c0fa 100644 --- a/crates/rolldown/src/bundler/module_loader/module_loader.rs +++ b/crates/rolldown/src/bundler/module_loader/module_loader.rs @@ -166,13 +166,12 @@ impl<'a> ModuleLoader<'a> { std::collections::hash_map::Entry::Occupied(visited) => *visited.get(), std::collections::hash_map::Entry::Vacant(not_visited) => { let id = intermediate_modules.push(None); + not_visited.insert(id); if info.is_external { let ext = ExternalModule::new(id, ResourceId::new(info.path.clone(), &self.input_options.cwd)); intermediate_modules[id] = Some(Module::External(ext)); } else { - not_visited.insert(id); - self.remaining += 1; let module_path = ResourceId::new(info.path.clone(), &self.input_options.cwd); diff --git a/crates/rolldown/tests/fixtures/external/implicit_import_external/artifacts.snap b/crates/rolldown/tests/fixtures/external/implicit_import_external/artifacts.snap new file mode 100644 index 00000000000..55331986199 --- /dev/null +++ b/crates/rolldown/tests/fixtures/external/implicit_import_external/artifacts.snap @@ -0,0 +1,21 @@ +--- +source: crates/rolldown/tests/common/case.rs +expression: content +input_file: crates/rolldown/tests/fixtures/external/implicit_import_external +--- +# Assets + +## main.mjs + +```js +import * as node_assert_ns from "node:assert"; +import { default as node_assert_default, equal } from "node:assert"; + +// main.js + + + +node_assert_default.equal(1, 1) +node_assert_ns.equal(1, 1) +equal(1, 1) +``` diff --git a/crates/rolldown/tests/fixtures/external/implicit_import_external/main.js b/crates/rolldown/tests/fixtures/external/implicit_import_external/main.js new file mode 100644 index 00000000000..f064495f787 --- /dev/null +++ b/crates/rolldown/tests/fixtures/external/implicit_import_external/main.js @@ -0,0 +1,6 @@ +import assert from 'node:assert' +import { equal } from 'node:assert' +import * as assert_star from 'node:assert' +assert.equal(1, 1) +assert_star.equal(1, 1) +equal(1, 1) diff --git a/crates/rolldown/tests/fixtures/external/implicit_import_external/test.config.json b/crates/rolldown/tests/fixtures/external/implicit_import_external/test.config.json new file mode 100644 index 00000000000..9e26dfeeb6e --- /dev/null +++ b/crates/rolldown/tests/fixtures/external/implicit_import_external/test.config.json @@ -0,0 +1 @@ +{} \ No newline at end of file