From 7c4478af8c9d929f6a2b945bc2359022fe320e04 Mon Sep 17 00:00:00 2001 From: "U-DESKTOP-07IKGBD\\Szymon Marczak" Date: Thu, 23 Aug 2018 12:14:51 +0200 Subject: [PATCH 1/7] Rewrite the `baseUrl` option --- source/create.js | 1 + source/normalize-arguments.js | 113 ++++++++++++++++++++-------------- test/arguments.js | 16 +++++ test/create.js | 8 +-- 4 files changed, 87 insertions(+), 51 deletions(-) diff --git a/source/create.js b/source/create.js index 1761c538b..02833b04c 100644 --- a/source/create.js +++ b/source/create.js @@ -21,6 +21,7 @@ const aliases = [ const create = defaults => { defaults = merge({}, defaults); + defaults.options = normalizeArguments.prenormalize(defaults.options); if (!defaults.handler) { defaults.handler = next; } diff --git a/source/normalize-arguments.js b/source/normalize-arguments.js index e26547fed..40bfb261a 100644 --- a/source/normalize-arguments.js +++ b/source/normalize-arguments.js @@ -13,6 +13,47 @@ const knownHookEvents = require('./known-hook-events'); const retryAfterStatusCodes = new Set([413, 429, 503]); +const prenormalize = options => { + options = { + headers: {}, + ...options + }; + + if (options.baseUrl && !options.baseUrl.toString().endsWith('/')) { + options.baseUrl += '/'; + } + + if (is.undefined(options.followRedirect)) { + options.followRedirect = true; + } + + if (is.nullOrUndefined(options.hooks)) { + options.hooks = {}; + } + if (is.object(options.hooks)) { + for (const hookEvent of knownHookEvents) { + const hooks = options.hooks[hookEvent]; + if (is.nullOrUndefined(hooks)) { + options.hooks[hookEvent] = []; + } else if (is.array(hooks)) { + for (const [index, hook] of hooks.entries()) { + if (!is.function(hook)) { + throw new TypeError( + `Parameter \`hooks.${hookEvent}[${index}]\` must be a function, not ${is(hook)}` + ); + } + } + } else { + throw new TypeError(`Parameter \`hooks.${hookEvent}\` must be an array, not ${is(hooks)}`); + } + } + } else { + throw new TypeError(`Parameter \`hooks\` must be an object, not ${is(options.hooks)}`); + } + + return options; +}; + module.exports = (url, options, defaults) => { if (Reflect.has(options, 'url') || (is.object(url) && Reflect.has(url, 'url'))) { throw new TypeError('Parameter `url` is not an option. Use got(url, options)'); @@ -22,8 +63,14 @@ module.exports = (url, options, defaults) => { throw new TypeError(`Parameter \`url\` must be a string or object, not ${is(url)}`); } + options = prenormalize(options); + if (is.string(url)) { if (options.baseUrl) { + if (url.toString().startsWith('/')) { + url = url.toString().slice(1); + } + url = urlToOptions(new URLGlobal(url, options.baseUrl)); } else { url = url.replace(/^unix:/, 'http://$&'); @@ -45,7 +92,6 @@ module.exports = (url, options, defaults) => { options = { path: '', - headers: {}, ...url, protocol: url.protocol || 'http:', // Override both null/undefined with default protocol ...options @@ -59,22 +105,17 @@ module.exports = (url, options, defaults) => { get: () => baseUrl }); - if (options.stream && options.json) { - options.json = false; - } - - if (options.decompress && is.undefined(options.headers['accept-encoding'])) { - options.headers['accept-encoding'] = 'gzip, deflate'; - } - const {query} = options; - if (!is.empty(query) || query instanceof URLSearchParamsGlobal) { const queryParams = new URLSearchParamsGlobal(query); options.path = `${options.path.split('?')[0]}?${queryParams.toString()}`; delete options.query; } + if (options.stream && options.json) { + options.json = false; + } + if (options.json && is.undefined(options.headers.accept)) { options.headers.accept = 'application/json'; } @@ -125,6 +166,10 @@ module.exports = (url, options, defaults) => { options.method = options.method.toUpperCase(); + if (options.decompress && is.undefined(options.headers['accept-encoding'])) { + options.headers['accept-encoding'] = 'gzip, deflate'; + } + if (options.hostname === 'unix') { const matches = /(.+?):(.+)/.exec(options.path); @@ -164,6 +209,15 @@ module.exports = (url, options, defaults) => { } } + if (is.number(options.timeout) || is.object(options.timeout)) { + if (is.number(options.timeout)) { + options.gotTimeout = {request: options.timeout}; + } else { + options.gotTimeout = options.timeout; + } + delete options.timeout; + } + if (!is.function(options.gotRetry.retries)) { const {retries} = options.gotRetry; @@ -197,42 +251,7 @@ module.exports = (url, options, defaults) => { }; } - if (is.undefined(options.followRedirect)) { - options.followRedirect = true; - } - - if (is.number(options.timeout) || is.object(options.timeout)) { - if (is.number(options.timeout)) { - options.gotTimeout = {request: options.timeout}; - } else { - options.gotTimeout = options.timeout; - } - delete options.timeout; - } - - if (is.nullOrUndefined(options.hooks)) { - options.hooks = {}; - } - if (is.object(options.hooks)) { - for (const hookEvent of knownHookEvents) { - const hooks = options.hooks[hookEvent]; - if (is.nullOrUndefined(hooks)) { - options.hooks[hookEvent] = []; - } else if (is.array(hooks)) { - for (const [index, hook] of hooks.entries()) { - if (!is.function(hook)) { - throw new TypeError( - `Parameter \`hooks.${hookEvent}[${index}]\` must be a function, not ${is(hook)}` - ); - } - } - } else { - throw new TypeError(`Parameter \`hooks.${hookEvent}\` must be an array, not ${is(hooks)}`); - } - } - } else { - throw new TypeError(`Parameter \`hooks\` must be an object, not ${is(options.hooks)}`); - } - return options; }; + +module.exports.prenormalize = prenormalize; diff --git a/test/arguments.js b/test/arguments.js index c24af4678..69eaa365e 100644 --- a/test/arguments.js +++ b/test/arguments.js @@ -22,6 +22,10 @@ test.before('setup', async () => { response.end(request.url); }); + s.on('/test/foobar', (request, response) => { + response.end(request.url); + }); + s.on('/stream', (request, response) => { response.end('ok'); }); @@ -181,6 +185,18 @@ test('allows extra keys in `hooks`', async t => { await t.notThrowsAsync(() => got(`${s.url}/test`, {hooks: {extra: {}}})); }); +test('baseUrl works', async t => { + const instanceA = got.extend({baseUrl: `${s.url}/test`}); + const {body} = await instanceA('/foobar'); + t.is(body, `/test/foobar`); +}); + +test('backslash is optional (baseUrl)', async t => { + const instanceA = got.extend({baseUrl: `${s.url}/test/`}); + const {body} = await instanceA('/foobar'); + t.is(body, `/test/foobar`); +}); + test('throws when trying to modify baseUrl after options got normalized', async t => { const instanceA = got.create({ methods: [], diff --git a/test/create.js b/test/create.js index 4e688b8b5..a11aa3350 100644 --- a/test/create.js +++ b/test/create.js @@ -97,7 +97,7 @@ test('extend keeps the old value if the new one is undefined', t => { test('extend merges URL instances', t => { const a = got.extend({baseUrl: new URL('https://example.com')}); const b = a.extend({baseUrl: '/foo'}); - t.is(b.defaults.options.baseUrl.toString(), 'https://example.com/foo'); + t.is(b.defaults.options.baseUrl.toString(), 'https://example.com/foo/'); }); test('create', async t => { @@ -128,7 +128,7 @@ test('no tampering with defaults', t => { const instance = got.create({ handler: got.defaults.handler, options: got.mergeOptions(got.defaults.options, { - baseUrl: 'example' + baseUrl: 'example/' }) }); @@ -142,8 +142,8 @@ test('no tampering with defaults', t => { instance.defaults.options.baseUrl = 'http://google.com'; }); - t.is(instance.defaults.options.baseUrl, 'example'); - t.is(instance2.defaults.options.baseUrl, 'example'); + t.is(instance.defaults.options.baseUrl, 'example/'); + t.is(instance2.defaults.options.baseUrl, 'example/'); }); test('only plain objects are freezed', async t => { From f2278b62f401498c9a45a027eac0013ecd930520 Mon Sep 17 00:00:00 2001 From: "U-DESKTOP-07IKGBD\\Szymon Marczak" Date: Thu, 23 Aug 2018 12:56:15 +0200 Subject: [PATCH 2/7] Nitpicks --- readme.md | 11 +++++++++++ source/create.js | 2 +- source/normalize-arguments.js | 6 +++--- test/arguments.js | 14 +++++++++++++- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/readme.md b/readme.md index 4651e6ae9..fac0a8f3a 100644 --- a/readme.md +++ b/readme.md @@ -122,6 +122,17 @@ Very useful when used with `got.extend()` to create niche-specific Got instances Can be a string or a [WHATWG `URL`](https://nodejs.org/api/url.html#url_class_url). +Examples: +- **baseUrl**: `https://example.com/v1`
+ **url**: `hello`
+ **result**: `https://example.com/v1/hello` +- **baseUrl**: `https://example.com/v1`
+ **url**: `/hello`
+ **result**: `https://example.com/v1/hello` +- **baseUrl**: `https://example.com/v1/`
+ **url**: `/hello`
+ **result**: `https://example.com/v1/hello` + ###### headers Type: `Object`
diff --git a/source/create.js b/source/create.js index 02833b04c..cbe32d033 100644 --- a/source/create.js +++ b/source/create.js @@ -21,7 +21,7 @@ const aliases = [ const create = defaults => { defaults = merge({}, defaults); - defaults.options = normalizeArguments.prenormalize(defaults.options); + defaults.options = normalizeArguments.preNormalize(defaults.options); if (!defaults.handler) { defaults.handler = next; } diff --git a/source/normalize-arguments.js b/source/normalize-arguments.js index 40bfb261a..216e41d9d 100644 --- a/source/normalize-arguments.js +++ b/source/normalize-arguments.js @@ -13,7 +13,7 @@ const knownHookEvents = require('./known-hook-events'); const retryAfterStatusCodes = new Set([413, 429, 503]); -const prenormalize = options => { +const preNormalize = options => { options = { headers: {}, ...options @@ -63,7 +63,7 @@ module.exports = (url, options, defaults) => { throw new TypeError(`Parameter \`url\` must be a string or object, not ${is(url)}`); } - options = prenormalize(options); + options = preNormalize(options); if (is.string(url)) { if (options.baseUrl) { @@ -254,4 +254,4 @@ module.exports = (url, options, defaults) => { return options; }; -module.exports.prenormalize = prenormalize; +module.exports.preNormalize = preNormalize; diff --git a/test/arguments.js b/test/arguments.js index 36ef30c50..ff8b2c31a 100644 --- a/test/arguments.js +++ b/test/arguments.js @@ -204,12 +204,24 @@ test('baseUrl works', async t => { t.is(body, `/test/foobar`); }); -test('backslash is optional (baseUrl)', async t => { +test('accepts WHATWG URL as the baseUrl option', async t => { + const instanceA = got.extend({baseUrl: new URL(`${s.url}/test`)}); + const {body} = await instanceA('/foobar'); + t.is(body, `/test/foobar`); +}); + +test('backslash in the end of `baseUrl` is optional', async t => { const instanceA = got.extend({baseUrl: `${s.url}/test/`}); const {body} = await instanceA('/foobar'); t.is(body, `/test/foobar`); }); +test('backslash in the beginning of `url` is optional when using baseUrl', async t => { + const instanceA = got.extend({baseUrl: `${s.url}/test`}); + const {body} = await instanceA('foobar'); + t.is(body, `/test/foobar`); +}); + test('throws when trying to modify baseUrl after options got normalized', async t => { const instanceA = got.create({ methods: [], From 1af5cde4349d964e821c0649e8ee775d2a571398 Mon Sep 17 00:00:00 2001 From: Szymon Marczak <36894700+szmarczak@users.noreply.github.com> Date: Thu, 23 Aug 2018 13:02:58 +0200 Subject: [PATCH 3/7] Update readme.md --- readme.md | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/readme.md b/readme.md index fac0a8f3a..e15d63abb 100644 --- a/readme.md +++ b/readme.md @@ -122,16 +122,14 @@ Very useful when used with `got.extend()` to create niche-specific Got instances Can be a string or a [WHATWG `URL`](https://nodejs.org/api/url.html#url_class_url). -Examples: -- **baseUrl**: `https://example.com/v1`
- **url**: `hello`
- **result**: `https://example.com/v1/hello` -- **baseUrl**: `https://example.com/v1`
- **url**: `/hello`
- **result**: `https://example.com/v1/hello` -- **baseUrl**: `https://example.com/v1/`
- **url**: `/hello`
- **result**: `https://example.com/v1/hello` +Backslash in the end of `baseUrl` is optional.
+Backslash in the beginning of `url` argument is optional too. + +Example: + +```js +await got('hello', {baseUrl: 'https://example.com/v1'}); // => https://example.com/v1/hello +``` ###### headers From f07409437cc505d0a55f9107e5c71cb542e46dd9 Mon Sep 17 00:00:00 2001 From: Sindre Sorhus Date: Thu, 23 Aug 2018 18:08:55 +0700 Subject: [PATCH 4/7] Update readme.md --- readme.md | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/readme.md b/readme.md index e15d63abb..28db6aa8e 100644 --- a/readme.md +++ b/readme.md @@ -122,13 +122,17 @@ Very useful when used with `got.extend()` to create niche-specific Got instances Can be a string or a [WHATWG `URL`](https://nodejs.org/api/url.html#url_class_url). -Backslash in the end of `baseUrl` is optional.
-Backslash in the beginning of `url` argument is optional too. - -Example: +Backslash at the end of `baseUrl` and at the beginning of the `url` argument is optional: ```js -await got('hello', {baseUrl: 'https://example.com/v1'}); // => https://example.com/v1/hello +await got('hello', {baseUrl: 'https://example.com/v1'}); +//=> 'https://example.com/v1/hello' + +await got('/hello', {baseUrl: 'https://example.com/v1/'}); +//=> 'https://example.com/v1/hello' + +await got('/hello', {baseUrl: 'https://example.com/v1'}); +//=> 'https://example.com/v1/hello' ``` ###### headers From fbf4174e73d709b4c9cf4422a910cd0e388d0185 Mon Sep 17 00:00:00 2001 From: Szymon Marczak <36894700+szmarczak@users.noreply.github.com> Date: Thu, 23 Aug 2018 13:24:11 +0200 Subject: [PATCH 5/7] Update normalize-arguments.js --- source/normalize-arguments.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/source/normalize-arguments.js b/source/normalize-arguments.js index 216e41d9d..e01a5ce2d 100644 --- a/source/normalize-arguments.js +++ b/source/normalize-arguments.js @@ -13,6 +13,8 @@ const knownHookEvents = require('./known-hook-events'); const retryAfterStatusCodes = new Set([413, 429, 503]); +// `preNormalize` handles all the stuff which is related with static options like `baseUrl`, `followRedirect`, `hooks` etc. +// While `normalize` does `preNormalize` + handles all the stuff related with dynamic options like url, headers, body etc. const preNormalize = options => { options = { headers: {}, From 2e17859230459b46a317d27de6c6dc2c3f24c6c5 Mon Sep 17 00:00:00 2001 From: Sindre Sorhus Date: Thu, 23 Aug 2018 18:26:42 +0700 Subject: [PATCH 6/7] Update normalize-arguments.js --- source/normalize-arguments.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/normalize-arguments.js b/source/normalize-arguments.js index e01a5ce2d..554c4283d 100644 --- a/source/normalize-arguments.js +++ b/source/normalize-arguments.js @@ -13,8 +13,8 @@ const knownHookEvents = require('./known-hook-events'); const retryAfterStatusCodes = new Set([413, 429, 503]); -// `preNormalize` handles all the stuff which is related with static options like `baseUrl`, `followRedirect`, `hooks` etc. -// While `normalize` does `preNormalize` + handles all the stuff related with dynamic options like url, headers, body etc. +// `preNormalize` handles things related to static options, like `baseUrl`, `followRedirect`, `hooks`, etc. +// While `normalize` does `preNormalize` + handles things related to dynamic options, like URL, headers, body, etc. const preNormalize = options => { options = { headers: {}, From ca78716ae73a0cab7bcd27ea9ac66ef41c871359 Mon Sep 17 00:00:00 2001 From: "U-DESKTOP-07IKGBD\\Szymon Marczak" Date: Thu, 23 Aug 2018 13:34:55 +0200 Subject: [PATCH 7/7] Keep coverage at 100% --- source/merge-instances.js | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/source/merge-instances.js b/source/merge-instances.js index 94ab001e1..9899c9551 100644 --- a/source/merge-instances.js +++ b/source/merge-instances.js @@ -12,17 +12,11 @@ module.exports = (instances, methods) => { options = merge({}, options, instance.defaults.options); const instanceHooks = instance.defaults.options.hooks; - if (instanceHooks) { - for (const name of knownHookEvents) { - if (!instanceHooks[name]) { - continue; - } - - if (hooks[name]) { - hooks[name] = hooks[name].concat(instanceHooks[name]); - } else { - hooks[name] = [...instanceHooks[name]]; - } + for (const name of knownHookEvents) { + if (hooks[name]) { + hooks[name] = hooks[name].concat(instanceHooks[name]); + } else { + hooks[name] = [...instanceHooks[name]]; } } }