diff --git a/crates/rspack_core/src/cache/persistent/occasion/make/module_graph.rs b/crates/rspack_core/src/cache/persistent/occasion/make/module_graph.rs index fb9c8ae9a270..1b7e4f3177a2 100644 --- a/crates/rspack_core/src/cache/persistent/occasion/make/module_graph.rs +++ b/crates/rspack_core/src/cache/persistent/occasion/make/module_graph.rs @@ -187,7 +187,7 @@ pub async fn recovery_module_graph( if module_exist { None } else { - mg.revoke_connection(dep_id, false) + mg.revoke_dependency(dep_id, false) } }) .collect::>(); diff --git a/crates/rspack_core/src/compiler/compilation.rs b/crates/rspack_core/src/compiler/compilation.rs index 923e1d5fe599..d9cbc9e2345d 100644 --- a/crates/rspack_core/src/compiler/compilation.rs +++ b/crates/rspack_core/src/compiler/compilation.rs @@ -821,7 +821,6 @@ impl Compilation { #[instrument("Compilation:make", skip_all)] pub async fn make(&mut self) -> Result<()> { self.make_artifact.reset_dependencies_incremental_info(); - // self.module_executor. // run module_executor if let Some(module_executor) = &mut self.module_executor { let mut module_executor = std::mem::take(module_executor); diff --git a/crates/rspack_core/src/compiler/make/cutout/mod.rs b/crates/rspack_core/src/compiler/make/cutout/mod.rs index 2d7adc9ea470..7c18c098882e 100644 --- a/crates/rspack_core/src/compiler/make/cutout/mod.rs +++ b/crates/rspack_core/src/compiler/make/cutout/mod.rs @@ -3,7 +3,7 @@ mod fix_build_meta; mod fix_issuers; mod has_module_graph_change; -use rspack_collections::IdentifierSet; +use rspack_collections::{IdentifierSet, UkeySet}; use rustc_hash::FxHashSet as HashSet; use self::{ @@ -27,33 +27,22 @@ impl Cutout { artifact: &mut MakeArtifact, params: Vec, ) -> HashSet { - let mut entry_dependencies = std::mem::take(&mut artifact.entry_dependencies); + let mut next_entry_dependencies = HashSet::default(); + let mut clean_entry_dependencies = false; + let mut useless_entry_dependencies = UkeySet::default(); let mut force_build_modules = IdentifierSet::default(); let mut force_build_deps = HashSet::default(); - let mut remove_entry_deps = HashSet::default(); let module_graph = artifact.get_module_graph(); for item in params { match item { MakeParam::BuildEntry(deps) => { - for dep_id in deps { - if !entry_dependencies.contains(&dep_id) { - force_build_deps.insert((dep_id, None)); - entry_dependencies.insert(dep_id); - } - } + next_entry_dependencies = deps; } MakeParam::BuildEntryAndClean(deps) => { - remove_entry_deps.extend(std::mem::take(&mut entry_dependencies)); - entry_dependencies = deps; - for dep_id in &entry_dependencies { - if remove_entry_deps.contains(dep_id) { - remove_entry_deps.remove(dep_id); - } else { - force_build_deps.insert((*dep_id, None)); - } - } + next_entry_dependencies = deps; + clean_entry_dependencies = true; } MakeParam::CheckNeedBuild => { force_build_modules.extend(module_graph.modules().values().filter_map(|module| { @@ -65,32 +54,30 @@ impl Cutout { })); } MakeParam::ModifiedFiles(files) => { - force_build_modules.extend(module_graph.modules().values().filter_map(|module| { + for module in module_graph.modules().values() { // check has dependencies modified if module.depends_on(&files) { - Some(module.identifier()) - } else { - None + // add module id + force_build_modules.insert(module.identifier()); } - })) + } } MakeParam::RemovedFiles(files) => { - force_build_modules.extend(module_graph.modules().values().flat_map(|module| { - let mut res = vec![]; - + for module in module_graph.modules().values() { // check has dependencies modified if module.depends_on(&files) { // add module id - res.push(module.identifier()); - // add parent module id - res.extend( - module_graph - .get_incoming_connections(&module.identifier()) - .filter_map(|connect| connect.original_module_identifier), - ) + force_build_modules.insert(module.identifier()); + // process parent module id + for connect in module_graph.get_incoming_connections(&module.identifier()) { + if let Some(original_module_identifier) = connect.original_module_identifier { + force_build_modules.insert(original_module_identifier); + } else { + useless_entry_dependencies.insert(connect.dependency_id); + } + } } - res - })) + } } MakeParam::ForceBuildDeps(deps) => { for item in deps { @@ -128,23 +115,55 @@ impl Cutout { force_build_deps.extend(artifact.revoke_module(&id)); } + let mut entry_dependencies = std::mem::take(&mut artifact.entry_dependencies); let mut module_graph = artifact.get_module_graph_mut(); - for dep_id in remove_entry_deps { + // remove useless entry dependencies + let mut remove_entry_dependencies = HashSet::default(); + for dep_id in useless_entry_dependencies { + if !next_entry_dependencies.contains(&dep_id) { + remove_entry_dependencies.insert(dep_id); + } + } + if clean_entry_dependencies { + remove_entry_dependencies.extend(entry_dependencies.difference(&next_entry_dependencies)); + } + for dep_id in remove_entry_dependencies { // connection may have been deleted by revoke module if let Some(con) = module_graph.connection_by_dependency_id(&dep_id) { + // need clean_isolated_module to check whether the module is still used by other deps self .clean_isolated_module .add_need_check_module(*con.module_identifier()); - module_graph.revoke_connection(&dep_id, true); } - force_build_deps.remove(&(dep_id, None)); + module_graph.revoke_dependency(&dep_id, true); + entry_dependencies.remove(&dep_id); + } + + // add entry dependencies + for dep in next_entry_dependencies + .difference(&entry_dependencies) + .copied() + .collect::>() + { + force_build_deps.insert((dep, None)); + entry_dependencies.insert(dep); } artifact.entry_dependencies = entry_dependencies; self.has_module_graph_change.analyze_artifact(artifact); + // only return available force_build_deps + let module_graph = artifact.get_module_graph(); force_build_deps + .into_iter() + .filter(|(dep_id, _)| { + let Some(dep) = module_graph.dependency_by_id(dep_id) else { + return false; + }; + dep.as_module_dependency().is_some() || dep.as_context_dependency().is_some() + }) + .collect() } pub fn fix_artifact(self, artifact: &mut MakeArtifact) { diff --git a/crates/rspack_core/src/compiler/make/repair/mod.rs b/crates/rspack_core/src/compiler/make/repair/mod.rs index a544005e581f..e4cf8802f1b1 100644 --- a/crates/rspack_core/src/compiler/make/repair/mod.rs +++ b/crates/rspack_core/src/compiler/make/repair/mod.rs @@ -102,22 +102,13 @@ pub async fn repair( let module_graph = artifact.get_module_graph_mut(); let init_tasks = build_dependencies .into_iter() - .filter_map::>, _>(|(id, parent_module_identifier)| { + .map::>, _>(|(id, parent_module_identifier)| { let dependency = module_graph .dependency_by_id(&id) .expect("dependency not found"); - // filter module_dependency and context_dependency - if dependency.as_module_dependency().is_none() && dependency.as_context_dependency().is_none() - { - return None; - } - // filter parent module existed dependency let parent_module = parent_module_identifier.and_then(|id| module_graph.module_by_identifier(&id)); - if parent_module_identifier.is_some() && parent_module.is_none() { - return None; - } let current_profile = compilation .options @@ -134,7 +125,7 @@ pub async fn repair( None } }); - Some(Box::new(factorize::FactorizeTask { + Box::new(factorize::FactorizeTask { compilation_id: compilation.id(), module_factory: compilation.get_dependency_factory(dependency), original_module_identifier: parent_module_identifier, @@ -149,7 +140,7 @@ pub async fn repair( options: compilation.options.clone(), current_profile, resolver_factory: compilation.resolver_factory.clone(), - })) + }) }) .collect::>(); diff --git a/crates/rspack_core/src/module_graph/mod.rs b/crates/rspack_core/src/module_graph/mod.rs index 3c55dd38921e..fdaf60dae81b 100644 --- a/crates/rspack_core/src/module_graph/mod.rs +++ b/crates/rspack_core/src/module_graph/mod.rs @@ -213,22 +213,27 @@ impl<'a> ModuleGraph<'a> { map } - /// Remove a connection and return connection origin module identifier and dependency + /// Remove dependency in mgm and target connection, return dependency_id and origin module identifier /// /// force will completely remove dependency, and you will not regenerate it from dependency_id - pub fn revoke_connection( + pub fn revoke_dependency( &mut self, dep_id: &DependencyId, force: bool, ) -> Option { - let connection = self.connection_by_dependency_id(dep_id)?; - let module_identifier = *connection.module_identifier(); - let original_module_identifier = connection.original_module_identifier; + let connection_info = self.connection_by_dependency_id(dep_id).map(|connection| { + ( + connection.original_module_identifier, + *connection.module_identifier(), + ) + }); let Some(active_partial) = &mut self.active else { panic!("should have active partial"); }; - active_partial.connections.insert(*dep_id, None); + if connection_info.is_some() { + active_partial.connections.insert(*dep_id, None); + } if force { active_partial.dependencies.insert(*dep_id, None); active_partial @@ -236,12 +241,13 @@ impl<'a> ModuleGraph<'a> { .insert(*dep_id, None); } + let (original_module_identifier, module_identifier) = connection_info?; // remove outgoing from original module graph module if let Some(original_module_identifier) = &original_module_identifier { if let Some(mgm) = self.module_graph_module_by_identifier_mut(original_module_identifier) { mgm.remove_outgoing_connection(dep_id); - // Because of mgm.dependencies is set when original module build success - // it does not need to remove dependency in mgm.dependencies. + // Because of mgm.all_dependencies is set when original module build success + // it does not need to remove dependency in mgm.all_dependencies. } } // remove incoming from module graph module @@ -258,12 +264,12 @@ impl<'a> ModuleGraph<'a> { .map(|m| Vec::from(m.get_blocks())) .unwrap_or_default(); - let (outgoing_connections, incoming_connections) = self + let (incoming_connections, all_dependencies) = self .module_graph_module_by_identifier(module_id) .map(|mgm| { ( - mgm.outgoing_connections().clone(), mgm.incoming_connections().clone(), + mgm.all_dependencies.clone(), ) }) .unwrap_or_default(); @@ -279,13 +285,13 @@ impl<'a> ModuleGraph<'a> { active_partial.blocks.insert(block, None); } - for cid in outgoing_connections { - self.revoke_connection(&cid, true); + for dep_id in all_dependencies { + self.revoke_dependency(&dep_id, true); } incoming_connections .iter() - .filter_map(|cid| self.revoke_connection(cid, false)) + .filter_map(|dep_id| self.revoke_dependency(dep_id, false)) .collect() } diff --git a/crates/rspack_plugin_library/src/modern_module_library_plugin.rs b/crates/rspack_plugin_library/src/modern_module_library_plugin.rs index f539922abd43..0516d5d78766 100644 --- a/crates/rspack_plugin_library/src/modern_module_library_plugin.rs +++ b/crates/rspack_plugin_library/src/modern_module_library_plugin.rs @@ -273,7 +273,7 @@ async fn finish_modules(&self, compilation: &mut Compilation) -> Result<()> { let boxed_dep = Box::new(new_dep.clone()) as BoxDependency; block.add_dependency_id(*new_dep.id()); mg.add_dependency(boxed_dep); - mg.revoke_connection(connection_id, true); + mg.revoke_dependency(connection_id, true); } } @@ -380,7 +380,7 @@ async fn finish_modules(&self, compilation: &mut Compilation) -> Result<()> { .expect("should have module"); importer.remove_dependency_id(*connection); - mg.revoke_connection(connection, true); + mg.revoke_dependency(connection, true); } } diff --git a/tests/e2e/cases/make/remove-dynamic-entry/index.test.ts b/tests/e2e/cases/make/remove-dynamic-entry/index.test.ts new file mode 100644 index 000000000000..96b7a813a3a4 --- /dev/null +++ b/tests/e2e/cases/make/remove-dynamic-entry/index.test.ts @@ -0,0 +1,19 @@ +import { expect, test } from "@/fixtures"; + +test("should compile", async ({ page, fileAction, rspack }) => { + await expect(page.getByText("index1")).toBeVisible(); + await expect(page.getByText("index2")).toBeVisible(); + + fileAction.deleteFile("src/index2.js"); + fileAction.updateFile("src/index1.js", content => content.replace("div.innerText = \"index1\";", "div.innerText = \"index1 updated\";")); + + await expect(async () => { + await page.reload(); + expect(await page.locator("#index1").innerText()).toBe("index1 updated"); + }).toPass(); + await expect(page.locator("#index2")).toHaveCount(0); + await expect(page.locator("#webpack-dev-server-client-overlay")).toHaveCount(0); + + const stats = rspack.compiler._lastCompilation?.getStats().toJson({ all: false, errors: true }); + expect(stats?.errors?.length).toBe(0); +}); diff --git a/tests/e2e/cases/make/remove-dynamic-entry/rspack.config.js b/tests/e2e/cases/make/remove-dynamic-entry/rspack.config.js new file mode 100644 index 000000000000..4aec6f15ebea --- /dev/null +++ b/tests/e2e/cases/make/remove-dynamic-entry/rspack.config.js @@ -0,0 +1,25 @@ +const path = require("path") +const fs = require("fs"); +const rspack = require("@rspack/core"); + +/** @type {import("@rspack/core").Configuration} */ +module.exports = { + entry: async () => { + const context = path.resolve(__dirname, "src"); + const files = await fs.promises.readdir(context); + let entries = files.filter(f => f.startsWith('index')); + entries.sort(); + return entries.reduce((acc, e, i) => { + acc[`index${i + 1}`] = path.resolve(context, e); + return acc; + }, {}) + }, + context: __dirname, + mode: "development", + plugins: [ + new rspack.HtmlRspackPlugin(), + ], + devServer: { + hot: true + }, +}; diff --git a/tests/e2e/cases/make/remove-dynamic-entry/src/index1.js b/tests/e2e/cases/make/remove-dynamic-entry/src/index1.js new file mode 100644 index 000000000000..c2b8145da4e1 --- /dev/null +++ b/tests/e2e/cases/make/remove-dynamic-entry/src/index1.js @@ -0,0 +1,6 @@ +Promise.resolve().then(() => { + const div = document.createElement("div"); + div.innerText = "index1"; + div.id = "index1"; + document.body.appendChild(div); +}) diff --git a/tests/e2e/cases/make/remove-dynamic-entry/src/index2.js b/tests/e2e/cases/make/remove-dynamic-entry/src/index2.js new file mode 100644 index 000000000000..53c4d8b510b2 --- /dev/null +++ b/tests/e2e/cases/make/remove-dynamic-entry/src/index2.js @@ -0,0 +1,4 @@ +const div = document.createElement("div"); +div.innerText = "index2"; +div.id = "index2"; +document.body.appendChild(div);