Skip to content

Commit ad4dfd7

Browse files
authored
refactor: remove option in module_graph_module_by_identifier_mut (#12610)
1 parent 11c4d87 commit ad4dfd7

File tree

7 files changed

+91
-102
lines changed

7 files changed

+91
-102
lines changed

crates/rspack_core/src/cache/persistent/occasion/make/module_graph.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -181,9 +181,7 @@ pub async fn recovery_module_graph(
181181
});
182182
// recovery incoming connections
183183
for (dep_id, module_identifier) in need_check_dep {
184-
let mgm = mg
185-
.module_graph_module_by_identifier_mut(&module_identifier)
186-
.expect("should mgm exist");
184+
let mgm = mg.module_graph_module_by_identifier_mut(&module_identifier);
187185
mgm.add_incoming_connection(dep_id);
188186
}
189187

crates/rspack_core/src/compilation/build_chunk_graph/artifact.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -200,10 +200,7 @@ where
200200
let module_idx = cache.module_idx.clone();
201201
let module_graph = compilation.get_module_graph_mut();
202202
for (m, (pre, post)) in module_idx {
203-
let Some(mgm) = module_graph.module_graph_module_by_identifier_mut(&m) else {
204-
continue;
205-
};
206-
203+
let mgm = module_graph.module_graph_module_by_identifier_mut(&m);
207204
mgm.pre_order_index = Some(pre);
208205
mgm.post_order_index = Some(post);
209206
}

crates/rspack_core/src/compilation/build_chunk_graph/code_splitter.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1162,9 +1162,7 @@ Or do you want to use the entrypoints '{name}' and '{runtime}' independently on
11621162

