Repository: sindresorhus/got
Pull request #532 (2018-07-19 12:03:33)

Pass normalized options to the handler
That's just an improvement.


Patch:
diff --git a/advanced-creation.md b/advanced-creation.md
index 59b14c9..0cae26a 100644
--- a/advanced-creation.md
+++ b/advanced-creation.md
@@ -31,26 +31,26 @@ Function making additional changes to the request.
 To inherit from parent, set it as `got.defaults.handler`.<br>
 To use the default handler, just omit specifying this.
 
-###### [url](readme.md#url)
-
 ###### [options](readme.md#options)
 
+**Note:** These options are [normalized](source/normalize-arguments.js).
+
 ###### next()
 
 Normalizes arguments and returns a `Promise` or a `Stream` depending on [`options.stream`](readme.md#stream).
 
 ```js
 const settings = {
-	handler: (url, options, next) => {
+	handler: (options, next) => {
 		if (options.stream) {
 			// It's a Stream
 			// We can perform stream-specific actions on it
-			return next(url, options)
+			return next(options)
 				.on('request', request => setTimeout(() => request.abort(), 50));
 		}
 
 		// It's a Promise
-		return next(url, options);
+		return next(options);
 	},
 	methods: got.defaults.methods,
 	options: {
@@ -64,9 +64,7 @@ const jsonGot = got.create(settings);
 
 ```js
 const defaults = {
-	handler: (url, options, next) => {
-		return next(url, options);
-	},
+	handler: (options, next) => next(options),
 	methods: [
 		'get',
 		'post',
diff --git a/package.json b/package.json
index 9ce6846..0d31716 100644
--- a/package.json
+++ b/package.json
@@ -35,7 +35,7 @@
 	],
 	"dependencies": {
 		"@sindresorhus/is": "^0.9.0",
-		"cacheable-request": "^2.1.1",
+		"cacheable-request": "^4.0.1",
 		"decompress-response": "^3.3.0",
 		"duplexer3": "^0.1.4",
 		"extend": "^3.0.1",
diff --git a/source/create.js b/source/create.js
index 55e173a..41bb8de 100644
--- a/source/create.js
+++ b/source/create.js
@@ -1,5 +1,4 @@
 'use strict';
-const URLGlobal = typeof URL === 'undefined' ? require('url').URL : URL; // TODO: Use the `URL` global when targeting Node.js 10
 const errors = require('./errors');
 const assignOptions = require('./assign-options');
 const asStream = require('./as-stream');
@@ -7,15 +6,7 @@ const asPromise = require('./as-promise');
 const normalizeArguments = require('./normalize-arguments');
 const deepFreeze = require('./deep-freeze');
 
-const makeNext = defaults => (path, options) => {
-	let url = path;
-
-	if (options.baseUrl) {
-		url = new URLGlobal(path, options.baseUrl);
-	}
-
-	options = normalizeArguments(url, options, defaults);
-
+const next = options => {
 	if (options.stream) {
 		return asStream(options);
 	}
@@ -24,15 +15,14 @@ const makeNext = defaults => (path, options) => {
 };
 
 const create = defaults => {
-	const next = makeNext(defaults);
 	if (!defaults.handler) {
-		defaults.handler = next;
+		defaults.handler = (options, next) => next(options);
 	}
 
 	function got(url, options) {
 		try {
 			options = assignOptions(defaults.options, options);
-			return defaults.handler(url, options, next);
+			return defaults.handler(normalizeArguments(url, options, defaults), next);
 		} catch (error) {
 			return Promise.reject(error);
 		}
@@ -48,7 +38,7 @@ const create = defaults => {
 	got.stream = (url, options) => {
 		options = assignOptions(defaults.options, options);
 		options.stream = true;
-		return defaults.handler(url, options, next);
+		return defaults.handler(normalizeArguments(url, options, defaults), next);
 	};
 
 	for (const method of defaults.methods) {
diff --git a/source/normalize-arguments.js b/source/normalize-arguments.js
index e09e97d..d427da9 100644
--- a/source/normalize-arguments.js
+++ b/source/normalize-arguments.js
@@ -1,5 +1,6 @@
 'use strict';
-const URLSearchParamsGlobal = typeof URLSearchParams === 'undefined' ? require('url').URLSearchParams : URLSearchParams; // TODO: Use the `URL` global when targeting Node.js 10
+const URLGlobal = typeof URL === 'undefined' ? require('url').URL : URL; // TODO: Use the `URL` global when targeting Node.js 10
+const URLSearchParamsGlobal = typeof URLSearchParams === 'undefined' ? require('url').URLSearchParams : URLSearchParams;
 const is = require('@sindresorhus/is');
 const toReadableStream = require('to-readable-stream');
 const urlParseLax = require('url-parse-lax');
@@ -17,18 +18,24 @@ module.exports = (url, options, defaults) => {
 
 	if (!is.string(url) && !is.object(url)) {
 		throw new TypeError(`Parameter \`url\` must be a string or object, not ${is(url)}`);
-	} else if (is.string(url)) {
-		url = url.replace(/^unix:/, 'http://$&');
+	}
 
-		try {
-			decodeURI(url);
-		} catch (_) {
-			throw new Error('Parameter `url` must contain valid UTF-8 character sequences');
-		}
+	if (is.string(url)) {
+		if (options.baseUrl) {
+			url = urlToOptions(new URLGlobal(url, options.baseUrl));
+		} else {
+			url = url.replace(/^unix:/, 'http://$&');
 
-		url = urlParseLax(url);
-		if (url.auth) {
-			throw new Error('Basic authentication must be done with the `auth` option');
+			try {
+				decodeURI(url);
+			} catch (_) {
+				throw new Error('Parameter `url` must contain valid UTF-8 character sequences');
+			}
+
+			url = urlParseLax(url);
+			if (url.auth) {
+				throw new Error('Basic authentication must be done with the `auth` option');
+			}
 		}
 	} else if (is(url) === 'URL') {
 		url = urlToOptions(url);
@@ -36,6 +43,7 @@ module.exports = (url, options, defaults) => {
 
 	options = {
 		path: '',
+		headers: {},
 		...url,
 		protocol: url.protocol || 'http:', // Override both null/undefined with default protocol
 		...options
diff --git a/source/progress.js b/source/progress.js
new file mode 100644
index 0000000..adbd804
--- /dev/null
+++ b/source/progress.js
@@ -0,0 +1,69 @@
+'use strict';
+module.exports = {
+	upload(req, emitter, uploadBodySize) {
+		const uploadEventFrequency = 150;
+		let uploaded = 0;
+		let progressInterval;
+
+		emitter.emit('uploadProgress', {
+			percent: 0,
+			transferred: 0,
+			total: uploadBodySize
+		});
+
+		req.once('error', () => {
+			clearInterval(progressInterval);
+		});
+
+		req.once('response', () => {
+			clearInterval(progressInterval);
+
+			emitter.emit('uploadProgress', {
+				percent: 1,
+				transferred: uploaded,
+				total: uploadBodySize
+			});
+		});
+
+		req.once('socket', socket => {
+			const onSocketConnect = () => {
+				progressInterval = setInterval(() => {
+					if (socket.destroyed) {
+						clearInterval(progressInterval);
+						return;
+					}
+
+					const lastUploaded = uploaded;
+					const headersSize = req._header ? Buffer.byteLength(req._header) : 0;
+					uploaded = socket.bytesWritten - headersSize;
+
+					// Prevent the known issue of `bytesWritten` being larger than body size
+					if (uploadBodySize && uploaded > uploadBodySize) {
+						uploaded = uploadBodySize;
+					}
+
+					// Don't emit events with unchanged progress and
+					// prevent last event from being emitted, because
+					// it's emitted when `response` is emitted
+					if (uploaded === lastUploaded || uploaded === uploadBodySize) {
+						return;
+					}
+
+					emitter.emit('uploadProgress', {
+						percent: uploadBodySize ? uploaded / uploadBodySize : 0,
+						transferred: uploaded,
+						total: uploadBodySize
+					});
+				}, uploadEventFrequency);
+			};
+
+			if (socket.connecting) {
+				socket.once('connect', onSocketConnect);
+			} else {
+				// The socket is being reused from pool,
+				// so the connect event will not be emitted
+				onSocketConnect();
+			}
+		});
+	}
+};
diff --git a/source/request-as-event-emitter.js b/source/request-as-event-emitter.js
index 80a9ee6..0f9f3bc 100644
--- a/source/request-as-event-emitter.js
+++ b/source/request-as-event-emitter.js
@@ -9,6 +9,7 @@ const is = require('@sindresorhus/is');
 const timedOut = require('./timed-out');
 const getBodySize = require('./get-body-size');
 const getResponse = require('./get-response');
+const progress = require('./progress');
 const {CacheError, UnsupportedProtocolError, MaxRedirectsError, RequestError} = require('./errors');
 
 const getMethodRedirectCodes = new Set([300, 301, 302, 303, 304, 305, 307, 308]);
@@ -23,7 +24,6 @@ module.exports = (options = {}) => {
 	let retryTries = 0;
 	let redirectUrl;
 	let uploadBodySize;
-	let uploaded = 0;
 
 	const get = options => {
 		if (options.protocol !== 'http:' && options.protocol !== 'https:') {
@@ -43,20 +43,9 @@ module.exports = (options = {}) => {
 			fn = electron.net || electron.remote.net;
 		}
 
-		let progressInterval;
-
 		const cacheableRequest = new CacheableRequest(fn.request, options.cache);
 		const cacheReq = cacheableRequest(options, response => {
-			clearInterval(progressInterval);
-
-			emitter.emit('uploadProgress', {
-				percent: 1,
-				transferred: uploaded,
-				total: uploadBodySize
-			});
-
 			const {statusCode} = response;
-
 			response.retryCount = retryCount;
 			response.url = redirectUrl || requestUrl;
 			response.requestUrl = requestUrl;
@@ -99,7 +88,6 @@ module.exports = (options = {}) => {
 				emitter.emit('redirect', response, redirectOpts);
 
 				get(redirectOpts);
-
 				return;
 			}
 
@@ -127,8 +115,6 @@ module.exports = (options = {}) => {
 			});
 
 			req.once('error', error => {
-				clearInterval(progressInterval);
-
 				if (aborted) {
 					return;
 				}
@@ -141,62 +127,9 @@ module.exports = (options = {}) => {
 				});
 			});
 
-			emitter.once('request', req => {
-				emitter.emit('uploadProgress', {
-					percent: 0,
-					transferred: 0,
-					total: uploadBodySize
-				});
-
-				const socket = req.connection;
-				if (socket) {
-					const onSocketConnect = () => {
-						const uploadEventFrequency = 150;
-
-						progressInterval = setInterval(() => {
-							if (socket.destroyed) {
-								clearInterval(progressInterval);
-								return;
-							}
-
-							const lastUploaded = uploaded;
-							const headersSize = req._header ? Buffer.byteLength(req._header) : 0;
-							uploaded = socket.bytesWritten - headersSize;
-
-							// Prevent the known issue of `bytesWritten` being larger than body size
-							if (uploadBodySize && uploaded > uploadBodySize) {
-								uploaded = uploadBodySize;
-							}
-
-							// Don't emit events with unchanged progress and
-							// prevent last event from being emitted, because
-							// it's emitted when `response` is emitted
-							if (uploaded === lastUploaded || uploaded === uploadBodySize) {
-								return;
-							}
-
-							emitter.emit('uploadProgress', {
-								percent: uploadBodySize ? uploaded / uploadBodySize : 0,
-								transferred: uploaded,
-								total: uploadBodySize
-							});
-						}, uploadEventFrequency);
-					};
-
-					// Only subscribe to `connect` event if we're actually connecting a new
-					// socket, otherwise if we're already connected (because this is a
-					// keep-alive connection) do not bother. This is important since we won't
-					// get a `connect` event for an already connected socket.
-					if (socket.connecting) {
-						socket.once('connect', onSocketConnect);
-					} else {
-						onSocketConnect();
-					}
-				}
-			});
+			progress.upload(req, emitter, uploadBodySize);
 
 			if (options.gotTimeout) {
-				clearInterval(progressInterval);
 				timedOut(req, options.gotTimeout);
 			}
 


Test patch:
diff --git a/test/arguments.js b/test/arguments.js
index ab70ab2..6ee1c36 100644
--- a/test/arguments.js
+++ b/test/arguments.js
@@ -60,7 +60,21 @@ test('requestUrl with url.parse object as first argument', async t => {
 });
 
 test('overrides querystring from opts', async t => {
-	t.is((await got(`${s.url}/?test=doge`, {query: {test: 'wow'}})).body, '/?test=wow');
+	const response = await got(
+		`${s.url}/?drop=this`,
+		{
+			query: {test: 'wow'},
+			cache: {
+				get(key) {
+					t.is(key, `cacheable-request:GET:${s.url}/?test=wow`);
+				},
+				set(key) {
+					t.is(key, `cacheable-request:GET:${s.url}/?test=wow`);
+				}
+			}
+		}
+	);
+	t.is(response.body, '/?test=wow');
 });
 
 test('should throw with auth in url string', async t => {
diff --git a/test/create.js b/test/create.js
index c5ad429..c225dff 100644
--- a/test/create.js
+++ b/test/create.js
@@ -76,17 +76,16 @@ test('custom headers (extend)', async t => {
 	t.is(headers.unicorn, 'rainbow');
 });
 
-test('custom endpoint with custom headers (create)', async t => {
-	const options = {headers: {unicorn: 'rainbow'}};
-	const handler = (url, options, next) => {
-		url = `${s.url}` + url;
-
-		return next(url, options);
-	};
-	const methods = ['get'];
-
-	const instance = got.create({options, methods, handler});
-	const headers = (await instance('/', {
+test('create', async t => {
+	const instance = got.create({
+		options: {},
+		methods: ['get'],
+		handler: (options, next) => {
+			options.headers.unicorn = 'rainbow';
+			return next(options);
+		}
+	});
+	const headers = (await instance(s.url, {
 		json: true
 	})).body;
 	t.is(headers.unicorn, 'rainbow');
@@ -99,7 +98,7 @@ test('custom endpoint with custom headers (extend)', async t => {
 		json: true
 	})).body;
 	t.is(headers.unicorn, 'rainbow');
-	t.is(headers['user-agent'] === undefined, false);
+	t.not(headers['user-agent'], undefined);
 });
 
 test('no tampering with defaults', t => {