Skip to content

Commit

Permalink
fix: change according reviews
Browse files Browse the repository at this point in the history
  • Loading branch information
OceanPresentChao committed Nov 6, 2023
1 parent cd8b79b commit e275d1a
Show file tree
Hide file tree
Showing 13 changed files with 44 additions and 37 deletions.
3 changes: 3 additions & 0 deletions crates/rspack_core/src/compiler/compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1185,6 +1185,9 @@ impl Compilation {
pub async fn seal(&mut self, plugin_driver: SharedPluginDriver) -> Result<()> {
let logger = self.get_logger("rspack.Compilation");

// https://github.com/webpack/webpack/blob/main/lib/Compilation.js#L2809
plugin_driver.seal(self)?;

let start = logger.time("optimize dependencies");
// https://github.com/webpack/webpack/blob/d15c73469fd71cf98734685225250148b68ddc79/lib/Compilation.js#L2812-L2814
while plugin_driver.optimize_dependencies(self).await?.is_some() {}
Expand Down
4 changes: 4 additions & 0 deletions crates/rspack_core/src/plugin/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,10 @@ pub trait Plugin: Debug + Send + Sync {
async fn after_emit(&self, _compilation: &mut Compilation) -> Result<()> {
Ok(())
}

fn seal(&self, _compilation: &mut Compilation) -> Result<()> {
Ok(())
}
}

pub type BoxPlugin = Box<dyn Plugin>;
Expand Down
8 changes: 8 additions & 0 deletions crates/rspack_core/src/plugin/plugin_driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -631,4 +631,12 @@ impl PluginDriver {
}
Ok(())
}

#[instrument(name = "plugin:seal", skip_all)]
pub fn seal(&self, compilation: &mut Compilation) -> Result<()> {
for plugin in &self.plugins {
plugin.seal(compilation)?;
}
Ok(())
}
}
2 changes: 1 addition & 1 deletion crates/rspack_ids/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ mod deterministic_module_ids_plugin;
pub use deterministic_module_ids_plugin::*;
mod named_module_ids_plugin;
pub use named_module_ids_plugin::*;
pub mod id_helpers;
pub(crate) mod id_helpers;
mod named_chunk_ids_plugin;
pub use named_chunk_ids_plugin::*;
mod stable_named_chunk_ids_plugin;
Expand Down
37 changes: 22 additions & 15 deletions crates/rspack_plugin_warn_sensitive_module/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,28 @@ impl WarnCaseSensitiveModulesPlugin {
Self
}

#[allow(clippy::borrowed_box)]
pub fn create_sensitive_modules_warning(
&self,
modules: &Vec<&&Box<dyn Module>>,
modules: &Vec<&dyn Module>,
graph: &ModuleGraph,
) -> String {
let mut message =
String::from("There are multiple modules with names that only differ in casing.\n");

for m in modules {
let mut module_msg = format!(" - {}\n", m.identifier().to_string());
graph.get_incoming_connections(m).iter().for_each(|c| {
if let Some(original_identifier) = c.original_module_identifier {
module_msg.push_str(&format!(" - used by {}\n", original_identifier));
}
});
message.push_str(&module_msg);
if let Some(boxed_m) = graph.module_by_identifier(&m.identifier()) {
let mut module_msg = format!(" - {}\n", m.identifier());
graph
.get_incoming_connections(boxed_m)
.iter()
.for_each(|c| {
if let Some(original_identifier) = c.original_module_identifier {
module_msg.push_str(&format!(" - used by {}\n", original_identifier));
}
});

message.push_str(&module_msg);
}
}

message
Expand All @@ -45,7 +50,7 @@ impl Plugin for WarnCaseSensitiveModulesPlugin {
"rspack.WarnCaseSensitiveModulesPlugin"
}

fn module_ids(&self, compilation: &mut Compilation) -> rspack_error::Result<()> {
fn seal(&self, compilation: &mut Compilation) -> rspack_error::Result<()> {
let logger = compilation.get_logger(self.name());
let start = logger.time("check case sensitive modules");
let diagnostics: DashSet<Diagnostic> = DashSet::default();
Expand All @@ -71,15 +76,17 @@ impl Plugin for WarnCaseSensitiveModulesPlugin {

let identifier = module.identifier().to_string();
let lower_identifier = identifier.to_lowercase();
let lower_map = module_without_case_map
.entry(lower_identifier)
.or_insert(HashMap::new());
let lower_map = module_without_case_map.entry(lower_identifier).or_default();
lower_map.insert(identifier, module);
}

for lower_map in module_without_case_map.values() {
// sort by module identifier, guarantee the warning order
let mut case_map_vec = module_without_case_map.into_iter().collect::<Vec<_>>();
case_map_vec.sort_by(|a, b| a.0.cmp(&b.0));

for (_, lower_map) in case_map_vec {
if lower_map.values().len() > 1 {
let mut case_modules = lower_map.values().collect::<Vec<_>>();
let mut case_modules = lower_map.values().map(|m| m.as_ref()).collect::<Vec<_>>();
case_modules.sort_by_key(|m| m.identifier());
diagnostics.insert(Diagnostic::warn(
"Sensitive Modules Warn".to_string(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
module.exports = [
[
/There are multiple modules with names that only differ in casing/,
/(case-sensitive.A\.js)|(case-sensitive.B.file\.js)/,
/(case-sensitive.a\.js)|(case-sensitive.b.file\.js)/
/case-sensitive.A\.js/,
/case-sensitive.a\.js/
],
[
/There are multiple modules with names that only differ in casing/,
/(case-sensitive.A\.js)|(case-sensitive.B.file\.js)/,
/(case-sensitive.a\.js)|(case-sensitive.b.file\.js)/
/case-sensitive.B.file\.js/,
/case-sensitive.b.file\.js/
]
];
Empty file.
Empty file.
Empty file.
Empty file.

This file was deleted.

This file was deleted.

4 changes: 2 additions & 2 deletions webpack-test/cases/errors/case-sensitive/warnings.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module.exports = [
[/There are multiple modules with names that only differ in casing/, /(case-sensitive.A\.js)|(case-sensitive.B.file\.js)/, /(case-sensitive.a\.js)|(case-sensitive.b.file\.js)/],
[/There are multiple modules with names that only differ in casing/, /(case-sensitive.A\.js)|(case-sensitive.B.file\.js)/, /(case-sensitive.a\.js)|(case-sensitive.b.file\.js)/]
[/There are multiple modules with names that only differ in casing/, /case-sensitive.A\.js/, /case-sensitive.a\.js/],
[/There are multiple modules with names that only differ in casing/, /case-sensitive.B.file\.js/, /case-sensitive.b.file\.js/]
];

0 comments on commit e275d1a

Please sign in to comment.