From 2649270b09b6131b0be25468e43a78431a005abc Mon Sep 17 00:00:00 2001 From: Szymon Marczak <36894700+szmarczak@users.noreply.github.com> Date: Wed, 25 Jul 2018 04:43:36 +0200 Subject: [PATCH] Pass normalized options to the handler (#532) --- advanced-creation.md | 14 ++++++-------- source/create.js | 18 ++++-------------- source/normalize-arguments.js | 30 +++++++++++++++++++----------- test/create.js | 23 +++++++++++------------ 4 files changed, 40 insertions(+), 45 deletions(-) diff --git a/advanced-creation.md b/advanced-creation.md index 59b14c966..0cae26ad2 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`.
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/source/create.js b/source/create.js index 55e173a85..41bb8de23 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 e09e97d1c..d427da99c 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/test/create.js b/test/create.js index c5ad42948..c225dff63 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 => {