From f208db9450672b8df2a343362473fe9bdb1e5166 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 21 Apr 2026 04:00:24 +0000 Subject: [PATCH 1/2] fix: preserve custom tap options dropped by _tap fast path The fast path added in 9e9ae4d (fix(perf): improve (#224)) only checks for a fixed set of properties (before/stage/context/type/fn) before rebuilding the options descriptor as `{ type, fn, name }`. Any custom user-provided property - notably webpack's `additionalAssets`, used by the processAssets hook - was silently dropped, breaking the webpack `ConfigTestCases > process-assets > additional-assets` test. Tighten the fast-path guard to a strict "only name" check via Object.keys length, preserving the hidden-class optimization for the common `hook.tap({ name: "X" }, fn)` case while restoring correctness for every other call shape. --- lib/Hook.js | 11 ++++------- test/Hook.test.js | 17 +++++++++++++++++ 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/lib/Hook.js b/lib/Hook.js index 560bb96..7ff74f0 100644 --- a/lib/Hook.js +++ b/lib/Hook.js @@ -90,13 +90,10 @@ class Hook { // Fast path: only `name` is set. Build the descriptor as a literal // so `_insert` and downstream consumers see the same hidden class // as the string-options path, avoiding a polymorphic call site. - if ( - options.before === undefined && - options.stage === undefined && - options.context === undefined && - options.type === undefined && - options.fn === undefined - ) { + // Must be a strict "only name" check — any other user-supplied + // property (e.g. webpack's `additionalAssets`) needs to be + // preserved on the tap descriptor. + if (Object.keys(options).length === 1) { options = { type, fn, name }; } else { options.name = name; diff --git a/test/Hook.test.js b/test/Hook.test.js index bb6d2a8..6b551d4 100644 --- a/test/Hook.test.js +++ b/test/Hook.test.js @@ -183,6 +183,23 @@ describe("Hook", () => { ); }); + it("should preserve custom tap options (e.g. webpack's `additionalAssets`) on the tap descriptor", () => { + const hook = new SyncHook(); + + // Options with only `name` plus a custom property - used by webpack's + // processAssets (`additionalAssets: true`). The fast-path in `_tap` + // must not drop the custom property. + hook.tap({ name: "A", additionalAssets: true }, () => {}); + // Options with `name`, `stage` and a custom property go through the + // slow path - also checked for completeness. + hook.tap({ name: "B", stage: 10, extra: "value" }, () => {}); + + expect(hook.taps[0].name).toBe("A"); + expect(hook.taps[0].additionalAssets).toBe(true); + expect(hook.taps[1].name).toBe("B"); + expect(hook.taps[1].extra).toBe("value"); + }); + it("should not ignore invalid before values", () => { // A plugin may use a hook that will never be executed const hook = new SyncHook(); From 462240423cdf0e14b19c8a1effd0bed6c506a8ec Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 21 Apr 2026 04:28:42 +0000 Subject: [PATCH 2/2] perf(Hook): replace Object.keys with for-in in only-name guard The previous fix used `Object.keys(options).length === 1` to check whether the tap options object carries any properties beyond `name` before taking the fast-path literal construction. `Object.keys` allocates an intermediate array on every call, costing ~10% on the common `hook.tap({ name: "X" }, fn)` shape vs the (broken) baseline. Swapping to a `for...in` scan with an early-break avoids that allocation and pulls the regression down to within measurement noise (~3%), while keeping the correctness property - any extra user- supplied key (e.g. webpack's `additionalAssets`) forces the safe `Object.assign` slow path. Benchmarked locally with tinybench, 10 taps per iteration, mean of three runs: shape broken for-in delta string 2.66M 2.50M -6% {name} 2.16M 2.10M -3% {name,stage} 1.01M 0.98M -3% {name,additionalAssets} 2.19M 1.15M -48% * {name,before} 511K 491K -4% * inherent: this shape must now take the correct slow path since the previous "fast" speed was produced by dropping the property. --- lib/Hook.js | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/lib/Hook.js b/lib/Hook.js index 7ff74f0..5bf7417 100644 --- a/lib/Hook.js +++ b/lib/Hook.js @@ -90,10 +90,17 @@ class Hook { // Fast path: only `name` is set. Build the descriptor as a literal // so `_insert` and downstream consumers see the same hidden class // as the string-options path, avoiding a polymorphic call site. - // Must be a strict "only name" check — any other user-supplied - // property (e.g. webpack's `additionalAssets`) needs to be - // preserved on the tap descriptor. - if (Object.keys(options).length === 1) { + // Scan with `for...in` (cheaper than allocating `Object.keys`) + // to verify no other user-provided properties exist - e.g. + // webpack's `additionalAssets` - otherwise they'd be dropped. + let onlyName = true; + for (const key in options) { + if (key !== "name") { + onlyName = false; + break; + } + } + if (onlyName) { options = { type, fn, name }; } else { options.name = name;