11631163
{
11641164
let module_graph = compilation.get_module_graph_mut();
1165-
let module = module_graph
1166-
.module_graph_module_by_identifier_mut(&item.module)
1167-
.unwrap_or_else(|| panic!("No module found {:?}", &item.module));
1165+
let module = module_graph.module_graph_module_by_identifier_mut(&item.module);
11681166

11691167
if module.pre_order_index.is_none() {
11701168
module.pre_order_index = Some(self.next_free_module_pre_order_index);
@@ -1207,9 +1205,7 @@ Or do you want to use the entrypoints '{name}' and '{runtime}' independently on
12071205
}
12081206

12091207
let module_graph = compilation.get_module_graph_mut();
1210-
let module = module_graph
1211-
.module_graph_module_by_identifier_mut(&item.module)
1212-
.unwrap_or_else(|| panic!("no module found: {:?}", &item.module));
1208+
let module = module_graph.module_graph_module_by_identifier_mut(&item.module);
12131209

12141210
if module.post_order_index.is_none() {
12151211
module.post_order_index = Some(self.next_free_module_post_order_index);

crates/rspack_core/src/compilation/build_module_graph/graph_updater/cutout/fix_issuers.rs

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@ use rspack_collections::{IdentifierMap, IdentifierSet};
44
use rustc_hash::FxHashSet as HashSet;
55

66
use super::BuildModuleGraphArtifact;
7-
use crate::{DependencyId, ModuleGraph, ModuleIdentifier, ModuleIssuer};
7+
use crate::{
8+
DependencyId, ModuleGraph, ModuleIdentifier, ModuleIssuer,
9+
module_graph::internal::try_get_module_graph_module_mut_by_identifier,
10+
};
811

912
/// Result of IssuerHelper.is_issuer.
1013
enum IsIssuerResult {
@@ -215,7 +218,7 @@ impl FixIssuers {
215218
need_check_modules
216219
.into_iter()
217220
.filter_map(|mid| {
218-
let Some(mgm) = module_graph.module_graph_module_by_identifier_mut(&mid) else {
221+
let Some(mgm) = try_get_module_graph_module_mut_by_identifier(module_graph, &mid) else {
219222
// no mgm means the module has been removed, ignored.
220223
return None;
221224
};
@@ -308,9 +311,7 @@ impl FixIssuers {
308311
revoke_module.insert(mid);
309312
continue;
310313
};
311-
let mgm = module_graph
312-
.module_graph_module_by_identifier_mut(&mid)
313-
.expect("should mgm exist");
314+
let mgm = module_graph.module_graph_module_by_identifier_mut(&mid);
314315
artifact.issuer_update_modules.insert(mgm.module_identifier);
315316
mgm.set_issuer(match first_parent {
316317
Some(id) => ModuleIssuer::Some(*id),
@@ -359,9 +360,7 @@ impl FixIssuers {
359360
for (mid, parents) in need_check_available_modules {
360361
match helper.calc_issuer(module_graph, &mid, parents) {
361362
CalcIssuerResult::Ok(issuer) => {
362-
let mgm = module_graph
363-
.module_graph_module_by_identifier_mut(&mid)
364-
.expect("should have mgm");
363+
let mgm = module_graph.module_graph_module_by_identifier_mut(&mid);
365364
artifact.issuer_update_modules.insert(mgm.module_identifier);
366365
mgm.set_issuer(issuer);
367366
// check cycled modules.
@@ -375,9 +374,7 @@ impl FixIssuers {
375374
need_clean_cycle_modules.retain(|id, paths| {
376375
for (issuer, cycle_paths) in paths {
377376
if cycle_paths.contains(&current_id) {
378-
let mgm = module_graph
379-
.module_graph_module_by_identifier_mut(id)
380-
.expect("should have mgm");
377+
let mgm = module_graph.module_graph_module_by_identifier_mut(id);
381378
artifact.issuer_update_modules.insert(mgm.module_identifier);
382379
mgm.set_issuer(ModuleIssuer::Some(*issuer));
383380
// this module issuer has been update, add module to queue to recheck need_clean_cycle_modules
@@ -440,9 +437,7 @@ impl FixIssuers {
440437

441438
match helper.calc_issuer(module_graph, &mid, parents) {
442439
CalcIssuerResult::Ok(issuer) => {
443-
let mgm = module_graph
444-
.module_graph_module_by_identifier_mut(&mid)
445-
.expect("should have mgm");
440+
let mgm = module_graph.module_graph_module_by_identifier_mut(&mid);
446441
artifact.issuer_update_modules.insert(mgm.module_identifier);
447442
mgm.set_issuer(issuer);
448443
// fix child issuer
@@ -461,9 +456,7 @@ impl FixIssuers {
461456
.collect();
462457
for child_mid in child_modules {
463458
if useless_module.remove(&child_mid) {
464-
let mgm = module_graph
465-
.module_graph_module_by_identifier_mut(&child_mid)
466-
.expect("should have mgm");
459+
let mgm = module_graph.module_graph_module_by_identifier_mut(&child_mid);
467460
artifact.issuer_update_modules.insert(mgm.module_identifier);
468461
mgm.set_issuer(ModuleIssuer::Some(mid));
469462
fixing_queue.push_back(child_mid);

crates/rspack_core/src/compilation/build_module_graph/graph_updater/repair/build.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -197,9 +197,7 @@ impl Task<TaskContext> for BuildResultTask {
197197
}
198198

199199
{
200-
let mgm = module_graph
201-
.module_graph_module_by_identifier_mut(&module.identifier())
202-
.expect("Failed to get mgm");
200+
let mgm = module_graph.module_graph_module_by_identifier_mut(&module.identifier());
203201
mgm.all_dependencies = all_dependencies.clone();
204202
if let Some(current_profile) = current_profile {
205203
mgm.set_profile(current_profile);
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/// Internal helpers for ModuleGraph that should only be used by specific modules.
2+
///
3+
/// **DO NOT USE THESE FUNCTIONS** unless you're in `compilation::build_module_graph`.
4+
///
5+
/// This module provides restricted access to potentially unsafe ModuleGraph operations
6+
/// that should only be used in specific contexts where modules may legitimately not exist.
7+
use crate::{ModuleGraph, ModuleGraphModule, ModuleIdentifier};
8+
9+
/// Try to get a mutable module graph module by identifier, returning None if not found.
10+
///
11+
/// # Restricted Use - DO NOT USE
12+
///
13+
/// **WARNING**: This function should ONLY be used in `compilation::build_module_graph`
14+
/// for handling module removal during graph updates where modules may have been removed
15+
/// during incremental compilation.
16+
///
17+
/// **All other code must use `ModuleGraph::module_graph_module_by_identifier_mut()`**
18+
/// which enforces the invariant that the module exists with a clear panic message.
19+
///
20+
/// # When to Use (only in build_module_graph)
21+
///
22+
/// Only use when you have a legitimate reason to expect the module might not exist:
23+
/// - During incremental compilation where modules may have been removed
24+
/// - During graph cleanup operations where referenced modules may already be deleted
25+
///
26+
/// If you're unsure, use `module_graph_module_by_identifier_mut()` instead.
27+
#[inline]
28+
pub(crate) fn try_get_module_graph_module_mut_by_identifier<'a>(
29+
module_graph: &'a mut ModuleGraph,
30+
identifier: &ModuleIdentifier,
31+
) -> Option<&'a mut ModuleGraphModule> {
32+
module_graph.inner.module_graph_modules.get_mut(identifier)
33+
}

0 commit comments

Comments
 (0)