feat: make sure chunks are evaluated for their side effects

crates/rolldown/src/bundler/chunk/chunk.rs

@@ -147,8 +147,4 @@ impl Chunk {
     let (content, map) = concat_sourcemaps(&content_and_sourcemaps)?;
     Ok(((content, Some(map)), rendered_modules))
   }
-
-  pub fn is_entry_point(&self) -> bool {
-    matches!(self.kind, ChunkKind::EntryPoint { .. })
-  }
 }

crates/rolldown/src/bundler/chunk/render_chunk_imports.rs

@@ -76,12 +76,16 @@ impl Chunk {
           }
         })
         .collect::<Vec<_>>();
-      import_items.sort();
-      s.append(format!(
-        "import {{ {} }} from \"./{}\";\n",
-        import_items.join(", "),
-        importee_chunk.file_name.as_ref().unwrap()
-      ));
+      let file_name = importee_chunk
+        .file_name
+        .as_ref()
+        .expect("At this point, file name should already be generated");
+      if import_items.is_empty() {
+        s.append(format!("import \"./{file_name}\";\n"));
+      } else {
+        import_items.sort();
+        s.append(format!("import {{ {} }} from \"./{file_name}\";\n", import_items.join(", ")));
+      }
     });
     s
   }

crates/rolldown/src/bundler/stages/bundle_stage/code_splitting.rs

@@ -152,6 +152,8 @@ impl<'a> BundleStage<'a> {
       chunk.modules.sort_by_key(|module_id| self.link_output.modules[*module_id].exec_order());
     });
 
+    tracing::trace!("Generated chunks: {:#?}", chunks);
+
     ChunkGraph { chunks, module_to_chunk }
   }
 }

crates/rolldown/src/bundler/stages/bundle_stage/compute_cross_chunk_links.rs

@@ -2,42 +2,41 @@ use std::{borrow::Cow, ptr::addr_of};
 
 use crate::{
   bundler::{
-    chunk::chunk::{ChunkKind, CrossChunkImportItem},
+    chunk::{
+      chunk::{ChunkKind, CrossChunkImportItem},
+      ChunkId,
+    },
     chunk_graph::ChunkGraph,
     module::Module,
   },
   OutputFormat,
 };
 
 use super::BundleStage;
-use index_vec::index_vec;
+use index_vec::{index_vec, IndexVec};
 use rayon::iter::{ParallelBridge, ParallelIterator};
 use rolldown_common::{ExportsKind, ImportKind, ModuleId, NamedImport, SymbolRef};
 use rustc_hash::{FxHashMap, FxHashSet};
 
