From a5f739f6cb301daa526272f129dc5bfe39432eb8 Mon Sep 17 00:00:00 2001 From: jinrui Date: Mon, 3 Jul 2023 19:46:01 +0800 Subject: [PATCH] fix: output.clean can not remove *.hot-update.js (#3689) fix: output.clean can not remove hot-update.js --- crates/node_binding/binding.d.ts | 1 + crates/rspack_core/src/compiler/mod.rs | 31 ++++++------- crates/rspack_fs/src/async.rs | 3 ++ crates/rspack_fs_node/src/lib.rs | 10 +++++ crates/rspack_fs_node/src/node.rs | 1 + .../cases/hooks/asset-emitted/index.test.ts | 44 ++++++++++++++++++- .../hooks/asset-emitted/rspack.config.js | 1 + packages/playground/fixtures/rspack.ts | 2 +- 8 files changed, 76 insertions(+), 17 deletions(-) diff --git a/crates/node_binding/binding.d.ts b/crates/node_binding/binding.d.ts index 586e6dccbb8..c4caae6eca7 100644 --- a/crates/node_binding/binding.d.ts +++ b/crates/node_binding/binding.d.ts @@ -373,6 +373,7 @@ export interface JsStatsWarning { export interface NodeFS { writeFile: (...args: any[]) => any + removeFile: (...args: any[]) => any mkdir: (...args: any[]) => any mkdirp: (...args: any[]) => any } diff --git a/crates/rspack_core/src/compiler/mod.rs b/crates/rspack_core/src/compiler/mod.rs index 30ae8ad160c..1c1b269542d 100644 --- a/crates/rspack_core/src/compiler/mod.rs +++ b/crates/rspack_core/src/compiler/mod.rs @@ -7,7 +7,6 @@ mod resolver; use std::{path::Path, sync::Arc}; pub use compilation::*; -use dashmap::DashMap; pub use make::MakeParam; pub use queue::*; pub use resolver::*; @@ -15,6 +14,7 @@ use rspack_error::Result; use rspack_fs::AsyncWritableFileSystem; use rspack_futures::FuturesResults; use rspack_identifier::IdentifierSet; +use rustc_hash::FxHashMap as HashMap; use tracing::instrument; use crate::{ @@ -34,8 +34,8 @@ where pub resolver_factory: Arc, pub cache: Arc, /// emitted asset versions - /// the key of DashMap is filename, the value of DashMap is version - pub emitted_asset_versions: DashMap, + /// the key of HashMap is filename, the value of HashMap is version + pub emitted_asset_versions: HashMap, } impl Compiler @@ -210,11 +210,10 @@ where let _ = self .emitted_asset_versions .iter() - .filter_map(|item| { - let filename = item.key(); + .filter_map(|(filename, _version)| { if !assets.contains_key(filename) { - self.emitted_asset_versions.remove(filename); - Some(self.output_filesystem.remove_file(filename)) + let file_path = Path::new(&self.options.output.path).join(filename); + Some(self.output_filesystem.remove_file(file_path)) } else { None } @@ -225,19 +224,28 @@ where self.plugin_driver.emit(&mut self.compilation).await?; + let mut new_emitted_asset_versions = HashMap::default(); let results = self .compilation .assets() .iter() .filter_map(|(filename, asset)| { + // collect version info to new_emitted_asset_versions + if self.options.is_incremental_rebuild_emit_asset_enabled() { + new_emitted_asset_versions.insert(filename.to_string(), asset.info.version.clone()); + } + if let Some(old_version) = self.emitted_asset_versions.get(filename) { - if old_version.as_str() == asset.info.version { + if old_version.as_str() == asset.info.version && !old_version.is_empty() { return None; } } + Some(self.emit_asset(&self.options.output.path, filename, asset)) }) .collect::>(); + + self.emitted_asset_versions = new_emitted_asset_versions; // return first error for item in results.into_inner() { item?; @@ -271,13 +279,6 @@ where .write(&file_path, source.buffer()) .await?; - if !asset.info.version.is_empty() && self.options.is_incremental_rebuild_emit_asset_enabled() - { - self - .emitted_asset_versions - .insert(filename.to_string(), asset.info.version.clone()); - } - self.compilation.emitted_assets.insert(filename.to_string()); let asset_emitted_args = AssetEmittedArgs { diff --git a/crates/rspack_fs/src/async.rs b/crates/rspack_fs/src/async.rs index 7fabdfe962e..d4f62bcb978 100644 --- a/crates/rspack_fs/src/async.rs +++ b/crates/rspack_fs/src/async.rs @@ -24,7 +24,10 @@ pub trait AsyncWritableFileSystem { /// This function will create a file if it does not exist, and will entirely replace its contents if it does. fn write, D: AsRef<[u8]>>(&self, file: P, data: D) -> BoxFuture<'_, Result<()>>; + /// Removes a file from the filesystem. fn remove_file>(&self, file: P) -> BoxFuture<'_, Result<()>>; + + /// Removes a directory at this path, after removing all its contents. Use carefully. fn remove_dir_all>(&self, dir: P) -> BoxFuture<'_, Result<()>>; } diff --git a/crates/rspack_fs_node/src/lib.rs b/crates/rspack_fs_node/src/lib.rs index f70a2c22325..2b688a4e2c8 100644 --- a/crates/rspack_fs_node/src/lib.rs +++ b/crates/rspack_fs_node/src/lib.rs @@ -84,6 +84,16 @@ mod node_test { pub async fn mkdirp(&self, file: String) { self.writable_fs.create_dir_all(file).await.unwrap(); } + + #[napi] + pub async fn remove_file(&self, file: String) { + self.writable_fs.remove_file(file).await.unwrap(); + } + + #[napi] + pub async fn remove_dir_all(&self, file: String) { + self.writable_fs.remove_dir_all(file).await.unwrap(); + } } } diff --git a/crates/rspack_fs_node/src/node.rs b/crates/rspack_fs_node/src/node.rs index 441bccaae97..942099fed9b 100644 --- a/crates/rspack_fs_node/src/node.rs +++ b/crates/rspack_fs_node/src/node.rs @@ -30,6 +30,7 @@ impl Drop for JsFunctionRef { #[napi(object, js_name = "NodeFS")] pub struct NodeFS { pub write_file: JsFunction, + pub remove_file: JsFunction, pub mkdir: JsFunction, pub mkdirp: JsFunction, } diff --git a/packages/playground/cases/hooks/asset-emitted/index.test.ts b/packages/playground/cases/hooks/asset-emitted/index.test.ts index 0dc649bd0ea..c1a064d80a5 100644 --- a/packages/playground/cases/hooks/asset-emitted/index.test.ts +++ b/packages/playground/cases/hooks/asset-emitted/index.test.ts @@ -37,5 +37,47 @@ test("asset emitted hook should only emit modified assets", async ({ return text === "__OTHER_TEXT____VALUE__"; }); // main.js contains runtime module, so it should also emit - expect(assets).toEqual(["src_foo_js.js", "main.js"]); + expect(assets.sort()).toEqual(["main.js", "src_foo_js.js"]); + + // check dist dir + // the outputFileSystem can contain only one main hot-update.js + const files = rspack.compiler.outputFileSystem.readdirSync( + "dist", + {} + ) as string[]; + expect( + files.filter(item => /^main(.+)\.hot-update\.js$/.test(item)).length + ).toBe(1); +}); + +test("asset emitted should not emit removed assets", async ({ + page, + rspack, + fileAction +}) => { + let assets: string[] = []; + rspack.compiler.hooks.assetEmitted.tap("test", function (name) { + if (name.includes(".hot-update.")) { + return; + } + assets.push(name); + }); + + expect(await page.textContent("#root")).toBe("__ROOT_TEXT____FOO_VALUE__"); + // update js file + fileAction.updateFile("src/index.js", () => { + return 'document.getElementById("root").innerText = "__ROOT_TEXT__"'; + }); + await rspack.waitingForHmr(async () => { + const text = await page.textContent("#root"); + return text === "__ROOT_TEXT__"; + }); + expect(assets).toEqual(["main.js"]); + + // check dist dir + const files = rspack.compiler.outputFileSystem.readdirSync( + "dist", + {} + ) as string[]; + expect(files.every(item => item !== "src_foo_js.js")).toBeTruthy(); }); diff --git a/packages/playground/cases/hooks/asset-emitted/rspack.config.js b/packages/playground/cases/hooks/asset-emitted/rspack.config.js index 3adff2d1122..2cd065d42b3 100644 --- a/packages/playground/cases/hooks/asset-emitted/rspack.config.js +++ b/packages/playground/cases/hooks/asset-emitted/rspack.config.js @@ -9,6 +9,7 @@ module.exports = { } ] }, + output: { clean: true }, experiments: { incrementalRebuild: { emitAsset: true diff --git a/packages/playground/fixtures/rspack.ts b/packages/playground/fixtures/rspack.ts index 7515dec5536..5098b5fd90a 100644 --- a/packages/playground/fixtures/rspack.ts +++ b/packages/playground/fixtures/rspack.ts @@ -56,7 +56,7 @@ class Rspack { if (tries === maxTries - 1) { throw new Error("outof max retry time"); } - await sleep(100); + await sleep(200); } } }