fix: afterResolve in ContextModuleFactory should be AsyncSeriesWaterfallHook

SyMind · SyMind · commit c693c7ae9da5 · 2024-05-09T17:34:53.000+08:00
diff --git a/crates/node_binding/binding.d.ts b/crates/node_binding/binding.d.ts
@@ -291,8 +291,10 @@ export interface JsCompatSource {
   map?: Buffer
 }
 
-export interface JsContextModuleFactoryAfterResolveArgs {
+export interface JsContextModuleFactoryAfterResolveResult {
   resource: string
+  context: string
+  request: string
   regExp?: string
 }
 
diff --git a/crates/node_binding/src/plugins/interceptor.rs b/crates/node_binding/src/plugins/interceptor.rs
@@ -12,9 +12,9 @@ use napi::{
 use rspack_binding_values::{
   CompatSource, JsAfterResolveData, JsAfterResolveOutput, JsAssetEmittedArgs, JsBeforeResolveArgs,
   JsBeforeResolveOutput, JsChunk, JsChunkAssetArgs, JsCompilation,
-  JsContextModuleFactoryAfterResolveArgs, JsContextModuleFactoryAfterResolveResult, JsCreateData,
-  JsExecuteModuleArg, JsModule, JsNormalModuleFactoryCreateModuleArgs, JsResolveForSchemeArgs,
-  JsResolveForSchemeOutput, JsRuntimeModule, JsRuntimeModuleArg, ToJsCompatSource, ToJsModule,
+  JsContextModuleFactoryAfterResolveResult, JsCreateData, JsExecuteModuleArg, JsModule,
+  JsNormalModuleFactoryCreateModuleArgs, JsResolveForSchemeArgs, JsResolveForSchemeOutput,
+  JsRuntimeModule, JsRuntimeModuleArg, ToJsCompatSource, ToJsModule,
 };
 use rspack_core::{
   rspack_sources::SourceExt, AfterResolveResult, AssetEmittedInfo, BoxModule, Chunk, ChunkUkey,
@@ -445,8 +445,8 @@ pub struct RegisterJsTaps {
     ts_type = "(stages: Array<number>) => Array<{ function: ((arg: JsAfterResolveData) => Promise<boolean | undefined>); stage: number; }>"
   )]
   pub register_context_module_factory_after_resolve_taps: RegisterFunction<
-    JsContextModuleFactoryAfterResolveArgs,
-    Promise<JsContextModuleFactoryAfterResolveResult>,
+    JsContextModuleFactoryAfterResolveResult,
+    Promise<Option<JsContextModuleFactoryAfterResolveResult>>,
   >,
 }
 
@@ -675,7 +675,7 @@ define_register!(
 );
 define_register!(
   RegisterContextModuleFactoryAfterResolveTaps,
-  tap = ContextModuleFactoryAfterResolveTap<JsContextModuleFactoryAfterResolveArgs, Promise<JsContextModuleFactoryAfterResolveResult>> @ ContextModuleFactoryAfterResolveHook,
+  tap = ContextModuleFactoryAfterResolveTap<JsContextModuleFactoryAfterResolveResult, Promise<Option<JsContextModuleFactoryAfterResolveResult>>> @ ContextModuleFactoryAfterResolveHook,
   cache = true,
   sync = false,
   kind = RegisterJsTapKind::ContextModuleFactoryAfterResolve,
@@ -1236,20 +1236,30 @@ impl ContextModuleFactoryBeforeResolve for ContextModuleFactoryBeforeResolveTap
 
 #[async_trait]
 impl ContextModuleFactoryAfterResolve for ContextModuleFactoryAfterResolveTap {
-  async fn run(&self, result: &mut AfterResolveResult) -> rspack_error::Result<Option<bool>> {
-    let (ret, res) = self
+  async fn run(
+    &self,
+    mut result: AfterResolveResult,
+  ) -> rspack_error::Result<Option<AfterResolveResult>> {
+    let js_result = self
       .function
-      .call_with_promise(JsContextModuleFactoryAfterResolveArgs {
+      .call_with_promise(JsContextModuleFactoryAfterResolveResult {
         resource: result.resource.to_owned(),
+        context: result.context.to_owned(),
+        request: result.request.to_owned(),
         reg_exp: result.reg_exp.clone().map(|r| r.to_string()),
       })
       .await?;
-    result.resource = res.resource;
-    result.reg_exp = match res.reg_exp {
-      Some(r) => Some(RspackRegex::new(&r)?),
-      None => None,
-    };
-    Ok(ret)
+    match js_result {
+      Some(js_result) => {
+        result.resource = js_result.resource;
+        result.reg_exp = match js_result.reg_exp {
+          Some(r) => Some(RspackRegex::new(&r)?),
+          None => None,
+        };
+        Ok(Some(result))
+      }
+      None => Ok(None),
+    }
   }
 
   fn stage(&self) -> i32 {
diff --git a/crates/rspack_binding_values/src/context_module_factory.rs b/crates/rspack_binding_values/src/context_module_factory.rs
@@ -1,10 +1,9 @@
 use napi_derive::napi;
 
 #[napi(object)]
-pub struct JsContextModuleFactoryAfterResolveArgs {
+pub struct JsContextModuleFactoryAfterResolveResult {
   pub resource: String,
+  pub context: String,
+  pub request: String,
   pub reg_exp: Option<String>,
 }
-
-pub type JsContextModuleFactoryAfterResolveResult =
-  (Option<bool>, JsContextModuleFactoryAfterResolveArgs);
diff --git a/crates/rspack_core/src/context_module_factory.rs b/crates/rspack_core/src/context_module_factory.rs
@@ -12,16 +12,17 @@ use crate::{
   SharedPluginDriver,
 };
 
+#[derive(Clone)]
 pub struct AfterResolveResult {
   pub resource: String,
-  // context: Context,
+  pub context: String,
   // dependencies
   // layer
   // resolve_options
   // file_dependencies: HashSet<String>,
   // missing_dependencies: HashSet<String>,
   // context_dependencies: HashSet<String>,
-  // request: String,
+  pub request: String,
   // mode
   // recursive: bool,
   pub reg_exp: Option<RspackRegex>,
@@ -37,7 +38,7 @@ pub struct AfterResolveResult {
 }
 
 define_hook!(ContextModuleFactoryBeforeResolve: AsyncSeriesBail(data: &mut ModuleFactoryCreateData) -> bool);
-define_hook!(ContextModuleFactoryAfterResolve: AsyncSeriesBail(data: &mut AfterResolveResult) -> bool);
+define_hook!(ContextModuleFactoryAfterResolve: AsyncSeriesWaterfall(data: AfterResolveResult) -> Option<AfterResolveResult>);
 
 #[derive(Debug, Default)]
 pub struct ContextModuleFactoryHooks {
@@ -205,8 +206,7 @@ impl ContextModuleFactory {
       Err(err) => (Err(err), false),
     };
 
-    let mut context_module_options = None;
-    let module = match resource_data {
+    let (module, context_module_options) = match resource_data {
       Ok(ResolveResult::Resource(resource)) => {
         let options = ContextModuleOptions {
           addon: loader_request.to_string(),
@@ -216,11 +216,11 @@ impl ContextModuleFactory {
           resolve_options: data.resolve_options.clone(),
           context_options: dependency.options().clone(),
         };
-        context_module_options = Some(options.clone());
-        Box::new(ContextModule::new(
-          options,
+        let module = Box::new(ContextModule::new(
+          options.clone(),
           plugin_driver.resolver_factory.clone(),
-        ))
+        ));
+        (module, Some(options))
       }
       Ok(ResolveResult::Ignored) => {
         let ident = format!("{}/{}", data.context, specifier);
@@ -254,28 +254,32 @@ impl ContextModuleFactory {
     &self,
     context_module_options: &mut ContextModuleOptions,
   ) -> Result<Option<ModuleFactoryResult>> {
-    let mut after_resolve_result = AfterResolveResult {
+    let context_options = &context_module_options.context_options;
+    let after_resolve_result = AfterResolveResult {
       resource: context_module_options.resource.to_owned(),
-      reg_exp: context_module_options.context_options.reg_exp.clone(),
+      context: context_options.context.to_owned(),
+      request: context_options.request.to_owned(),
+      reg_exp: context_options.reg_exp.clone(),
     };
 
-    if let Some(false) = self
+    match self
       .plugin_driver
       .context_module_factory_hooks
       .after_resolve
-      .call(&mut after_resolve_result)
+      .call(after_resolve_result)
       .await?
     {
-      return Ok(None);
-    }
-
-    context_module_options.resource = after_resolve_result.resource;
-    context_module_options.context_options.reg_exp = after_resolve_result.reg_exp;
+      Some(after_resolve_result) => {
+        context_module_options.resource = after_resolve_result.resource;
+        context_module_options.context_options.reg_exp = after_resolve_result.reg_exp;
 
-    let module = ContextModule::new(
-      context_module_options.clone(),
-      self.loader_resolver_factory.clone(),
-    );
-    Ok(Some(ModuleFactoryResult::new_with_module(Box::new(module))))
+        let module = ContextModule::new(
+          context_module_options.clone(),
+          self.loader_resolver_factory.clone(),
+        );
+        Ok(Some(ModuleFactoryResult::new_with_module(Box::new(module))))
+      }
+      None => Ok(None),
+    }
   }
 }
diff --git a/crates/rspack_macros/src/hook.rs b/crates/rspack_macros/src/hook.rs
@@ -29,6 +29,18 @@ impl Parse for DefineHookInput {
       "SyncSeriesBail" => ExecKind::SyncSeriesBail {
         ret: ExecKind::parse_ret(input)?,
       },
+      "AsyncSeriesWaterfall" => {
+        let ret = match ExecKind::parse_ret(input)? {
+          Some(t) => t,
+          None => {
+            return Err(Error::new(
+              input.span(),
+              "Waterfall hooks must explicitly define a return type",
+            ))
+          }
+        };
+        ExecKind::AsyncSeriesWaterfall { ret }
+      }
       "AsyncSeries" => ExecKind::AsyncSeries,
       "AsyncParallel" => ExecKind::AsyncParallel,
       "SyncSeries" => ExecKind::SyncSeries,
@@ -142,6 +154,7 @@ impl DefineHookInput {
 enum ExecKind {
   AsyncSeries,
   AsyncSeriesBail { ret: Option<TypePath> },
+  AsyncSeriesWaterfall { ret: TypePath },
   AsyncParallel,
   SyncSeries,
   SyncSeriesBail { ret: Option<TypePath> },
@@ -160,7 +173,10 @@ impl ExecKind {
 
   pub fn is_async(&self) -> bool {
     match self {
-      Self::AsyncSeries | Self::AsyncSeriesBail { .. } | Self::AsyncParallel => true,
+      Self::AsyncSeries
+      | Self::AsyncSeriesBail { .. }
+      | Self::AsyncSeriesWaterfall { .. }
+      | Self::AsyncParallel => true,
       Self::SyncSeries | Self::SyncSeriesBail { .. } => false,
     }
   }
@@ -174,6 +190,9 @@ impl ExecKind {
           quote! { rspack_hook::__macro_helper::Result<std::option::Option<()>> }
         }
       }
+      Self::AsyncSeriesWaterfall { ret } => {
+        quote! { rspack_hook::__macro_helper::Result<#ret> }
+      }
       _ => quote! { rspack_hook::__macro_helper::Result<()> },
     }
   }
@@ -219,6 +238,18 @@ impl ExecKind {
           Ok(None)
         }
       }
+      Self::AsyncSeriesWaterfall { .. } => {
+        quote! {
+          #additional_taps
+          let mut data = #args;
+          for tap in all_taps {
+            if let Some(res) = tap.run(data.clone()).await? {
+              data = res;
+            }
+          }
+          Ok(Some(data))
+        }
+      }
       Self::AsyncParallel => {
         quote! {
           #additional_taps
diff --git a/packages/rspack/src/Compiler.ts b/packages/rspack/src/Compiler.ts
@@ -47,7 +47,7 @@ import { assertNotNill } from "./util/assertNotNil";
 import { FileSystemInfoEntry } from "./FileSystemInfo";
 import { RuntimeGlobals } from "./RuntimeGlobals";
 import { tryRunOrWebpackError } from "./lib/HookWebpackError";
-import { CodeGenerationResult, ContextModuleFactoryResolveData, Module, ResolveData } from "./Module";
+import { CodeGenerationResult, ContextModuleFactoryAfterResolveResult, Module, ResolveData } from "./Module";
 import { canInherentFromParent } from "./builtin-plugin/base";
 import ExecuteModulePlugin from "./ExecuteModulePlugin";
 import { Chunk } from "./Chunk";
@@ -510,7 +510,7 @@ class Compiler {
 						createData: arg.createData
 					};
 					const ret = await queried.promise(data);
-					return [ret, data.createData];
+					return [!!ret, ret];
 				}
 			),
 			registerNormalModuleFactoryCreateModuleTaps: this.#createHookRegisterTaps(
@@ -546,23 +546,23 @@ class Compiler {
 				this.#createHookRegisterTaps(
 					binding.RegisterJsTapKind.ContextModuleFactoryAfterResolve,
 					() => this.compilationParams!.contextModuleFactory.hooks.afterResolve,
-					queried => async (args: binding.JsContextModuleFactoryAfterResolveArgs) => {
-						const resolveData: ContextModuleFactoryResolveData = {
+					queried => async (args: binding.JsContextModuleFactoryAfterResolveResult) => {
+						const resolveData: ContextModuleFactoryAfterResolveResult = {
 							resource: args.resource,
-							regExp: args.regExp ? new RegExp(args.regExp) : undefined
-							// request: arg.request,
-							// context: arg.context,
-							// fileDependencies: arg.fileDependencies,
-							// missingDependencies: arg.missingDependencies,
-							// contextDependencies: arg.contextDependencies,
-							// createData: arg.createData
+							regExp: args.regExp ? new RegExp(args.regExp) : undefined,
+							request: args.request,
+							context: args.context,
 						};
 						const raw = await queried.promise(resolveData);
 						const result = raw ? {
 							resource: raw.resource,
-							regExp: raw.regExp?.toString()
-						} satisfies binding.JsContextModuleFactoryAfterResolveArgs : undefined;
-						return [!!raw, result];
+							context: '',
+							request: '',
+							regExp: raw.regExp?.toString(),
+							// TODO: Dependencies are not fully supported yet; this is a placeholder to prevent errors in moment-locales-webpack-plugin.
+							dependencies: [],
+						} : undefined;
+						return result;
 					}
 				)
 		};
diff --git a/packages/rspack/src/ContextModuleFactory.ts b/packages/rspack/src/ContextModuleFactory.ts
@@ -1,14 +1,14 @@
 import * as liteTapable from "./lite-tapable";
-import { ContextModuleFactoryResolveData, ResolveData } from "./Module";
+import { ContextModuleFactoryAfterResolveResult, ResolveData } from "./Module";
 
 export class ContextModuleFactory {
 	hooks: {
 		// TODO: second param resolveData
 		// resolveForScheme: HookMap<
 		// 	AsyncSeriesBailHook<[ResourceDataWithData], true | void>
 		// >;
-		beforeResolve: liteTapable.AsyncSeriesBailHook<[ResolveData], false | void>;
-		afterResolve: liteTapable.AsyncSeriesBailHook<[ContextModuleFactoryResolveData], false | void | ContextModuleFactoryResolveData>;
+		beforeResolve: liteTapable.AsyncSeriesWaterfallHook<[ResolveData], false | void>;
+		afterResolve: liteTapable.AsyncSeriesWaterfallHook<[ContextModuleFactoryAfterResolveResult], false | void | ContextModuleFactoryAfterResolveResult>;
 	};
 	constructor() {
 		this.hooks = {
diff --git a/packages/rspack/src/Module.ts b/packages/rspack/src/Module.ts
@@ -26,8 +26,10 @@ export type ResolveData = {
 	createData?: CreateData;
 };
 
-export type ContextModuleFactoryResolveData = {
+export type ContextModuleFactoryAfterResolveResult = {
 	resource: string;
+	context: string
+	request: string
 	regExp?: RegExp;
 }
 
diff --git a/packages/rspack/src/lite-tapable/index.ts b/packages/rspack/src/lite-tapable/index.ts

Original file line number	Diff line number	Diff line change
`@@ -291,8 +291,10 @@ export interface JsCompatSource {`
`291`	`291`	`map?: Buffer`
`292`	`292`	`}`
`293`	`293`
`294`		`-export interface JsContextModuleFactoryAfterResolveArgs {`
	`294`	`+export interface JsContextModuleFactoryAfterResolveResult {`
`295`	`295`	`resource: string`
	`296`	`+ context: string`
	`297`	`+ request: string`
`296`	`298`	`regExp?: string`
`297`	`299`	`}`
`298`	`300`