Allow explicitly unsetting hooks on .extend() (#599)

* feat: allow explicitly unsetting hooks on `.extend()`

The implementation here is pretty simple: just give hook merging some custom
logic so it either merges OR resets to an empty array. Matches what was
discussed in the related ticket.

Documentation has been updated to describe this behavior as well.

The dedicated test is just a modified copy of the existing `ky.extend()` one.
Admittedly, I did modify a couple of other tests to cover the edge cases
instead of building brand new tests. No reason other than noticing they were
already close to what I needed and I'm being lazy.

Closes #408

* chore: remove unused import

Author: Kenneth-Sills

readme.md

@@ -487,6 +487,8 @@ You can pass headers as a `Headers` instance or a plain object.
 You can remove a header with `.extend()` by passing the header with an `undefined` value.
 Passing `undefined` as a string removes the header only if it comes from a `Headers` instance.
 
+Similarly, you can remove existing `hooks` entries by extending the hook with an explicit `undefined`.
+
 ```js
 import ky from 'ky';
 
@@ -496,16 +498,26 @@ const original = ky.create({
 	headers: {
 		rainbow: 'rainbow',
 		unicorn: 'unicorn'
-	}
+	},
+	hooks: {
+		beforeRequest: [ () => console.log('before 1') ],
+		afterResponse: [ () => console.log('after 1') ],
+	},
 });
 
 const extended = original.extend({
 	headers: {
 		rainbow: undefined
+	},
+	hooks: {
+		beforeRequest: undefined,
+		afterResponse: [ () => console.log('after 2') ],
 	}
 });
 
 const response = await extended(url).json();
+//=> after 1
+//=> after 2
 
 console.log('rainbow' in response);
 //=> false

source/core/Ky.ts

@@ -1,6 +1,5 @@
 import {HTTPError} from '../errors/HTTPError.js';
 import {TimeoutError} from '../errors/TimeoutError.js';
-import type {Hooks} from '../types/hooks.js';
 import type {
 	Input,
 	InternalOptions,
@@ -9,7 +8,7 @@ import type {
 	SearchParamsInit,
 } from '../types/options.js';
 import {type ResponsePromise} from '../types/ResponsePromise.js';
-import {deepMerge, mergeHeaders} from '../utils/merge.js';
+import {mergeHeaders, mergeHooks} from '../utils/merge.js';
 import {normalizeRequestMethod, normalizeRetryOptions} from '../utils/normalize.js';
 import timeout, {type TimeoutOptions} from '../utils/timeout.js';
 import delay from '../utils/delay.js';
@@ -133,7 +132,7 @@ export class Ky {
 			...(credentials && {credentials}), // For exactOptionalPropertyTypes
 			...options,
 			headers: mergeHeaders((this._input as Request).headers, options.headers),
-			hooks: deepMerge<Required<Hooks>>(
+			hooks: mergeHooks(
 				{
 					beforeRequest: [],
 					beforeRetry: [],

source/utils/merge.ts

@@ -1,4 +1,5 @@
 import type {KyHeadersInit, Options} from '../types/options.js';
+import type {Hooks} from '../types/hooks.js';
 import {isObject} from './is.js';
 
 export const validateAndMerge = (...sources: Array<Partial<Options> | undefined>): Partial<Options> => {
@@ -27,10 +28,26 @@ export const mergeHeaders = (source1: KyHeadersInit = {}, source2: KyHeadersInit
 	return result;
 };
 
+function newHookValue<K extends keyof Hooks>(original: Hooks, incoming: Hooks, property: K): Required<Hooks>[K] {
+	return (Object.hasOwn(incoming, property) && incoming[property] === undefined)
+		? []
+		: deepMerge<Required<Hooks>[K]>(original[property] ?? [], incoming[property] ?? []);
+}
+
+export const mergeHooks = (original: Hooks = {}, incoming: Hooks = {}): Required<Hooks> => (
+	{
+		beforeRequest: newHookValue(original, incoming, 'beforeRequest'),
+		beforeRetry: newHookValue(original, incoming, 'beforeRetry'),
+		afterResponse: newHookValue(original, incoming, 'afterResponse'),
+		beforeError: newHookValue(original, incoming, 'beforeError'),
+	}
+);
+
 // TODO: Make this strongly-typed (no `any`).
 export const deepMerge = <T>(...sources: Array<Partial<T> | undefined>): T => {
 	let returnValue: any = {};
 	let headers = {};
+	let hooks = {};
 
 	for (const source of sources) {
 		if (Array.isArray(source)) {
@@ -48,6 +65,11 @@ export const deepMerge = <T>(...sources: Array<Partial<T> | undefined>): T => {
 				returnValue = {...returnValue, [key]: value};
 			}
 
+			if (isObject((source as any).hooks)) {
+				hooks = mergeHooks(hooks, (source as any).hooks);
+				returnValue.hooks = hooks;
+			}
+
 			if (isObject((source as any).headers)) {
 				headers = mergeHeaders(headers, (source as any).headers);
 				returnValue.headers = headers;

test/main.ts

@@ -500,6 +500,7 @@ test('ky.create() with deep array', async t => {
 
 	let isOriginBeforeRequestTrigged = false;
 	let isExtendBeforeRequestTrigged = false;
+	let isExtendAfterResponseTrigged = false;
 
 	const extended = ky.create({
 		hooks: {
@@ -518,11 +519,17 @@ test('ky.create() with deep array', async t => {
 					isExtendBeforeRequestTrigged = true;
 				},
 			],
+			afterResponse: [
+				() => {
+					isExtendAfterResponseTrigged = true;
+				},
+			],
 		},
 	});
 
 	t.is(isOriginBeforeRequestTrigged, true);
 	t.is(isExtendBeforeRequestTrigged, true);
+	t.is(isExtendAfterResponseTrigged, true);
 
 	const {ok} = await extended.head(server.url);
 	t.true(ok);
@@ -549,6 +556,7 @@ const extendHooksMacro = test.macro<[{useFunction: boolean}]>(async (t, {useFunc
 	});
 
 	let isOriginBeforeRequestTrigged = false;
+	let isOriginAfterResponseTrigged = false;
 	let isExtendBeforeRequestTrigged = false;
 
 	const intermediateOptions = {
@@ -558,6 +566,11 @@ const extendHooksMacro = test.macro<[{useFunction: boolean}]>(async (t, {useFunc
 					isOriginBeforeRequestTrigged = true;
 				},
 			],
+			afterResponse: [
+				() => {
+					isOriginAfterResponseTrigged = true;
+				},
+			],
 		},
 	};
 	const extendedOptions = {
@@ -577,6 +590,7 @@ const extendHooksMacro = test.macro<[{useFunction: boolean}]>(async (t, {useFunc
 	await extended(server.url);
 
 	t.is(isOriginBeforeRequestTrigged, true);
+	t.is(isOriginAfterResponseTrigged, true);
 	t.is(isExtendBeforeRequestTrigged, true);
 
 	const {ok} = await extended.head(server.url);
@@ -639,6 +653,48 @@ test('ky.extend() with function retains parent defaults when not specified', asy
 	await server.close();
 });
 
+test('ky.extend() can remove hooks', async t => {
+	const server = await createHttpTestServer();
+	server.get('/', (_request, response) => {
+		response.end();
+	});
+
+	let isOriginalBeforeRequestTrigged = false;
+	let isOriginalAfterResponseTrigged = false;
+
+	const extended = ky
+		.extend({
+			hooks: {
+				beforeRequest: [
+					() => {
+						isOriginalBeforeRequestTrigged = true;
+					},
+				],
+				afterResponse: [
+					() => {
+						isOriginalAfterResponseTrigged = true;
+					},
+				],
+			},
+		})
+		.extend({
+			hooks: {
+				beforeRequest: undefined,
+				afterResponse: [],
+			},
+		});
+
+	await extended(server.url);
+
+	t.is(isOriginalBeforeRequestTrigged, false);
+	t.is(isOriginalAfterResponseTrigged, true);
+
+	const {ok} = await extended.head(server.url);
+	t.true(ok);
+
+	await server.close();
+});
+
 test('throws DOMException/Error with name AbortError when aborted by user', async t => {
 	const server = await createHttpTestServer();
 	// eslint-disable-next-line @typescript-eslint/no-empty-function