Fix Init hook logic

That was a regression - it was the same as the `beforeRequest` hook
sindresorhus · Feb 16, 2020 · 64aeb40 · 64aeb40
1 parent 32e609f
commit 64aeb40
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 23 deletions.
diff --git a/source/known-hook-events.ts b/source/known-hook-events.ts
@@ -7,7 +7,7 @@ Called with plain request options, right before their normalization. This is esp
 
 @see [Request migration guide](https://github.com/sindresorhus/got/blob/master/migration-guides.md#breaking-changes) for an example.
 */
-export type InitHook = (options: NormalizedOptions) => void;
+export type InitHook = (options: Options) => void;
 
 /**
 Called with normalized [request options](https://github.com/sindresorhus/got#options). Got will make no further changes to the request before it is sent (except the body serialization). This is especially useful in conjunction with [`got.extend()`](https://github.com/sindresorhus/got#instances) when you want to create an API client that, for example, uses HMAC-signing.

diff --git a/source/normalize-arguments.ts b/source/normalize-arguments.ts
@@ -11,7 +11,7 @@ import is from '@sindresorhus/is';
 import CacheableLookup from 'cacheable-lookup';
 import {Merge} from 'type-fest';
 import {UnsupportedProtocolError} from './errors';
-import knownHookEvents from './known-hook-events';
+import knownHookEvents, {InitHook} from './known-hook-events';
 import dynamicRequire from './utils/dynamic-require';
 import getBodySize from './utils/get-body-size';
 import isFormData from './utils/is-form-data';
@@ -274,21 +274,39 @@ export const normalizeArguments = (url: URLOrOptions, options?: Options, default
 		options = {};
 	}
 
-	if (is.urlInstance(url) || is.string(url)) {
+	const runInitHooks = (hooks?: InitHook[], options?: Options): void => {
+		if (hooks) {
+			for (const hook of hooks) {
+				const result = hook(options!);
+
+				if (is.promise(result)) {
+					throw new TypeError('The `init` hook must be a synchronous function');
+				}
+			}
+		}
+	};
+
+	const hasUrl = is.urlInstance(url) || is.string(url);
+	if (hasUrl) {
 		if (Reflect.has(options, 'url')) {
 			throw new TypeError('The `url` option cannot be used if the input is a valid URL.');
 		}
 
 		// @ts-ignore URL is not URL
 		options.url = url;
 
-		options = mergeOptions(defaults?.options ?? {}, options);
+		runInitHooks(options.hooks?.init, options);
+	} else if (Reflect.has(url as object, 'resolve')) {
+		throw new Error('The legacy `url.Url` is deprecated. Use `URL` instead.');
 	} else {
-		if (Reflect.has(url, 'resolve')) {
-			throw new Error('The legacy `url.Url` is deprecated. Use `URL` instead.');
-		}
+		runInitHooks((url as Options).hooks?.init, url as Options);
+		runInitHooks(options.hooks?.init, options);
+	}
 
-		options = mergeOptions(defaults?.options ?? {}, url, options);
+	if (hasUrl) {
+		options = mergeOptions(defaults?.options ?? {}, options);
+	} else {
+		options = mergeOptions(defaults?.options ?? {}, url as object, options);
 	}
 
 	// Normalize URL
@@ -335,14 +353,6 @@ export const normalizeArguments = (url: URLOrOptions, options?: Options, default
 		}
 	}
 
-	for (const hook of normalizedOptions.hooks.init) {
-		const result = hook(normalizedOptions);
-
-		if (is.promise(result)) {
-			throw new TypeError('The `init` hook must be a synchronous function');
-		}
-	}
-
 	return normalizedOptions;
 };
 

diff --git a/test/arguments.ts b/test/arguments.ts
@@ -330,7 +330,7 @@ test('`context` option is accessible when using hooks', withServer, async (t, se
 	await got({
 		context,
 		hooks: {
-			init: [
+			beforeRequest: [
 				options => {
 					t.is(options.context, context);
 					t.false({}.propertyIsEnumerable.call(options, 'context'));

diff --git a/test/hooks.ts b/test/hooks.ts
@@ -186,15 +186,18 @@ test('catches beforeError errors', async t => {
 test('init is called with options', withServer, async (t, server, got) => {
 	server.get('/', echoHeaders);
 
+	const context = {};
+
 	await got({
 		hooks: {
 			init: [
 				options => {
-					t.is(options.url.pathname, '/');
-					t.is(options.url.hostname, 'localhost');
+					t.is(options.url, undefined);
+					t.is(options.context, context);
 				}
 			]
-		}
+		},
+		context
 	});
 });
 
@@ -203,11 +206,13 @@ test('init allows modifications', withServer, async (t, server, got) => {
 		response.end(request.headers.foo);
 	});
 
-	const {body} = await got({
+	const {body} = await got('meh', {
+		headers: {},
 		hooks: {
 			init: [
 				options => {
-					options.headers.foo = 'bar';
+					options.url = '';
+					options.headers!.foo = 'bar';
 				}
 			]
 		}
@@ -679,7 +684,7 @@ test('timeout can be modified using a hook', withServer, async (t, server, got)
 	await t.throwsAsync(got({
 		timeout: 1000,
 		hooks: {
-			init: [
+			beforeRequest: [
 				options => {
 					options.timeout.request = 500;
 				}