From 16520be66629e2506bc6c08396126ef486b41cfb Mon Sep 17 00:00:00 2001 From: pshu Date: Tue, 7 Apr 2026 16:50:50 +0800 Subject: [PATCH 1/5] feat: implement compilation.hooks.shouldRecord and integrate with NoEmitOnErrorsPlugin Align with webpack's `shouldRecord` hook behavior so that `NoEmitOnErrorsPlugin` (`emitOnErrors: false`) skips recording compilation state when errors are present. This ensures the next HMR rebuild compares against the last successful compilation rather than the errored one, fixing broken HMR recovery after compilation errors. Closes: https://github.com/DigitecGalaxus/next-yak/pull/508 --- crates/rspack_core/src/compilation/mod.rs | 6 +++-- crates/rspack_core/src/compiler/mod.rs | 2 ++ crates/rspack_core/src/compiler/rebuild.rs | 22 ++++++++++++--- crates/rspack_plugin_hmr/src/lib.rs | 14 +++++----- .../src/lib.rs | 15 ++++++++++- packages/rspack/src/Compilation.ts | 2 ++ .../a.js | 8 ++++++ .../errors1.js | 3 +++ .../errors2.js | 3 +++ .../index.js | 27 +++++++++++++++++++ .../rspack.config.js | 6 +++++ .../recover/no-emit-on-errors-recover/a.js | 6 +++++ .../no-emit-on-errors-recover/errors1.js | 3 +++ .../no-emit-on-errors-recover/index.js | 19 +++++++++++++ .../rspack.config.js | 6 +++++ 15 files changed, 129 insertions(+), 13 deletions(-) create mode 100644 tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/a.js create mode 100644 tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/errors1.js create mode 100644 tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/errors2.js create mode 100644 tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/index.js create mode 100644 tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/rspack.config.js create mode 100644 tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/a.js create mode 100644 tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/errors1.js create mode 100644 tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/index.js create mode 100644 tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/rspack.config.js diff --git a/crates/rspack_core/src/compilation/mod.rs b/crates/rspack_core/src/compilation/mod.rs index 930f321ebaf2..00c705a1fb03 100644 --- a/crates/rspack_core/src/compilation/mod.rs +++ b/crates/rspack_core/src/compilation/mod.rs @@ -127,6 +127,7 @@ define_hook!(CompilationRenderManifest: Series(compilation: &Compilation, chunk_ define_hook!(CompilationChunkAsset: Series(compilation: &Compilation, chunk_ukey: &ChunkUkey, filename: &str)); define_hook!(CompilationProcessAssets: Series(compilation: &mut Compilation)); define_hook!(CompilationAfterProcessAssets: Series(compilation: &Compilation, diagnostics: &mut Vec)); +define_hook!(CompilationShouldRecord: SeriesBail(compilation: &Compilation) -> bool); define_hook!(CompilationAfterSeal: Series(compilation: &Compilation),tracing=true); #[derive(Debug, Default)] @@ -166,6 +167,7 @@ pub struct CompilationHooks { pub chunk_asset: CompilationChunkAssetHook, pub process_assets: CompilationProcessAssetsHook, pub after_process_assets: CompilationAfterProcessAssetsHook, + pub should_record: CompilationShouldRecordHook, pub after_seal: CompilationAfterSealHook, } @@ -201,7 +203,7 @@ pub struct Compilation { // The status is different, should generate different hash for `.hot-update.js` // So use compilation hash update `hot_index` to fix it. pub hot_index: u32, - pub records: Option, + pub records: Option>, pub options: Arc, pub platform: Arc, pub entries: Entry, @@ -325,7 +327,7 @@ impl Compilation { buildtime_plugin_driver: SharedPluginDriver, resolver_factory: Arc, loader_resolver_factory: Arc, - records: Option, + records: Option>, incremental: Incremental, module_executor: Option, modified_files: ArcPathSet, diff --git a/crates/rspack_core/src/compiler/mod.rs b/crates/rspack_core/src/compiler/mod.rs index bbf7b74c15a1..89ccdf796e00 100644 --- a/crates/rspack_core/src/compiler/mod.rs +++ b/crates/rspack_core/src/compiler/mod.rs @@ -99,6 +99,7 @@ pub struct Compiler { pub emitted_asset_versions: HashMap, pub platform: Arc, compiler_context: Arc, + last_records: Option>, } impl Compiler { @@ -195,6 +196,7 @@ impl Compiler { input_filesystem, platform, compiler_context, + last_records: None, } } diff --git a/crates/rspack_core/src/compiler/rebuild.rs b/crates/rspack_core/src/compiler/rebuild.rs index b2c02f7ef166..3e462ab15525 100644 --- a/crates/rspack_core/src/compiler/rebuild.rs +++ b/crates/rspack_core/src/compiler/rebuild.rs @@ -1,4 +1,4 @@ -use std::path::Path; +use std::{path::Path, sync::Arc}; use rspack_collections::IdentifierMap; use rspack_error::Result; @@ -57,7 +57,23 @@ impl Compiler { changed_files: FxHashSet, deleted_files: FxHashSet, ) -> Result<()> { - let records = CompilationRecords::record(&self.compilation); + let should_record = !matches!( + self + .plugin_driver + .compilation_hooks + .should_record + .call(&self.compilation) + .await?, + Some(false) + ); + + let records = if should_record { + let records = Arc::new(CompilationRecords::record(&self.compilation)); + self.last_records = Some(Arc::clone(&records)); + Some(records) + } else { + self.last_records.clone() + }; // build without stats { @@ -79,7 +95,7 @@ impl Compiler { self.buildtime_plugin_driver.clone(), self.resolver_factory.clone(), self.loader_resolver_factory.clone(), - Some(records), + records, Incremental::new_hot(self.options.incremental), Some(ModuleExecutor::default()), modified_files, diff --git a/crates/rspack_plugin_hmr/src/lib.rs b/crates/rspack_plugin_hmr/src/lib.rs index 8424f0744cf1..38a9b6b9cf87 100644 --- a/crates/rspack_plugin_hmr/src/lib.rs +++ b/crates/rspack_plugin_hmr/src/lib.rs @@ -56,16 +56,16 @@ async fn compilation( #[plugin_hook(CompilationProcessAssets for HotModuleReplacementPlugin, stage = Compilation::PROCESS_ASSETS_STAGE_ADDITIONAL)] async fn process_assets(&self, compilation: &mut Compilation) -> Result<()> { - let Some(CompilationRecords { + let Some(records) = compilation.records.take() else { + return Ok(()); + }; + let CompilationRecords { chunks: old_chunks, runtimes: all_old_runtime, modules: old_all_modules, runtime_modules: old_runtime_modules, hash: old_hash, - }) = compilation.records.take() - else { - return Ok(()); - }; + } = records.as_ref(); if let Some(old_hash) = &old_hash && let Some(hash) = &compilation.hash @@ -85,7 +85,7 @@ async fn process_assets(&self, compilation: &mut Compilation) -> Result<()> { let mut updated_runtime_modules: IdentifierSet = Default::default(); let mut updated_chunks: HashMap> = Default::default(); - for (identifier, old_runtime_module_hash) in &old_runtime_modules { + for (identifier, old_runtime_module_hash) in old_runtime_modules { if let Some(new_runtime_module_hash) = compilation.runtime_modules_hash.get(identifier) { // updated if new_runtime_module_hash != old_runtime_module_hash { @@ -107,7 +107,7 @@ async fn process_assets(&self, compilation: &mut Compilation) -> Result<()> { .collect(); let mut completely_removed_modules: HashSet = Default::default(); - for (chunk_id, (old_runtime, old_module_ids)) in &old_chunks { + for (chunk_id, (old_runtime, old_module_ids)) in old_chunks { let mut remaining_modules: HashSet = Default::default(); for old_module_id in old_module_ids { if !all_module_ids.contains_key(old_module_id) { diff --git a/crates/rspack_plugin_no_emit_on_errors/src/lib.rs b/crates/rspack_plugin_no_emit_on_errors/src/lib.rs index 23f745b4b5d1..9d3f5d6c3bb9 100644 --- a/crates/rspack_plugin_no_emit_on_errors/src/lib.rs +++ b/crates/rspack_plugin_no_emit_on_errors/src/lib.rs @@ -1,6 +1,6 @@ use std::fmt::Debug; -use rspack_core::{Compilation, CompilerShouldEmit, Plugin}; +use rspack_core::{Compilation, CompilationShouldRecord, CompilerShouldEmit, Plugin}; use rspack_error::Result; use rspack_hook::{plugin, plugin_hook}; @@ -23,6 +23,15 @@ async fn should_emit(&self, compilation: &mut Compilation) -> Result Result> { + if compilation.get_errors().next().is_some() { + Ok(Some(false)) + } else { + Ok(None) + } +} + impl Plugin for NoEmitOnErrorsPlugin { fn name(&self) -> &'static str { "NoEmitOnErrorsPlugin" @@ -30,6 +39,10 @@ impl Plugin for NoEmitOnErrorsPlugin { fn apply(&self, ctx: &mut rspack_core::ApplyContext<'_>) -> Result<()> { ctx.compiler_hooks.should_emit.tap(should_emit::new(self)); + ctx + .compilation_hooks + .should_record + .tap(should_record::new(self)); Ok(()) } } diff --git a/packages/rspack/src/Compilation.ts b/packages/rspack/src/Compilation.ts index 0b290c8ebee5..f3858afa3d08 100644 --- a/packages/rspack/src/Compilation.ts +++ b/packages/rspack/src/Compilation.ts @@ -255,6 +255,7 @@ export class Compilation { runtimeModule: liteTapable.SyncHook<[RuntimeModule, Chunk]>; seal: liteTapable.SyncHook<[]>; afterSeal: liteTapable.AsyncSeriesHook<[], void>; + shouldRecord: liteTapable.SyncBailHook<[], boolean>; needAdditionalPass: liteTapable.SyncBailHook<[], boolean>; }>; name?: string; @@ -389,6 +390,7 @@ BREAKING CHANGE: Asset processing hooks in Compilation has been merged into a si runtimeModule: new liteTapable.SyncHook(['module', 'chunk']), seal: new liteTapable.SyncHook([]), afterSeal: new liteTapable.AsyncSeriesHook([]), + shouldRecord: new liteTapable.SyncBailHook([]), needAdditionalPass: new liteTapable.SyncBailHook([]), }; diff --git a/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/a.js b/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/a.js new file mode 100644 index 000000000000..d05ace5be990 --- /dev/null +++ b/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/a.js @@ -0,0 +1,8 @@ +export default 1; +--- +]}); +export default 2; +--- +export default <<<<<<; +--- +export default 4; diff --git a/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/errors1.js b/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/errors1.js new file mode 100644 index 000000000000..c4950058484b --- /dev/null +++ b/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/errors1.js @@ -0,0 +1,3 @@ +module.exports = [ + [/Module parse failed/] +] diff --git a/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/errors2.js b/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/errors2.js new file mode 100644 index 000000000000..c4950058484b --- /dev/null +++ b/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/errors2.js @@ -0,0 +1,3 @@ +module.exports = [ + [/Module parse failed/] +] diff --git a/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/index.js b/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/index.js new file mode 100644 index 000000000000..1e037d0c8745 --- /dev/null +++ b/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/index.js @@ -0,0 +1,27 @@ +import a from "./a"; + +it("should recover after consecutive errors with no-emit-on-errors", async () => { + expect(a).toBe(1); + + // Step 1: syntax error - emitOnErrors: false prevents emit and record + try { + await NEXT_HMR(); + } catch (e) { + // Expected: no update available because emit was prevented + } + expect(a).toBe(1); + + // Step 2: another syntax error - still no emit and no record + try { + await NEXT_HMR(); + } catch (e) { + // Expected: no update available because emit was prevented + } + expect(a).toBe(1); + + // Step 3: fix - HMR should compare against step 0 (last good compilation) + await NEXT_HMR(); + expect(a).toBe(4); +}); + +module.hot.accept("./a"); diff --git a/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/rspack.config.js b/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/rspack.config.js new file mode 100644 index 000000000000..cac743954215 --- /dev/null +++ b/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/rspack.config.js @@ -0,0 +1,6 @@ +/** @type {import("@rspack/core").Configuration} */ +module.exports = { + optimization: { + emitOnErrors: false, + }, +}; diff --git a/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/a.js b/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/a.js new file mode 100644 index 000000000000..bc111d3376cc --- /dev/null +++ b/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/a.js @@ -0,0 +1,6 @@ +export default 1; +--- +]}); +export default 2; +--- +export default 3; diff --git a/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/errors1.js b/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/errors1.js new file mode 100644 index 000000000000..c4950058484b --- /dev/null +++ b/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/errors1.js @@ -0,0 +1,3 @@ +module.exports = [ + [/Module parse failed/] +] diff --git a/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/index.js b/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/index.js new file mode 100644 index 000000000000..cf9c32f614f8 --- /dev/null +++ b/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/index.js @@ -0,0 +1,19 @@ +import a from "./a"; + +it("should recover after error with no-emit-on-errors", async () => { + expect(a).toBe(1); + + // Step 1: syntax error - emitOnErrors: false prevents emit and record + try { + await NEXT_HMR(); + } catch (e) { + // Expected: no update available because emit was prevented + } + expect(a).toBe(1); + + // Step 2: fix - HMR should compare against step 0 (last good compilation) + await NEXT_HMR(); + expect(a).toBe(3); +}); + +module.hot.accept("./a"); diff --git a/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/rspack.config.js b/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/rspack.config.js new file mode 100644 index 000000000000..cac743954215 --- /dev/null +++ b/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/rspack.config.js @@ -0,0 +1,6 @@ +/** @type {import("@rspack/core").Configuration} */ +module.exports = { + optimization: { + emitOnErrors: false, + }, +}; From d1e11d6458880eab4cfbd6d90305c5b1e217a726 Mon Sep 17 00:00:00 2001 From: pshu Date: Tue, 7 Apr 2026 18:07:16 +0800 Subject: [PATCH 2/5] feat: register shouldRecord JS binding for compilation hook Wire up CompilationShouldRecord through the napi binding layer so that JS plugins tapping compilation.hooks.shouldRecord are observed by Rust. --- crates/node_binding/napi-binding.d.ts | 52 ++++++++++--------- .../src/plugins/interceptor.rs | 36 ++++++++++--- .../src/plugins/js_hooks_plugin.rs | 10 ++++ packages/rspack/src/taps/compilation.ts | 19 +++++++ 4 files changed, 86 insertions(+), 31 deletions(-) diff --git a/crates/node_binding/napi-binding.d.ts b/crates/node_binding/napi-binding.d.ts index 2d58627286b4..e32ad3e9f87c 100644 --- a/crates/node_binding/napi-binding.d.ts +++ b/crates/node_binding/napi-binding.d.ts @@ -3110,31 +3110,32 @@ export declare enum RegisterJsTapKind { CompilationProcessAssets = 23, CompilationAfterProcessAssets = 24, CompilationSeal = 25, - CompilationAfterSeal = 26, - NormalModuleFactoryBeforeResolve = 27, - NormalModuleFactoryFactorize = 28, - NormalModuleFactoryResolve = 29, - NormalModuleFactoryAfterResolve = 30, - NormalModuleFactoryCreateModule = 31, - NormalModuleFactoryResolveForScheme = 32, - ContextModuleFactoryBeforeResolve = 33, - ContextModuleFactoryAfterResolve = 34, - JavascriptModulesChunkHash = 35, - HtmlPluginBeforeAssetTagGeneration = 36, - HtmlPluginAlterAssetTags = 37, - HtmlPluginAlterAssetTagGroups = 38, - HtmlPluginAfterTemplateExecution = 39, - HtmlPluginBeforeEmit = 40, - HtmlPluginAfterEmit = 41, - RuntimePluginCreateScript = 42, - RuntimePluginCreateLink = 43, - RuntimePluginLinkPreload = 44, - RuntimePluginLinkPrefetch = 45, - RsdoctorPluginModuleGraph = 46, - RsdoctorPluginChunkGraph = 47, - RsdoctorPluginModuleIds = 48, - RsdoctorPluginModuleSources = 49, - RsdoctorPluginAssets = 50 + CompilationShouldRecord = 26, + CompilationAfterSeal = 27, + NormalModuleFactoryBeforeResolve = 28, + NormalModuleFactoryFactorize = 29, + NormalModuleFactoryResolve = 30, + NormalModuleFactoryAfterResolve = 31, + NormalModuleFactoryCreateModule = 32, + NormalModuleFactoryResolveForScheme = 33, + ContextModuleFactoryBeforeResolve = 34, + ContextModuleFactoryAfterResolve = 35, + JavascriptModulesChunkHash = 36, + HtmlPluginBeforeAssetTagGeneration = 37, + HtmlPluginAlterAssetTags = 38, + HtmlPluginAlterAssetTagGroups = 39, + HtmlPluginAfterTemplateExecution = 40, + HtmlPluginBeforeEmit = 41, + HtmlPluginAfterEmit = 42, + RuntimePluginCreateScript = 43, + RuntimePluginCreateLink = 44, + RuntimePluginLinkPreload = 45, + RuntimePluginLinkPrefetch = 46, + RsdoctorPluginModuleGraph = 47, + RsdoctorPluginChunkGraph = 48, + RsdoctorPluginModuleIds = 49, + RsdoctorPluginModuleSources = 50, + RsdoctorPluginAssets = 51 } export interface RegisterJsTaps { @@ -3164,6 +3165,7 @@ export interface RegisterJsTaps { registerCompilationProcessAssetsTaps: (stages: Array) => Array<{ function: ((arg: JsCompilation) => Promise); stage: number; }> registerCompilationAfterProcessAssetsTaps: (stages: Array) => Array<{ function: ((arg: JsCompilation) => void); stage: number; }> registerCompilationSealTaps: (stages: Array) => Array<{ function: (() => void); stage: number; }> + registerCompilationShouldRecordTaps: (stages: Array) => Array<{ function: (() => boolean | undefined); stage: number; }> registerCompilationAfterSealTaps: (stages: Array) => Array<{ function: (() => Promise); stage: number; }> registerNormalModuleFactoryBeforeResolveTaps: (stages: Array) => Array<{ function: ((arg: JsResolveData) => Promise<[boolean | undefined, JsResolveData]>); stage: number; }> registerNormalModuleFactoryFactorizeTaps: (stages: Array) => Array<{ function: ((arg: JsResolveData) => Promise); stage: number; }> diff --git a/crates/rspack_binding_api/src/plugins/interceptor.rs b/crates/rspack_binding_api/src/plugins/interceptor.rs index 3ccfb979c1c6..e31bde5438e1 100644 --- a/crates/rspack_binding_api/src/plugins/interceptor.rs +++ b/crates/rspack_binding_api/src/plugins/interceptor.rs @@ -27,12 +27,13 @@ use rspack_core::{ CompilationOptimizeTreeHook, CompilationParams, CompilationProcessAssets, CompilationProcessAssetsHook, CompilationRuntimeModule, CompilationRuntimeModuleHook, CompilationRuntimeRequirementInTree, CompilationRuntimeRequirementInTreeHook, CompilationSeal, - CompilationSealHook, CompilationStillValidModule, CompilationStillValidModuleHook, - CompilationSucceedModule, CompilationSucceedModuleHook, CompilerAfterEmit, CompilerAfterEmitHook, - CompilerAssetEmitted, CompilerAssetEmittedHook, CompilerCompilation, CompilerCompilationHook, - CompilerEmit, CompilerEmitHook, CompilerFinishMake, CompilerFinishMakeHook, CompilerId, - CompilerMake, CompilerMakeHook, CompilerShouldEmit, CompilerShouldEmitHook, - CompilerThisCompilation, CompilerThisCompilationHook, ContextModuleFactoryAfterResolve, + CompilationSealHook, CompilationShouldRecord, CompilationShouldRecordHook, + CompilationStillValidModule, CompilationStillValidModuleHook, CompilationSucceedModule, + CompilationSucceedModuleHook, CompilerAfterEmit, CompilerAfterEmitHook, CompilerAssetEmitted, + CompilerAssetEmittedHook, CompilerCompilation, CompilerCompilationHook, CompilerEmit, + CompilerEmitHook, CompilerFinishMake, CompilerFinishMakeHook, CompilerId, CompilerMake, + CompilerMakeHook, CompilerShouldEmit, CompilerShouldEmitHook, CompilerThisCompilation, + CompilerThisCompilationHook, ContextModuleFactoryAfterResolve, ContextModuleFactoryAfterResolveHook, ContextModuleFactoryBeforeResolve, ContextModuleFactoryBeforeResolveHook, ExecuteModuleId, Module, ModuleFactoryCreateData, ModuleId, ModuleIdentifier, ModuleIdsArtifact, NormalModuleCreateData, @@ -385,6 +386,7 @@ pub enum RegisterJsTapKind { CompilationProcessAssets, CompilationAfterProcessAssets, CompilationSeal, + CompilationShouldRecord, CompilationAfterSeal, NormalModuleFactoryBeforeResolve, NormalModuleFactoryFactorize, @@ -536,6 +538,10 @@ pub struct RegisterJsTaps { pub register_compilation_after_process_assets_taps: RegisterFunction, #[napi(ts_type = "(stages: Array) => Array<{ function: (() => void); stage: number; }>")] pub register_compilation_seal_taps: RegisterFunction<(), ()>, + #[napi( + ts_type = "(stages: Array) => Array<{ function: (() => boolean | undefined); stage: number; }>" + )] + pub register_compilation_should_record_taps: RegisterFunction<(), Option>, #[napi( ts_type = "(stages: Array) => Array<{ function: (() => Promise); stage: number; }>" )] @@ -852,6 +858,13 @@ define_register!( kind = RegisterJsTapKind::CompilationSeal, skip = true, ); +define_register!( + RegisterCompilationShouldRecordTaps, + tap = CompilationShouldRecordTap<(), Option> @ CompilationShouldRecordHook, + cache = false, + kind = RegisterJsTapKind::CompilationShouldRecord, + skip = true, +); define_register!( RegisterCompilationAfterSealTaps, tap = CompilationAfterSealTap<(), Promise<()>> @ CompilationAfterSealHook, @@ -1565,6 +1578,17 @@ impl CompilationSeal for CompilationSealTap { } } +#[async_trait] +impl CompilationShouldRecord for CompilationShouldRecordTap { + async fn run(&self, _compilation: &Compilation) -> rspack_error::Result> { + self.function.call_with_sync(()).await + } + + fn stage(&self) -> i32 { + self.stage + } +} + #[async_trait] impl CompilationAfterSeal for CompilationAfterSealTap { async fn run(&self, _compilation: &Compilation) -> rspack_error::Result<()> { diff --git a/crates/rspack_binding_api/src/plugins/js_hooks_plugin.rs b/crates/rspack_binding_api/src/plugins/js_hooks_plugin.rs index 229761006322..e583cfc6a701 100644 --- a/crates/rspack_binding_api/src/plugins/js_hooks_plugin.rs +++ b/crates/rspack_binding_api/src/plugins/js_hooks_plugin.rs @@ -46,6 +46,7 @@ pub struct JsHooksAdapterPlugin { register_compilation_process_assets_taps: RegisterCompilationProcessAssetsTaps, register_compilation_after_process_assets_taps: RegisterCompilationAfterProcessAssetsTaps, register_compilation_seal_taps: RegisterCompilationSealTaps, + register_compilation_should_record_taps: RegisterCompilationShouldRecordTaps, register_compilation_after_seal_taps: RegisterCompilationAfterSealTaps, register_normal_module_factory_before_resolve_taps: RegisterNormalModuleFactoryBeforeResolveTaps, register_normal_module_factory_factorize_taps: RegisterNormalModuleFactoryFactorizeTaps, @@ -200,6 +201,10 @@ impl Plugin for JsHooksAdapterPlugin { .compilation_hooks .seal .intercept(self.register_compilation_seal_taps.clone()); + ctx + .compilation_hooks + .should_record + .intercept(self.register_compilation_should_record_taps.clone()); ctx .compilation_hooks .after_seal @@ -313,6 +318,7 @@ impl Plugin for JsHooksAdapterPlugin { .register_compilation_after_process_assets_taps .clear_cache(); self.register_compilation_seal_taps.clear_cache(); + self.register_compilation_should_record_taps.clear_cache(); self.register_compilation_after_seal_taps.clear_cache(); self .register_normal_module_factory_before_resolve_taps @@ -589,6 +595,10 @@ impl JsHooksAdapterPlugin { register_js_taps.register_compilation_seal_taps, non_skippable_registers.clone(), ), + register_compilation_should_record_taps: RegisterCompilationShouldRecordTaps::new( + register_js_taps.register_compilation_should_record_taps, + non_skippable_registers.clone(), + ), register_compilation_after_seal_taps: RegisterCompilationAfterSealTaps::new( register_js_taps.register_compilation_after_seal_taps, non_skippable_registers.clone(), diff --git a/packages/rspack/src/taps/compilation.ts b/packages/rspack/src/taps/compilation.ts index e355d726fdab..bca3b5788912 100644 --- a/packages/rspack/src/taps/compilation.ts +++ b/packages/rspack/src/taps/compilation.ts @@ -1,4 +1,5 @@ import binding from '@rspack/binding'; +import { SyncBailHook } from '@rspack/lite-tapable'; import { tryRunOrWebpackError } from '../lib/HookWebpackError'; import type { Module } from '../Module'; import { @@ -500,6 +501,24 @@ export const createCompilationHooksRegisters: CreatePartialRegisters< }; }, ), + registerCompilationShouldRecordTaps: createTap( + binding.RegisterJsTapKind.CompilationShouldRecord, + + function () { + // shouldRecord is called in rebuild_inner on the old compilation + // before the new compilation is created, so guard against undefined. + return ( + getCompiler().__internal__get_compilation()?.hooks.shouldRecord ?? + new SyncBailHook<[], boolean>([]) + ); + }, + + function (queried) { + return function () { + return queried.call(); + }; + }, + ), registerCompilationAfterSealTaps: createTap( binding.RegisterJsTapKind.CompilationAfterSeal, From 04fb30168cf7f68f357afe7340aa9c9ce7807709 Mon Sep 17 00:00:00 2001 From: pshu Date: Tue, 7 Apr 2026 23:40:02 +0800 Subject: [PATCH 3/5] test: convert no-emit-on-errors HMR tests from hotCases to e2e Replace hotCases unit tests with Playwright e2e tests that verify the full dev-server HMR recovery flow with emitOnErrors: false, including error overlay visibility and page content after recovery. --- .../index.test.ts | 42 +++++++++++++++++++ .../rspack.config.js | 15 +++++++ .../src/index.js | 8 ++++ .../no-emit-on-errors-recover/index.test.ts | 35 ++++++++++++++++ .../rspack.config.js | 15 +++++++ .../no-emit-on-errors-recover/src/index.js | 8 ++++ .../a.js | 8 ---- .../errors1.js | 3 -- .../errors2.js | 3 -- .../index.js | 27 ------------ .../rspack.config.js | 6 --- .../recover/no-emit-on-errors-recover/a.js | 6 --- .../no-emit-on-errors-recover/errors1.js | 3 -- .../no-emit-on-errors-recover/index.js | 19 --------- .../rspack.config.js | 6 --- 15 files changed, 123 insertions(+), 81 deletions(-) create mode 100644 tests/e2e/cases/hooks/no-emit-on-errors-recover-consecutive/index.test.ts create mode 100644 tests/e2e/cases/hooks/no-emit-on-errors-recover-consecutive/rspack.config.js create mode 100644 tests/e2e/cases/hooks/no-emit-on-errors-recover-consecutive/src/index.js create mode 100644 tests/e2e/cases/hooks/no-emit-on-errors-recover/index.test.ts create mode 100644 tests/e2e/cases/hooks/no-emit-on-errors-recover/rspack.config.js create mode 100644 tests/e2e/cases/hooks/no-emit-on-errors-recover/src/index.js delete mode 100644 tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/a.js delete mode 100644 tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/errors1.js delete mode 100644 tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/errors2.js delete mode 100644 tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/index.js delete mode 100644 tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/rspack.config.js delete mode 100644 tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/a.js delete mode 100644 tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/errors1.js delete mode 100644 tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/index.js delete mode 100644 tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/rspack.config.js diff --git a/tests/e2e/cases/hooks/no-emit-on-errors-recover-consecutive/index.test.ts b/tests/e2e/cases/hooks/no-emit-on-errors-recover-consecutive/index.test.ts new file mode 100644 index 000000000000..ac093707ae8a --- /dev/null +++ b/tests/e2e/cases/hooks/no-emit-on-errors-recover-consecutive/index.test.ts @@ -0,0 +1,42 @@ +import { expect, test } from '@/fixtures'; + +test('should recover after consecutive errors with emitOnErrors: false', async ({ + page, + fileAction, + rspack, +}) => { + // Step 0: initial build succeeds + await expect(page.getByText('value:1')).toBeVisible(); + + // Step 1: introduce syntax error + fileAction.updateFile('src/index.js', () => ']});\nexport default 2;'); + await rspack.waitingForBuild(); + await expect(page.locator('#rspack-dev-server-client-overlay')).toHaveCount( + 1, + ); + await expect(page.getByText('value:1')).toBeVisible(); + + // Step 2: introduce a different syntax error + fileAction.updateFile('src/index.js', () => 'export default <<<<<<;'); + await rspack.waitingForBuild(); + await expect(page.locator('#rspack-dev-server-client-overlay')).toHaveCount( + 1, + ); + await expect(page.getByText('value:1')).toBeVisible(); + + // Step 3: fix the error → HMR should compare against step 0 (last good compilation) + fileAction.updateFile('src/index.js', () => + [ + 'const div = document.getElementById("root") || document.createElement("div");', + 'div.id = "root";', + 'div.innerText = "value:4";', + 'document.body.appendChild(div);', + 'if (module.hot) { module.hot.accept(); }', + ].join('\n'), + ); + await rspack.waitingForBuild(); + await expect(page.locator('#rspack-dev-server-client-overlay')).toHaveCount( + 0, + ); + await expect(page.getByText('value:4')).toBeVisible(); +}); diff --git a/tests/e2e/cases/hooks/no-emit-on-errors-recover-consecutive/rspack.config.js b/tests/e2e/cases/hooks/no-emit-on-errors-recover-consecutive/rspack.config.js new file mode 100644 index 000000000000..fb08701ce58e --- /dev/null +++ b/tests/e2e/cases/hooks/no-emit-on-errors-recover-consecutive/rspack.config.js @@ -0,0 +1,15 @@ +const rspack = require('@rspack/core'); + +/** @type {import("@rspack/core").Configuration} */ +module.exports = { + entry: './src/index.js', + context: __dirname, + mode: 'development', + plugins: [new rspack.HtmlRspackPlugin()], + optimization: { + emitOnErrors: false, + }, + devServer: { + hot: true, + }, +}; diff --git a/tests/e2e/cases/hooks/no-emit-on-errors-recover-consecutive/src/index.js b/tests/e2e/cases/hooks/no-emit-on-errors-recover-consecutive/src/index.js new file mode 100644 index 000000000000..b239d59bea73 --- /dev/null +++ b/tests/e2e/cases/hooks/no-emit-on-errors-recover-consecutive/src/index.js @@ -0,0 +1,8 @@ +const div = document.createElement('div'); +div.id = 'root'; +div.innerText = 'value:1'; +document.body.appendChild(div); + +if (module.hot) { + module.hot.accept(); +} diff --git a/tests/e2e/cases/hooks/no-emit-on-errors-recover/index.test.ts b/tests/e2e/cases/hooks/no-emit-on-errors-recover/index.test.ts new file mode 100644 index 000000000000..d82b701add11 --- /dev/null +++ b/tests/e2e/cases/hooks/no-emit-on-errors-recover/index.test.ts @@ -0,0 +1,35 @@ +import { expect, test } from '@/fixtures'; + +test('should recover after error with emitOnErrors: false', async ({ + page, + fileAction, + rspack, +}) => { + // Step 0: initial build succeeds + await expect(page.getByText('value:1')).toBeVisible(); + + // Step 1: introduce syntax error → compilation fails, emitOnErrors: false prevents emit + fileAction.updateFile('src/index.js', () => ']});\nexport default 2;'); + await rspack.waitingForBuild(); + await expect(page.locator('#rspack-dev-server-client-overlay')).toHaveCount( + 1, + ); + // Page still shows old content since emit was prevented + await expect(page.getByText('value:1')).toBeVisible(); + + // Step 2: fix the error → HMR should compare against step 0 (last good compilation) + fileAction.updateFile('src/index.js', () => + [ + 'const div = document.getElementById("root") || document.createElement("div");', + 'div.id = "root";', + 'div.innerText = "value:3";', + 'document.body.appendChild(div);', + 'if (module.hot) { module.hot.accept(); }', + ].join('\n'), + ); + await rspack.waitingForBuild(); + await expect(page.locator('#rspack-dev-server-client-overlay')).toHaveCount( + 0, + ); + await expect(page.getByText('value:3')).toBeVisible(); +}); diff --git a/tests/e2e/cases/hooks/no-emit-on-errors-recover/rspack.config.js b/tests/e2e/cases/hooks/no-emit-on-errors-recover/rspack.config.js new file mode 100644 index 000000000000..fb08701ce58e --- /dev/null +++ b/tests/e2e/cases/hooks/no-emit-on-errors-recover/rspack.config.js @@ -0,0 +1,15 @@ +const rspack = require('@rspack/core'); + +/** @type {import("@rspack/core").Configuration} */ +module.exports = { + entry: './src/index.js', + context: __dirname, + mode: 'development', + plugins: [new rspack.HtmlRspackPlugin()], + optimization: { + emitOnErrors: false, + }, + devServer: { + hot: true, + }, +}; diff --git a/tests/e2e/cases/hooks/no-emit-on-errors-recover/src/index.js b/tests/e2e/cases/hooks/no-emit-on-errors-recover/src/index.js new file mode 100644 index 000000000000..b239d59bea73 --- /dev/null +++ b/tests/e2e/cases/hooks/no-emit-on-errors-recover/src/index.js @@ -0,0 +1,8 @@ +const div = document.createElement('div'); +div.id = 'root'; +div.innerText = 'value:1'; +document.body.appendChild(div); + +if (module.hot) { + module.hot.accept(); +} diff --git a/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/a.js b/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/a.js deleted file mode 100644 index d05ace5be990..000000000000 --- a/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/a.js +++ /dev/null @@ -1,8 +0,0 @@ -export default 1; ---- -]}); -export default 2; ---- -export default <<<<<<; ---- -export default 4; diff --git a/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/errors1.js b/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/errors1.js deleted file mode 100644 index c4950058484b..000000000000 --- a/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/errors1.js +++ /dev/null @@ -1,3 +0,0 @@ -module.exports = [ - [/Module parse failed/] -] diff --git a/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/errors2.js b/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/errors2.js deleted file mode 100644 index c4950058484b..000000000000 --- a/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/errors2.js +++ /dev/null @@ -1,3 +0,0 @@ -module.exports = [ - [/Module parse failed/] -] diff --git a/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/index.js b/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/index.js deleted file mode 100644 index 1e037d0c8745..000000000000 --- a/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/index.js +++ /dev/null @@ -1,27 +0,0 @@ -import a from "./a"; - -it("should recover after consecutive errors with no-emit-on-errors", async () => { - expect(a).toBe(1); - - // Step 1: syntax error - emitOnErrors: false prevents emit and record - try { - await NEXT_HMR(); - } catch (e) { - // Expected: no update available because emit was prevented - } - expect(a).toBe(1); - - // Step 2: another syntax error - still no emit and no record - try { - await NEXT_HMR(); - } catch (e) { - // Expected: no update available because emit was prevented - } - expect(a).toBe(1); - - // Step 3: fix - HMR should compare against step 0 (last good compilation) - await NEXT_HMR(); - expect(a).toBe(4); -}); - -module.hot.accept("./a"); diff --git a/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/rspack.config.js b/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/rspack.config.js deleted file mode 100644 index cac743954215..000000000000 --- a/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover-consecutive/rspack.config.js +++ /dev/null @@ -1,6 +0,0 @@ -/** @type {import("@rspack/core").Configuration} */ -module.exports = { - optimization: { - emitOnErrors: false, - }, -}; diff --git a/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/a.js b/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/a.js deleted file mode 100644 index bc111d3376cc..000000000000 --- a/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/a.js +++ /dev/null @@ -1,6 +0,0 @@ -export default 1; ---- -]}); -export default 2; ---- -export default 3; diff --git a/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/errors1.js b/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/errors1.js deleted file mode 100644 index c4950058484b..000000000000 --- a/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/errors1.js +++ /dev/null @@ -1,3 +0,0 @@ -module.exports = [ - [/Module parse failed/] -] diff --git a/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/index.js b/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/index.js deleted file mode 100644 index cf9c32f614f8..000000000000 --- a/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/index.js +++ /dev/null @@ -1,19 +0,0 @@ -import a from "./a"; - -it("should recover after error with no-emit-on-errors", async () => { - expect(a).toBe(1); - - // Step 1: syntax error - emitOnErrors: false prevents emit and record - try { - await NEXT_HMR(); - } catch (e) { - // Expected: no update available because emit was prevented - } - expect(a).toBe(1); - - // Step 2: fix - HMR should compare against step 0 (last good compilation) - await NEXT_HMR(); - expect(a).toBe(3); -}); - -module.hot.accept("./a"); diff --git a/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/rspack.config.js b/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/rspack.config.js deleted file mode 100644 index cac743954215..000000000000 --- a/tests/rspack-test/hotCases/recover/no-emit-on-errors-recover/rspack.config.js +++ /dev/null @@ -1,6 +0,0 @@ -/** @type {import("@rspack/core").Configuration} */ -module.exports = { - optimization: { - emitOnErrors: false, - }, -}; From d2d09c029bcc67a6d21e4fd849e35185bf93430d Mon Sep 17 00:00:00 2001 From: pshu Date: Wed, 8 Apr 2026 10:30:19 +0800 Subject: [PATCH 4/5] fixup! feat: implement compilation.hooks.shouldRecord and integrate with NoEmitOnErrorsPlugin fix clippy: remove redundant & --- crates/rspack_plugin_hmr/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/rspack_plugin_hmr/src/lib.rs b/crates/rspack_plugin_hmr/src/lib.rs index 38a9b6b9cf87..80877e44d177 100644 --- a/crates/rspack_plugin_hmr/src/lib.rs +++ b/crates/rspack_plugin_hmr/src/lib.rs @@ -134,7 +134,7 @@ async fn process_assets(&self, compilation: &mut Compilation) -> Result<()> { if let Some(current_chunk) = current_chunk { new_runtime = current_chunk .runtime() - .intersection(&all_old_runtime) + .intersection(all_old_runtime) .copied() .collect(); From 0be71225c40f302052ef248625746da24f656259 Mon Sep 17 00:00:00 2001 From: pshu Date: Thu, 9 Apr 2026 16:33:45 +0800 Subject: [PATCH 5/5] refactor: specify mode in incremental testing --- .../cases/incremental/remove-optimized-module/rspack.config.js | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/e2e/cases/incremental/remove-optimized-module/rspack.config.js b/tests/e2e/cases/incremental/remove-optimized-module/rspack.config.js index 6998b9669500..d6994a027c4c 100644 --- a/tests/e2e/cases/incremental/remove-optimized-module/rspack.config.js +++ b/tests/e2e/cases/incremental/remove-optimized-module/rspack.config.js @@ -2,6 +2,7 @@ const { rspack } = require('@rspack/core'); /** @type {import("@rspack/core").Configuration} */ module.exports = { + mode: 'development', entry: './index.js', cache: true, experiments: {