Visitar URL original
fix: 8027 · rolldown/rolldown@efd1716 · GitHub
Skip to content

Commit efd1716

Browse files
fix: 8027
1 parent 6f552c8 commit efd1716

9 files changed

Lines changed: 175 additions & 30 deletions

File tree

‎crates/rolldown/src/stages/generate_stage/chunk_optimizer.rs‎

Lines changed: 67 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@ use itertools::Itertools;
55
use oxc_index::IndexVec;
66
use rolldown_common::{
77
Chunk, ChunkDebugInfo, ChunkIdx, ChunkKind, ChunkMeta, ChunkReasonType,
8-
FacadeChunkEliminationReason, Module, ModuleIdx, ModuleNamespaceIncludedReason, ModuleTable,
9-
PostChunkOptimizationOperation, PreserveEntrySignatures, RuntimeHelper, StmtInfos, WrapKind,
8+
FacadeChunkEliminationReason, IdxDebugExt, Module, ModuleIdx, ModuleNamespaceIncludedReason,
9+
ModuleTable, PostChunkOptimizationOperation, PreserveEntrySignatures, RuntimeHelper, StmtInfos,
10+
WrapKind,
1011
};
1112
use rolldown_utils::{BitSet, indexmap::FxIndexMap};
1213
use rustc_hash::{FxHashMap, FxHashSet};
@@ -22,8 +23,10 @@ use crate::{
2223
};
2324

2425
use super::{
25-
GenerateStage, chunk_ext::ChunkCreationReason, chunk_ext::ChunkDebugExt,
26-
code_splitting::IndexSplittingInfo,
26+
GenerateStage,
27+
chunk_ext::ChunkCreationReason,
28+
chunk_ext::ChunkDebugExt,
29+
code_splitting::{IndexSplittingInfo, PendingCommonChunkInfo},
2730
};
2831

