fix: passively rebuild modules imported by importModule (web-infra-…

…dev#8595)

* fix

* finish

* add test

* update tests

* ifx

crates/rspack_core/src/compiler/make/mod.rs

@@ -1,4 +1,4 @@
-mod cutout;
+pub mod cutout;
 pub mod repair;
 
 use rspack_collections::IdentifierSet;

crates/rspack_core/src/compiler/module_executor/mod.rs

@@ -8,6 +8,7 @@ use dashmap::{mapref::entry::Entry, DashSet};
 pub use execute::ExecuteModuleId;
 pub use execute::ExecutedRuntimeModule;
 use rspack_collections::{Identifier, IdentifierDashMap, IdentifierDashSet};
+use rustc_hash::FxHashSet as HashSet;
 use tokio::sync::{
   mpsc::{unbounded_channel, UnboundedSender},
   oneshot,
@@ -19,17 +20,26 @@ use self::{
   execute::{ExecuteModuleResult, ExecuteTask},
   overwrite::OverwriteTask,
 };
-use super::make::{repair::MakeTaskContext, update_module_graph, MakeArtifact, MakeParam};
+use super::make::cutout::Cutout;
+use super::make::repair::repair;
+use super::make::{repair::MakeTaskContext, MakeArtifact, MakeParam};
 use crate::cache::new_cache;
 use crate::incremental::Mutation;
 use crate::{
   task_loop::run_task_loop_with_event, Compilation, CompilationAsset, Context, Dependency,
   DependencyId, LoaderImportDependency, PublicPath,
 };
 
+#[derive(Debug)]
+struct DepStatus {
+  id: DependencyId,
+  should_update: bool,
+}
+
 #[derive(Debug, Default)]
 pub struct ModuleExecutor {
-  request_dep_map: DashMap<(String, Option<String>), DependencyId>,
+  cutout: Cutout,
+  request_dep_map: DashMap<(String, Option<String>), DepStatus>,
   pub make_artifact: MakeArtifact,
 
   event_sender: Option<UnboundedSender<Event>>,
@@ -65,7 +75,20 @@ impl ModuleExecutor {
     make_artifact.diagnostics = Default::default();
     make_artifact.has_module_graph_change = false;
 
-    make_artifact = update_module_graph(compilation, make_artifact, params)
+    // Modules imported by `importModule` are passively loaded.
+    let mut build_dependencies = self.cutout.cutout_artifact(&mut make_artifact, params);
+    let mut build_dependencies_id = build_dependencies
+      .iter()
+      .map(|(id, _)| *id)
+      .collect::<HashSet<_>>();
+    for mut dep_status in self.request_dep_map.iter_mut() {
+      if build_dependencies_id.contains(&dep_status.id) {
+        dep_status.should_update = true;
+        build_dependencies_id.remove(&dep_status.id);
+      }
+    }
+    build_dependencies.retain(|dep| build_dependencies_id.contains(&dep.0));
+    make_artifact = repair(compilation, make_artifact, build_dependencies)
       .await
       .unwrap_or_default();
 
@@ -116,6 +139,9 @@ impl ModuleExecutor {
       panic!("receive make artifact failed");
     }
 
+    let cutout = std::mem::take(&mut self.cutout);
+    cutout.fix_artifact(&mut self.make_artifact);
+
     let module_assets = std::mem::take(&mut self.module_assets);
     for (original_module_identifier, files) in module_assets {
       let assets = compilation
@@ -205,12 +231,26 @@ impl ModuleExecutor {
           original_module_context.unwrap_or(Context::from("")),
         );
         let dep_id = *dep.id();
-        v.insert(dep_id);
+        v.insert(DepStatus {
+          id: dep_id,
+          should_update: false,
+        });
         (ExecuteParam::Entry(Box::new(dep), layer.clone()), dep_id)
       }
-      Entry::Occupied(v) => {
-        let dep_id = *v.get();
-        (ExecuteParam::DependencyId(dep_id), dep_id)
+      Entry::Occupied(mut v) => {
+        let dep_status = v.get_mut();
+        let dep_id = dep_status.id;
+        if dep_status.should_update {
+          let dep = LoaderImportDependency::new_with_id(
+            dep_id,
+            request.clone(),
+            original_module_context.unwrap_or(Context::from("")),
+          );
+          dep_status.should_update = false;
+          (ExecuteParam::Entry(Box::new(dep), layer.clone()), dep_id)
+        } else {
+          (ExecuteParam::DependencyId(dep_id), dep_id)
+        }
       }
     };
 

crates/rspack_core/src/dependency/loader_import.rs

@@ -22,6 +22,14 @@ impl LoaderImportDependency {
       id: DependencyId::new(),
     }
   }
+
+  pub fn new_with_id(id: DependencyId, request: String, context: Context) -> Self {
+    Self {
+      request,
+      context,
+      id,
+    }
+  }
 }
 
 impl AsDependencyTemplate for LoaderImportDependency {}

packages/rspack-test-tools/tests/hotCases/loader/import-module-1/__snapshots__/web/0.snap.txt

@@ -0,0 +1,12 @@
+# Case import-module-1: Step 0
+
+## Changed Files
+
+
+## Asset Files
+- Bundle: bundle.js
+
+## Manifest
+
+
+## Update

packages/rspack-test-tools/tests/hotCases/loader/import-module-1/__snapshots__/web/1.snap.txt

@@ -0,0 +1,56 @@
+# Case import-module-1: Step 1
+
+## Changed Files
+- a.js
+
+## Asset Files
+- Bundle: bundle.js
+- Manifest: main.LAST_HASH.hot-update.json, size: 28
+- Update: main.LAST_HASH.hot-update.js, size: 532
+
+## Manifest
+
+### main.LAST_HASH.hot-update.json
+
+```json
+{"c":["main"],"r":[],"m":[]}
+```
+
+
+## Update
+
+
+### main.LAST_HASH.hot-update.js
+
+#### Changed Modules
+- ./loader.js!./a.js
+
+#### Changed Runtime Modules
+- webpack/runtime/get_full_hash
+
+#### Changed Content
+```js
+"use strict";
+self["webpackHotUpdate"]('main', {
+"./loader.js!./a.js": (function (__unused_webpack_module, __webpack_exports__, __webpack_require__) {
+__webpack_require__.r(__webpack_exports__);
+__webpack_require__.d(__webpack_exports__, {
+  "default": function() { return __WEBPACK_DEFAULT_EXPORT__; }
+});
+/* ESM default export */ const __WEBPACK_DEFAULT_EXPORT__ = (2);
+
+
+}),
+
+},function(__webpack_require__) {
+// webpack/runtime/get_full_hash
+(() => {
+__webpack_require__.h = function () {
+	return "CURRENT_HASH";
+};
+
+})();
+
+}
+);
+```

packages/rspack-test-tools/tests/hotCases/loader/import-module-1/a.js

@@ -0,0 +1,3 @@
+export default 1;
+---
+export default 2;

packages/rspack-test-tools/tests/hotCases/loader/import-module-1/index.js

@@ -0,0 +1,13 @@
+import a from "./loader.js!./a";
+
+it("module and its loader-referencing module should update in right order", (done) => {
+  expect(a).toBe(1);
+  NEXT(
+    require('../../update')(done, true, () => {
+      expect(a).toBe(2);
+      done();
+    }),
+  );
+});
+
+module.hot.accept('./loader.js!./a');

packages/rspack-test-tools/tests/hotCases/loader/import-module-1/loader.js

@@ -0,0 +1,6 @@
+module.exports = async function (content) {
+	if (!content.includes("2")) {
+		await this.importModule("./loader2.js!./a.js");
+	}
+	return content;
+};

packages/rspack-test-tools/tests/hotCases/loader/import-module-1/loader2.js

@@ -0,0 +1,6 @@
+module.exports = async function (content) {
+	if (content.includes("2")) {
+		throw "should not throw";
+	}
+	return content;
+};

packages/rspack-test-tools/tests/hotCases/loader/import-module-2/__snapshots__/web/0.snap.txt

@@ -0,0 +1,12 @@
+# Case import-module-2: Step 0
+
+## Changed Files
+
+
+## Asset Files
+- Bundle: bundle.js
+
+## Manifest
+
+
+## Update

packages/rspack-test-tools/tests/hotCases/loader/import-module-2/__snapshots__/web/1.snap.txt

@@ -0,0 +1,50 @@
+# Case import-module-2: Step 1
+
+## Changed Files
+- import_module_sub.js
+
+## Asset Files
+- Bundle: bundle.js
+- Manifest: main.LAST_HASH.hot-update.json, size: 28
+- Update: main.LAST_HASH.hot-update.js, size: 257
+
+## Manifest
+
+### main.LAST_HASH.hot-update.json
+
+```json
+{"c":["main"],"r":[],"m":[]}
+```
+
+
+## Update
+
+
+### main.LAST_HASH.hot-update.js
+
+#### Changed Modules
+- ./loader.js!./a.js
+
+#### Changed Runtime Modules
+- webpack/runtime/get_full_hash
+
+#### Changed Content
+```js
+self["webpackHotUpdate"]('main', {
+"./loader.js!./a.js": (function (module) {
+module.exports = 3;
+
+}),
+
+},function(__webpack_require__) {
+// webpack/runtime/get_full_hash
+(() => {
+__webpack_require__.h = function () {
+	return "CURRENT_HASH";
+};
+
+})();
+
+}
+);
+```

packages/rspack-test-tools/tests/hotCases/loader/import-module-2/a.js

@@ -0,0 +1 @@
+module.exports = 1;

packages/rspack-test-tools/tests/hotCases/loader/import-module-2/import_module_root.js

@@ -0,0 +1,2 @@
+const loader = require("./import_module_sub.js");
+module.exports = loader;

packages/rspack-test-tools/tests/hotCases/loader/import-module-2/import_module_sub.js

@@ -0,0 +1,3 @@
+module.exports = 2;
+---
+module.exports = 3;

packages/rspack-test-tools/tests/hotCases/loader/import-module-2/index.js

@@ -0,0 +1,10 @@
+it("module and its loader-referencing module should update in right order", done => {
+	expect(require("./loader.js!./a")).toBe(2);
+	NEXT(
+		require("../../update")(done, true, () => {
+			expect(require("./loader.js!./a")).toBe(3);
+			done();
+		})
+	);
+});
+module.hot.accept("./loader.js!./a");

packages/rspack-test-tools/tests/hotCases/loader/import-module-2/loader.js

@@ -0,0 +1,4 @@
+module.exports = async function (content) {
+	let res = await this.importModule("./import_module_root.js");
+	return content.replace("1", res);
+};