Skip to content

Commit 780b3c0

Browse files
fix: preserve custom tap options dropped by _tap fast path (#227)
1 parent 5da5021 commit 780b3c0

2 files changed

Lines changed: 28 additions & 7 deletions

File tree

lib/Hook.js

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -90,13 +90,17 @@ class Hook {
9090
// Fast path: only `name` is set. Build the descriptor as a literal
9191
// so `_insert` and downstream consumers see the same hidden class
9292
// as the string-options path, avoiding a polymorphic call site.
93-
if (
94-
options.before === undefined &&
95-
options.stage === undefined &&
96-
options.context === undefined &&
97-
options.type === undefined &&
98-
options.fn === undefined
99-
) {
93+
// Scan with `for...in` (cheaper than allocating `Object.keys`)
94+
// to verify no other user-provided properties exist - e.g.
95+
// webpack's `additionalAssets` - otherwise they'd be dropped.
96+
let onlyName = true;
97+
for (const key in options) {
98+
if (key !== "name") {
99+
onlyName = false;
100+
break;
101+
}
102+
}
103+
if (onlyName) {
100104
options = { type, fn, name };
101105
} else {
102106
options.name = name;

test/Hook.test.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,23 @@ describe("Hook", () => {
183183
);
184184
});
185185

186+
it("should preserve custom tap options (e.g. webpack's `additionalAssets`) on the tap descriptor", () => {
187+
const hook = new SyncHook();
188+
189+
// Options with only `name` plus a custom property - used by webpack's
190+
// processAssets (`additionalAssets: true`). The fast-path in `_tap`
191+
// must not drop the custom property.
192+
hook.tap({ name: "A", additionalAssets: true }, () => {});
193+
// Options with `name`, `stage` and a custom property go through the
194+
// slow path - also checked for completeness.
195+
hook.tap({ name: "B", stage: 10, extra: "value" }, () => {});
196+
197+
expect(hook.taps[0].name).toBe("A");
198+
expect(hook.taps[0].additionalAssets).toBe(true);
199+
expect(hook.taps[1].name).toBe("B");
200+
expect(hook.taps[1].extra).toBe("value");
201+
});
202+
186203
it("should not ignore invalid before values", () => {
187204
// A plugin may use a hook that will never be executed
188205
const hook = new SyncHook();

0 commit comments

Comments
 (0)