-impl<'a> BundleStage<'a> {
-  // TODO(hyf0): refactor this function
-  #[allow(clippy::too_many_lines, clippy::cognitive_complexity)]
-  pub fn compute_cross_chunk_links(&mut self, chunk_graph: &mut ChunkGraph) {
-    let mut chunk_meta_imports_vec =
-      index_vec![FxHashSet::<SymbolRef>::default(); chunk_graph.chunks.len()];
-    let mut chunk_meta_exports_vec =
-      index_vec![FxHashSet::<SymbolRef>::default(); chunk_graph.chunks.len()];
-    let mut chunk_meta_imports_from_external_modules_vec =
-      index_vec![FxHashMap::<ModuleId, Vec<NamedImport>>::default(); chunk_graph.chunks.len()];
+type ChunkMetaImports = IndexVec<ChunkId, FxHashSet<SymbolRef>>;
+type ChunkMetaImportsForExternalModules = IndexVec<ChunkId, FxHashMap<ModuleId, Vec<NamedImport>>>;
+type ChunkMetaExports = IndexVec<ChunkId, FxHashSet<SymbolRef>>;
 
+impl<'a> BundleStage<'a> {
+  /// - Assign each symbol to the chunk it belongs to
+  /// - Collect all referenced symbols and consider them potential imports
+  fn collect_potential_chunk_imports(
+    &mut self,
+    chunk_graph: &mut ChunkGraph,
+    chunk_meta_imports: &mut ChunkMetaImports,
+    chunk_meta_imports_from_external_modules: &mut ChunkMetaImportsForExternalModules,
+  ) {
     let symbols = &self.link_output.symbols;
 
-    // - Assign each symbol to the chunk it belongs to
-    // - Collect all referenced symbols and consider them potential imports
     chunk_graph
       .chunks
       .iter_enumerated()
-      .zip(
-        chunk_meta_imports_vec
-          .iter_mut()
-          .zip(chunk_meta_imports_from_external_modules_vec.iter_mut()),
-      )
+      .zip(chunk_meta_imports.iter_mut().zip(chunk_meta_imports_from_external_modules.iter_mut()))
       .par_bridge()
       .for_each(|((chunk_id, chunk), (chunk_meta_imports, imports_from_external_modules))| {
         chunk.modules.iter().copied().for_each(|module_id| {
@@ -52,6 +51,13 @@ impl<'a> BundleStage<'a> {
               imports_from_external_modules.entry(importee.id).or_default();
             });
 
+          module.named_imports.iter().for_each(|(_, import)| {
+            let rec = &module.import_records[import.record_id];
+            if let Module::External(importee) = &self.link_output.modules[rec.resolved_module] {
+              imports_from_external_modules.entry(importee.id).or_default().push(import.clone());
+            }
+          });
+
           module.stmt_infos.iter().for_each(|stmt_info| {
             if !stmt_info.is_included {
               return;
@@ -106,81 +112,24 @@ impl<'a> BundleStage<'a> {
           }
         }
       });
+  }
 
-    for (chunk_id, chunk) in chunk_graph.chunks.iter_enumerated() {
-      let chunk_meta_imports = &mut chunk_meta_imports_vec[chunk_id];
-      let imports_from_external_modules =
-        &mut chunk_meta_imports_from_external_modules_vec[chunk_id];
-
-      for module_id in chunk.modules.iter().copied() {
-        match &self.link_output.modules[module_id] {
-          Module::Normal(module) => {
-            module.import_records.iter().for_each(|rec| {
-              match &self.link_output.modules[rec.resolved_module] {
-                Module::External(importee) if matches!(rec.kind, ImportKind::Import) => {
-                  // Make sure the side effects of external module are evaluated.
-                  imports_from_external_modules.entry(importee.id).or_default();
-                }
-                _ => {}
-              }
-            });
-            module.named_imports.iter().for_each(|(_, import)| {
-              let rec = &module.import_records[import.record_id];
-              if let Module::External(importee) = &self.link_output.modules[rec.resolved_module] {
-                imports_from_external_modules.entry(importee.id).or_default().push(import.clone());
-              }
-            });
-            for stmt_info in module.stmt_infos.iter() {
-              if !stmt_info.is_included {
-                continue;
-              }
-              for declared in &stmt_info.declared_symbols {
-                let symbol = self.link_output.symbols.get_mut(*declared);
-                debug_assert!(
-                  symbol.chunk_id.unwrap_or(chunk_id) == chunk_id,
-                  "Symbol: {:?}, {:?} in {:?} should only be declared in one chunk",
-                  symbol.name,
-                  declared,
-                  module.resource_id,
-                );
-
-                self.link_output.symbols.get_mut(*declared).chunk_id = Some(chunk_id);
-              }
-
-              for referenced in &stmt_info.referenced_symbols {
-                let canonical_ref = self.link_output.symbols.canonical_ref_for(*referenced);
-                chunk_meta_imports.insert(canonical_ref);
-              }
-            }
-          }
-          Module::External(_) => {}
-        }
-      }
+  pub fn compute_cross_chunk_links(&mut self, chunk_graph: &mut ChunkGraph) {
+    let mut chunk_meta_imports_vec: ChunkMetaImports =
+      index_vec![FxHashSet::<SymbolRef>::default(); chunk_graph.chunks.len()];
+    let mut chunk_meta_exports_vec: ChunkMetaExports =
+      index_vec![FxHashSet::<SymbolRef>::default(); chunk_graph.chunks.len()];
+    let mut chunk_meta_imports_from_external_modules_vec: ChunkMetaImportsForExternalModules =
+      index_vec![FxHashMap::<ModuleId, Vec<NamedImport>>::default(); chunk_graph.chunks.len()];
 
-      if let ChunkKind::EntryPoint { module: entry_module_id, .. } = &chunk.kind {
-        let entry_module = &self.link_output.modules[*entry_module_id];
-        let Module::Normal(entry_module) = entry_module else {
-          return;
-        };
-        let entry_linking_info = &self.link_output.linking_infos[entry_module.id];
-        if matches!(entry_module.exports_kind, ExportsKind::CommonJs)
-          && matches!(self.output_options.format, OutputFormat::Esm)
-        {
-          chunk_meta_imports
-            .insert(entry_linking_info.wrapper_ref.expect("cjs should be wrapped in esm output"));
-        }
-        for export_ref in entry_linking_info.resolved_exports.values() {
-          let mut canonical_ref = self.link_output.symbols.canonical_ref_for(export_ref.symbol_ref);
-          let symbol = self.link_output.symbols.get(canonical_ref);
-          if let Some(ns_alias) = &symbol.namespace_alias {
-            canonical_ref = ns_alias.namespace_ref;
-          }
-          chunk_meta_imports.insert(canonical_ref);
-        }
-      }
-    }
+    self.collect_potential_chunk_imports(
+      chunk_graph,
+      &mut chunk_meta_imports_vec,
+      &mut chunk_meta_imports_from_external_modules_vec,
+    );
 
-    for (chunk_id, chunk) in chunk_graph.chunks.iter_mut_enumerated() {
+    // - Find out what imports are actually come from other chunks
+    chunk_graph.chunks.iter_enumerated().for_each(|(chunk_id, chunk)| {
       let chunk_meta_imports = &chunk_meta_imports_vec[chunk_id];
       for import_ref in chunk_meta_imports.iter().copied() {
         let import_symbol = self.link_output.symbols.get(import_ref);
@@ -194,25 +143,37 @@ impl<'a> BundleStage<'a> {
             symbol_owner.resource_id()
           )
         });
-        // Find out the import_ref whether comes from the chunk or external module.
+        // Check if the import is from another chunk
         if chunk_id != importee_chunk_id {
-          chunk
-            .imports_from_other_chunks
+          let imports_from_other_chunks =
+          // Safety: No race condition here:
+          // - This loop is executed sequentially in a single thread
+            unsafe { &mut (*addr_of!(chunk.imports_from_other_chunks).cast_mut()) };
+          imports_from_other_chunks
             .entry(importee_chunk_id)
             .or_default()
             .push(CrossChunkImportItem { import_ref, export_alias: None });
           chunk_meta_exports_vec[importee_chunk_id].insert(import_ref);
         }
       }
 
-      if chunk.is_entry_point() {
-        continue;
-      }
       // If this is an entry point, make sure we import all chunks belonging to
       // this entry point, even if there are no imports. We need to make sure
       // these chunks are evaluated for their side effects too.
-      // TODO: ensure chunks are evaluated for their side effects too.
-    }
+      if let ChunkKind::EntryPoint { bit: importer_chunk_bit, .. } = &chunk.kind {
+        chunk_graph
+          .chunks
+          .iter_enumerated()
+          .filter(|(id, _)| *id != chunk_id)
+          .filter(|(_, importee_chunk)| importee_chunk.bits.has_bit(*importer_chunk_bit))
+          .for_each(|(importee_chunk_id, _)| {
+            let imports_from_other_chunks =
+              unsafe { &mut (*addr_of!(chunk.imports_from_other_chunks).cast_mut()) };
+            imports_from_other_chunks.entry(importee_chunk_id).or_default();
+          });
+      }
+    });
+
     // Generate cross-chunk exports. These must be computed before cross-chunk
     // imports because of export alias renaming, which must consider all export
     // aliases simultaneously to avoid collisions.

crates/rolldown/tests/esbuild/default/duplicate_entry_point/artifacts.snap

@@ -8,7 +8,7 @@ input_file: crates/rolldown/tests/esbuild/default/duplicate_entry_point
 ## entry2_js.mjs
 
 ```js
-
+import "./entry_js-2.mjs";
 ```
 ## entry_js-2.mjs
 
@@ -19,5 +19,5 @@ console.log(123);
 ## entry_js.mjs
 
 ```js
-
+import "./entry_js-2.mjs";
 ```

crates/rolldown/tests/esbuild/splitting/cross_chunk_assignment_dependencies/artifacts.snap

@@ -16,6 +16,8 @@ setValue(123);
 ## b.mjs
 
 ```js
+import "./shared_js.mjs";
+
 // b.js
 ```
 ## shared_js.mjs

crates/rolldown/tests/esbuild/splitting/duplicate_chunk_collision/artifacts.snap

@@ -8,6 +8,8 @@ input_file: crates/rolldown/tests/esbuild/splitting/duplicate_chunk_collision
 ## a.mjs
 
 ```js
+import "./ab_js.mjs";
+
 // a.js
 ```
 ## ab_js.mjs
@@ -19,12 +21,14 @@ console.log(123);
 ## b.mjs
 
 ```js
+import "./ab_js.mjs";
+
 // b.js
 ```
 ## c.mjs
 
 ```js
-
+import "./cd_js.mjs";
 ```
 ## cd_js.mjs
 
@@ -37,5 +41,5 @@ console.log(123);
 ## d.mjs
 
 ```js
-
+import "./cd_js.mjs";
 ```

crates/rolldown/tests/esbuild/splitting/hybrid_esm_and_cjs_issue617/artifacts.snap

@@ -8,6 +8,7 @@ input_file: crates/rolldown/tests/esbuild/splitting/hybrid_esm_and_cjs_issue617
 ## a.mjs
 
 ```js
+import "./$runtime$.mjs";
 import { foo } from "./a_js.mjs";
 
 export { foo };

crates/rolldown/tests/esbuild/splitting/shared-commonjs-into-es6/artifacts.snap

@@ -8,6 +8,7 @@ input_file: crates/rolldown/tests/esbuild/splitting/shared-commonjs-into-es6
 ## a.mjs
 
 ```js
+import "./$runtime$.mjs";
 import { require_shared } from "./shared_js.mjs";
 
 // a.js
@@ -17,6 +18,7 @@ console.log(foo);
 ## b.mjs
 
 ```js
+import "./$runtime$.mjs";
 import { require_shared } from "./shared_js.mjs";
 
 // b.js

crates/rolldown/tests/fixtures/code_splitting/artifacts.snap → crates/rolldown/tests/fixtures/code_splitting/basic/artifacts.snap

@@ -1,7 +1,7 @@
 ---
 source: crates/rolldown/tests/common/case.rs
 expression: content
-input_file: crates/rolldown/tests/fixtures/code_splitting
+input_file: crates/rolldown/tests/fixtures/code_splitting/basic
 ---
 # Assets
 
@@ -14,12 +14,16 @@ console.log('dynamic');
 ## main1.mjs
 
 ```js
+import "./share_js.mjs";
+
 // main1.js
 import('./dynamic_js.mjs');
 ```
 ## main2.mjs
 
 ```js
+import "./share_js.mjs";
+
 // main2.js
 ```
 ## share_js.mjs

crates/rolldown/tests/fixtures/code_splitting/ensure_side_effect_executed/_test.mjs

@@ -0,0 +1,2 @@
+import assert from 'assert';
+assert(globalThis.sideEffectExecuted, 'side effect not executed')

crates/rolldown/tests/fixtures/code_splitting/ensure_side_effect_executed/artifacts.snap

@@ -0,0 +1,23 @@
+---
+source: crates/rolldown/tests/common/case.rs
+expression: content
+input_file: crates/rolldown/tests/fixtures/code_splitting/ensure_side_effect_executed
+---
+# Assets
+
+## entry2_js.mjs
+
+```js
+import "./entry_js-2.mjs";
+```
+## entry_js-2.mjs
+
+```js
+// entry.js
+globalThis.sideEffectExecuted = true;
+```
+## entry_js.mjs
+
+```js
+import "./entry_js-2.mjs";
+```

crates/rolldown/tests/fixtures/code_splitting/ensure_side_effect_executed/entry.js

@@ -0,0 +1 @@
+globalThis.sideEffectExecuted = true;