2932
/// Information about a facade chunk merge operation.
@@ -93,16 +96,40 @@ impl GenerateStage<'_> {
9396
chunk_graph: &mut ChunkGraph,
9497
bits_to_chunk: &mut FxHashMap<BitSet, ChunkIdx>,
9598
input_base: &ArcStr,
96-
pending_common_chunks: FxIndexMap<BitSet, Vec<ModuleIdx>>,
99+
pending_common_chunks: FxIndexMap<BitSet, PendingCommonChunkInfo>,
97100
) {
98101
let static_entry_chunk_reference: FxHashMap<ChunkIdx, FxHashSet<ChunkIdx>> =
99102
self.construct_static_entry_to_reached_dynamic_entries_map(chunk_graph);
100103

101104
let entry_chunk_idx =
102105
chunk_graph.chunk_table.iter_enumerated().map(|(idx, _)| idx).collect::<FxHashSet<_>>();
106+
// Calculate on demand to avoid add a new field on each NormalModule.
107+
let dynamic_entry_to_dynamic_importers: FxHashMap<ModuleIdx, FxHashSet<ModuleIdx>> = {
108+
// Get dynamic entry modules from chunk_table, then find matched entry points
109+
// and extract importers from related_stmt_infos
110+
let mut map: FxHashMap<ModuleIdx, FxHashSet<ModuleIdx>> = FxHashMap::default();
111+
for chunk in chunk_graph.chunk_table.iter() {
112+
let ChunkKind::EntryPoint { meta, module, .. } = chunk.kind else {
113+
continue;
114+
};
115+
if meta != ChunkMeta::DynamicImported {
116+
continue;
117+
}
118+
// Find the matched dynamic entry point from link_output.entries
119+
if let Some(entries) = self.link_output.entries.get(&module) {
120+
for entry in entries.iter().filter(|e| e.kind.is_dynamic_import()) {
121+
// Extract importers from related_stmt_infos (first element is the importer ModuleIdx)
122+
for (importer_idx, _, _, _) in &entry.related_stmt_infos {
123+
map.entry(module).or_default().insert(*importer_idx);
124+
}
125+
}
126+
}
127+
}
128+
map
129+
};
103130
// extract entry chunk module relation
104131
// this means `key_chunk` also referenced all entry module in value `vec`
105-
for (bits, modules) in pending_common_chunks {
132+
for (bits, PendingCommonChunkInfo { modules, meta: _ }) in pending_common_chunks {
106133
let chunk_idxs = bits
107134
.index_of_one()
108135
.into_iter()
@@ -117,6 +144,8 @@ impl GenerateStage<'_> {
117144
&static_entry_chunk_reference,
118145
chunk_graph,
119146
&self.link_output.module_table,
147+
&dynamic_entry_to_dynamic_importers,
148+
&modules,
120149
);
121150

122151
self.assign_modules_to_chunk(
@@ -238,6 +267,8 @@ impl GenerateStage<'_> {
238267
entry_chunk_reference: &FxHashMap<ChunkIdx, FxHashSet<ChunkIdx>>,
239268
chunk_graph: &ChunkGraph,
240269
module_table: &ModuleTable,
270+
dynamic_entry_to_dynamic_importers: &FxHashMap<ModuleIdx, FxHashSet<ModuleIdx>>,
271+
modules: &[ModuleIdx],
241272
) -> Option<ChunkIdx> {
242273
let mut user_defined_entry = vec![];
243274
let mut dynamic_entry = vec![];
@@ -262,18 +293,33 @@ impl GenerateStage<'_> {
262293
Self::find_merge_target(&user_defined_entry, &user_defined_entry_modules, module_table);
263294
if !user_defined_entry.is_empty() {
264295
let chunk_idx = merged_user_defined_chunk?;
265-
266-
let ret = dynamic_entry.iter().all(|idx| {
267-
entry_chunk_reference
268-
.get(&chunk_idx)
269-
.map(|reached_dynamic_chunk| reached_dynamic_chunk.contains(idx))
270-
.unwrap_or(false)
296+
let chunk = &chunk_graph.chunk_table[chunk_idx];
297+
let reached_dynamic_chunk = entry_chunk_reference.get(&chunk_idx);
298+
// Check if all dynamic entry chunks are reachable from the merged user-defined entry chunk.
299+
// If not, we cannot merge the shared modules into the user-defined entry chunk because
300+
// the dynamic entries would not be able to access the shared modules.
301+
let all_dynamic_entries_reachable =
302+
dynamic_entry.iter().all(|idx| reached_dynamic_chunk.is_some_and(|set| set.contains(idx)));
303+
if !all_dynamic_entries_reachable {
304+
return None;
305+
}
306+
let merged_entry_module = chunk.entry_module(module_table)?;
307+
// For each module in the shared modules, if it is a dynamic entry module,
308+
// all its dynamic importers must also statically import the entry module of the merged chunk.
309+
// This ensures that when a dynamic import occurs, the merged entry chunk (containing the
310+
// dynamic entry module) is already loaded, preventing missing module errors at runtime.
311+
let all_dynamic_entry_importers_valid = modules.iter().all(|&module_idx| {
312+
let Some(importers) = dynamic_entry_to_dynamic_importers.get(&module_idx) else {
313+
// Not a dynamic entry module, no constraint
314+
return true;
315+
};
316+
importers.iter().all(|importer| *importer == merged_entry_module.idx)
271317
});
272-
return ret.then_some(chunk_idx);
318+
all_dynamic_entry_importers_valid.then_some(chunk_idx)
319+
} else {
320+
let dynamic_chunk_entry_modules = Self::collect_entry_modules(&dynamic_entry, chunk_graph)?;
321+
Self::find_merge_target(&dynamic_entry, &dynamic_chunk_entry_modules, module_table)
273322
}
274-
275-
let dynamic_chunk_entry_modules = Self::collect_entry_modules(&dynamic_entry, chunk_graph)?;
276-
Self::find_merge_target(&dynamic_entry, &dynamic_chunk_entry_modules, module_table)
277323
}
278324

279325
/// Collects entry module indices from a list of chunk indices.
@@ -350,6 +396,7 @@ impl GenerateStage<'_> {
350396
/// Finds empty dynamic entry chunks that should be merged with their target common chunks.
351397
/// Returns a tuple of (merge_entry_to_chunk, emitted_chunk_groups).
352398
fn find_facade_chunk_merge_candidates(
399+
&self,
353400
chunk_graph: &ChunkGraph,
354401
) -> (FxHashMap<ModuleIdx, FacadeChunkMergeInfo>, FxHashMap<ChunkIdx, Vec<ChunkIdx>>) {
355402
let mut merge_entry_to_chunk = FxHashMap::default();
@@ -385,6 +432,9 @@ impl GenerateStage<'_> {
385432
ChunkReasonType::ManualCodeSplitting { .. }
386433
);
387434
let is_pure_user_defined_to_entry_chunk = matches!(target_chunk.kind, ChunkKind::EntryPoint { meta, bit: _, module: _ } if meta.is_pure_user_defined_entry());
435+
dbg!(&chunk_idx.debug(&self.link_output.module_table, &chunk_graph.chunk_table));
436+
dbg!(&target_chunk_idx.debug(&self.link_output.module_table, &chunk_graph.chunk_table));
437+
dbg!(&target_chunk.kind);
388438
// Three optimization scenarios:
389439
// 1. Emitted chunk (AllowExtension) merged into manual code splitting group
390440
// → Group by target chunk to detect export name conflicts before merging
@@ -477,7 +527,7 @@ impl GenerateStage<'_> {
477527
) {
478528
// Find empty dynamic entry chunks that should be merged with their target common chunks
479529
let (mut merge_entry_to_chunk, emitted_chunk_groups) =
480-
Self::find_facade_chunk_merge_candidates(chunk_graph);
530+
self.find_facade_chunk_merge_candidates(chunk_graph);
481531

482532
if merge_entry_to_chunk.is_empty() && emitted_chunk_groups.is_empty() {
483533
return;

‎crates/rolldown/src/stages/generate_stage/code_splitting.rs‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,20 @@ use rustc_hash::{FxHashMap, FxHashSet};
2424

2525
use super::{GenerateStage, chunk_ext::ChunkCreationReason};
2626

27+
bitflags::bitflags! {
28+
#[derive(Debug, Clone, Copy, Default)]
29+
pub struct PendingCommonChunkMeta: u8 {
30+
// Empty for now, will be extended as needed
31+
}
32+
}
33+
34+
#[derive(Debug, Default)]
35+
pub struct PendingCommonChunkInfo {
36+
pub modules: Vec<ModuleIdx>,
37+
#[expect(dead_code, reason = "Reserved for future use")]
38+
pub meta: PendingCommonChunkMeta,
39+
}
40+
2741
#[derive(Clone, Debug)]
2842
pub struct SplittingInfo {
2943
pub bits: BitSet,
@@ -803,7 +817,8 @@ impl GenerateStage<'_> {
803817
)
804818
.await?;
805819

806-
let mut pending_common_chunks: FxIndexMap<BitSet, Vec<ModuleIdx>> = FxIndexMap::default();
820+
let mut pending_common_chunks: FxIndexMap<BitSet, PendingCommonChunkInfo> =
821+
FxIndexMap::default();
807822
// If it is allow to allow that entry chunks have the different exports as the underlying entry module.
808823
// This is used to generate less chunks when possible.
809824
// TODO: maybe we could bailout peer chunk?
@@ -838,7 +853,7 @@ impl GenerateStage<'_> {
838853
self.link_output.metas[normal_module.idx].depended_runtime_helper,
839854
);
840855
} else if allow_chunk_optimization {
841-
pending_common_chunks.entry(bits.clone()).or_default().push(normal_module.idx);
856+
pending_common_chunks.entry(bits.clone()).or_default().modules.push(normal_module.idx);
842857
} else {
843858
let mut chunk =
844859
Chunk::new(None, None, bits.clone(), vec![], ChunkKind::Common, input_base.clone(), None);
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
{
2+
"config": {
3+
"input": [
4+
{
5+
"name": "main",
6+
"import": "./main.js"
7+
},
8+
{
9+
"name": "other",
10+
"import": "./other.js"
11+
}
12+
]
13+
}
14+
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
---
2+
source: crates/rolldown_testing/src/integration_test.rs
3+
---
4+
# Assets
5+
6+
## main.js
7+
8+
```js
9+
//#region main.js
10+
import("./shared.js").then(({ func }) => {
11+
func();
12+
});
13+
14+
//#endregion
15+
```
16+
17+
## other.js
18+
19+
```js
20+
21+
```
22+
23+
## shared.js
24+
25+
```js
26+
import { t as func } from "./shared2.js";
27+
28+
export { func };
29+
```
30+
31+
## shared2.js
32+
33+
```js
34+
//#region shared.js
35+
function func() {
36+
return "This is a shared function";
37+
}
38+
39+
//#endregion
40+
export { func as t };
41+
```
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import("./shared.js").then(({ func }) => {
2+
func();
3+
});
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
import "./shared.js";
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export function func() {
2+
return "This is a shared function";
3+
}

‎crates/rolldown/tests/rolldown/tree_shaking/issue_4682/artifacts.snap‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ console.log("dynamic no-treeshake");
1212
//#endregion
1313
//#region dynamic.js
1414
const lazyLoad = async () => {
15-
await import("./main.js");
15+
await import("./static2.js");
1616
document.body.classList.add("loaded");
1717
};
1818
@@ -23,19 +23,28 @@ export { lazyLoad };
2323
## main.js
2424

2525
```js
26-
// HIDDEN [rolldown:runtime]
27-
//#region static.no-treeshake.js
28-
console.log("static no-treeshake");
26+
import "./static.js";
2927
30-
//#endregion
31-
//#region static.js
32-
var static_exports = /* @__PURE__ */ __exportAll({});
33-
34-
//#endregion
3528
//#region main.js
3629
import("./dynamic.js").then(async ({ lazyLoad }) => {
3730
await lazyLoad();
3831
});
3932
4033
//#endregion
4134
```
35+
36+
## static.js
37+
38+
```js
39+
//#region static.no-treeshake.js
40+
console.log("static no-treeshake");
41+
42+
//#endregion
43+
```
44+
45+
## static2.js
46+
47+
```js
48+
import "./static.js";
49+
50+
```

‎crates/rolldown/tests/snapshots/integration_rolldown__filename_with_hash.snap‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3908,6 +3908,13 @@ expression: output
39083908
- common-!~{004}~.js => common-CVRK5TUp.js
39093909
- rolldown-runtime-!~{002}~.js => rolldown-runtime-ntFOhXPQ.js
39103910
3911+
# tests/rolldown/function/advanced_chunks/dynamic_entry_shared_module_not_merged
3912+
3913+
- main-!~{000}~.js => main-K27wQ6cE.js
3914+
- other-!~{001}~.js => other-B3P9919W.js
3915+
- shared-!~{004}~.js => shared-CclkS2J7.js
3916+
- shared-!~{002}~.js => shared-Cm3wnSX5.js
3917+
39113918
# tests/rolldown/function/advanced_chunks/include_dependencies_recursively
39123919
39133920
- main-!~{000}~.js => main-D4MNl3Sj.js
@@ -6361,8 +6368,10 @@ expression: output
63616368
63626369
# tests/rolldown/tree_shaking/issue_4682
63636370
6364-
- main-!~{000}~.js => main-CEjIU1Yi.js
6365-
- dynamic-!~{003}~.js => dynamic-YC0iu9Gc.js
6371+
- main-!~{000}~.js => main-B9FlAHGI.js
6372+
- dynamic-!~{005}~.js => dynamic-C1dXXjI-.js
6373+
- static-!~{003}~.js => static-BP37ah8X.js
6374+
- static-!~{001}~.js => static-CE4ZR3EF.js
63666375
63676376
# tests/rolldown/tree_shaking/json_default_import
63686377

0 commit comments

Comments
 (0)