From db60acaf397377252159c897460c49c221c5adb6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 18 Jan 2026 20:33:34 +0000 Subject: [PATCH 1/2] feat(HookMap): add tap, tapAsync, tapPromise convenience methods Add tap(), tapAsync(), and tapPromise() methods to HookMap and QueriedHookMap classes for webpack compatibility. These methods delegate to for(key).tap/tapAsync/tapPromise, matching the API provided by webpack's tapable library. This enables webpack plugins that use patterns like hookMap.tap(key, "name", fn) to work correctly with rspack. --- src/index.ts | 48 +++++++++++++++ test/HookMap.test.js | 135 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 183 insertions(+) create mode 100644 test/HookMap.test.js diff --git a/src/index.ts b/src/index.ts index a55d967..85dbd9e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1042,6 +1042,30 @@ export class HookMap { return newHook; } + tap( + key: HookMapKey, + options: Options>, + fn: Fn, ExtractHookReturn>, + ) { + return this.for(key).tap(options, fn); + } + + tapAsync( + key: HookMapKey, + options: Options>, + fn: FnAsync, ExtractHookReturn>, + ) { + return this.for(key).tapAsync(options, fn); + } + + tapPromise( + key: HookMapKey, + options: Options>, + fn: FnPromise, ExtractHookReturn>, + ) { + return this.for(key).tapPromise(options, fn); + } + intercept(interceptor: HookMapInterceptor) { this._interceptors.push( Object.assign( @@ -1093,6 +1117,30 @@ export class QueriedHookMap { } return false; } + + tap( + key: HookMapKey, + options: Options>, + fn: Fn, ExtractHookReturn>, + ) { + return this.hookMap.for(key).tap(options, fn); + } + + tapAsync( + key: HookMapKey, + options: Options>, + fn: FnAsync, ExtractHookReturn>, + ) { + return this.hookMap.for(key).tapAsync(options, fn); + } + + tapPromise( + key: HookMapKey, + options: Options>, + fn: FnPromise, ExtractHookReturn>, + ) { + return this.hookMap.for(key).tapPromise(options, fn); + } } export class MultiHook { diff --git a/test/HookMap.test.js b/test/HookMap.test.js new file mode 100644 index 0000000..ec63eef --- /dev/null +++ b/test/HookMap.test.js @@ -0,0 +1,135 @@ +/** + * Tests for HookMap tap convenience methods + */ + +"use strict"; + +const { HookMap, SyncHook, AsyncSeriesHook } = require("../"); + +describe("HookMap", () => { + describe("tap convenience methods", () => { + it("should redirect tap to for(key).tap", () => { + const hookMap = new HookMap(() => new SyncHook(["arg"])); + const calls = []; + + hookMap.tap("key1", "Plugin1", (arg) => { + calls.push({ key: "key1", arg }); + }); + + hookMap.tap("key2", "Plugin2", (arg) => { + calls.push({ key: "key2", arg }); + }); + + hookMap.for("key1").call("value1"); + hookMap.for("key2").call("value2"); + + expect(calls).toEqual([ + { key: "key1", arg: "value1" }, + { key: "key2", arg: "value2" } + ]); + }); + + it("should redirect tapAsync to for(key).tapAsync", async () => { + const hookMap = new HookMap(() => new AsyncSeriesHook(["arg"])); + const calls = []; + + hookMap.tapAsync("key1", "Plugin1", (arg, callback) => { + calls.push({ key: "key1", arg }); + callback(); + }); + + await hookMap.for("key1").promise("value1"); + + expect(calls).toEqual([{ key: "key1", arg: "value1" }]); + }); + + it("should redirect tapPromise to for(key).tapPromise", async () => { + const hookMap = new HookMap(() => new AsyncSeriesHook(["arg"])); + const calls = []; + + hookMap.tapPromise("key1", "Plugin1", async (arg) => { + calls.push({ key: "key1", arg }); + }); + + await hookMap.for("key1").promise("value1"); + + expect(calls).toEqual([{ key: "key1", arg: "value1" }]); + }); + + it("should create hook on first tap if not exists", () => { + const hookMap = new HookMap(() => new SyncHook(["arg"])); + + expect(hookMap.get("key1")).toBeUndefined(); + + hookMap.tap("key1", "Plugin1", () => {}); + + expect(hookMap.get("key1")).toBeDefined(); + }); + + it("should reuse existing hook when tapping multiple times", () => { + const hookMap = new HookMap(() => new SyncHook(["arg"])); + const calls = []; + + hookMap.tap("key1", "Plugin1", () => calls.push("first")); + hookMap.tap("key1", "Plugin2", () => calls.push("second")); + + hookMap.for("key1").call(); + + expect(calls).toEqual(["first", "second"]); + }); + }); + + describe("for method", () => { + it("should create hook on first access", () => { + const hookMap = new HookMap(() => new SyncHook(["arg"])); + + expect(hookMap.get("key1")).toBeUndefined(); + + const hook = hookMap.for("key1"); + + expect(hook).toBeDefined(); + expect(hookMap.get("key1")).toBe(hook); + }); + + it("should return same hook on subsequent access", () => { + const hookMap = new HookMap(() => new SyncHook(["arg"])); + + const hook1 = hookMap.for("key1"); + const hook2 = hookMap.for("key1"); + + expect(hook1).toBe(hook2); + }); + }); + + describe("get method", () => { + it("should return undefined for non-existent key", () => { + const hookMap = new HookMap(() => new SyncHook(["arg"])); + + expect(hookMap.get("nonexistent")).toBeUndefined(); + }); + + it("should return hook after it has been created", () => { + const hookMap = new HookMap(() => new SyncHook(["arg"])); + + hookMap.for("key1"); + + expect(hookMap.get("key1")).toBeDefined(); + }); + }); + + describe("isUsed", () => { + it("should return false when no hooks are used", () => { + const hookMap = new HookMap(() => new SyncHook(["arg"])); + + expect(hookMap.isUsed()).toBe(false); + }); + + it("should return true when a hook has taps", () => { + const hookMap = new HookMap(() => new SyncHook(["arg"])); + + hookMap.tap("key1", "Plugin1", () => {}); + + expect(hookMap.isUsed()).toBe(true); + }); + }); +}); From a26dc9b5c3815660d6803a9b467e5f40e439e249 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 18 Jan 2026 20:48:09 +0000 Subject: [PATCH 2/2] fix: remove tap methods from QueriedHookMap Remove tap(), tapAsync(), tapPromise() from QueriedHookMap as they bypass the stageRange constraint by delegating directly to the underlying hookMap. This breaks the abstraction since QueriedHook doesn't have tap methods and the query context should be respected. The HookMap convenience methods are kept as they correctly delegate to for(key).tap() without any stage filtering concerns. --- src/index.ts | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/src/index.ts b/src/index.ts index 85dbd9e..fec57bd 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1117,30 +1117,6 @@ export class QueriedHookMap { } return false; } - - tap( - key: HookMapKey, - options: Options>, - fn: Fn, ExtractHookReturn>, - ) { - return this.hookMap.for(key).tap(options, fn); - } - - tapAsync( - key: HookMapKey, - options: Options>, - fn: FnAsync, ExtractHookReturn>, - ) { - return this.hookMap.for(key).tapAsync(options, fn); - } - - tapPromise( - key: HookMapKey, - options: Options>, - fn: FnPromise, ExtractHookReturn>, - ) { - return this.hookMap.for(key).tapPromise(options, fn); - } } export class MultiHook {