From 26fdae4179cd8cee5b1f30bcf064cc477cee6442 Mon Sep 17 00:00:00 2001 From: Jonathan Stewmon Date: Wed, 25 Jul 2018 17:52:29 -0500 Subject: [PATCH 1/7] use lodash to merge options and document assignOptions behaviors Option merging is now consistent, so null header removal is done during options normalization. null could not be used to indicate property removal because null is a significant value for some settings. --- package.json | 2 ++ readme.md | 21 ++++++++++------ source/assign-options.js | 47 +++++++++++++++++++++-------------- source/normalize-arguments.js | 8 +++++- test/create.js | 31 +++++++++++++++++++++++ 5 files changed, 81 insertions(+), 28 deletions(-) diff --git a/package.json b/package.json index 14e56b9ab..c6652eb11 100644 --- a/package.json +++ b/package.json @@ -40,6 +40,8 @@ "duplexer3": "^0.1.4", "extend": "^3.0.1", "get-stream": "^3.0.0", + "lodash.clonedeep": "^4.5.0", + "lodash.mergewith": "^4.6.1", "mimic-response": "^1.0.0", "p-cancelable": "^0.5.0", "to-readable-stream": "^1.0.0", diff --git a/readme.md b/readme.md index d1795528e..8496783d8 100644 --- a/readme.md +++ b/readme.md @@ -358,10 +358,8 @@ Sets `options.method` to the method name and makes a request. #### got.extend([options]) -Configure a new `got` instance with default `options` and (optionally) a custom `baseUrl`: +Configure a new `got` instance with default `options`. `options` are merged with the extended instance's `defaults.options` as described in [`got.assignOptions`](#got.assignoptionsparentoptions,-newoptions). -**Note:** You can extend another extended instance. `got.defaults` provides settings used by that instance.
-Check out the [unchanged default values](source/index.js). ```js const client = got.extend({ @@ -407,14 +405,21 @@ client.get('/demo'); #### got.assignOptions(parentOptions, newOptions) -Extends parent options. Avoid using [object spread](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax#Spread_in_object_literals) as it doesn't work recursively: +Extends parent options. Options are deeply merged to a new object as follows: + +- If either value is `null`, a primitive, or an `Array`, the result is the newOptions value. +- If the parentOptions value is an instance of `URL`, the resulting value is the result of `new URL(new, parent)`. +- If the newOptions value is `undefined` the key is removed. +- If the newOptions value is an `Object`, its values are merged recursively. + +Avoid using [object spread](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax#Spread_in_object_literals) as it doesn't work recursively: ```js -const a = {headers: {cat: 'meow'}}; -const b = {headers: {dog: 'woof'}}; +const a = {headers: {cat: 'meow', habitat: ['house']}}; +const b = {headers: {cow: 'moo', habitat: ['barn']}}; -{...a, ...b} // => {headers: {dog: 'woof'}} -got.assignOptions(a, b) // => {headers: {cat: 'meow', dog: 'woof'}} +{...a, ...b} // => {headers: {cow: 'moo'}} +got.assignOptions(a, b) // => {headers: {cat: 'meow', dog: 'woof', habitat: ['barn']}} ``` ## Errors diff --git a/source/assign-options.js b/source/assign-options.js index 176876ce2..6dbacd369 100644 --- a/source/assign-options.js +++ b/source/assign-options.js @@ -1,27 +1,36 @@ -const extend = require('extend'); +const url = require('url'); +const mergeWith = require('lodash.mergewith'); +const cloneDeep = require('lodash.clonedeep'); const is = require('@sindresorhus/is'); module.exports = (defaults, options = {}) => { - const returnOptions = extend(true, {}, defaults, options); + return mergeWith( + {opts: cloneDeep(defaults)}, + {opts: options}, + customizer + ).opts; +}; - if (Reflect.has(options, 'headers')) { - for (const [key, value] of Object.entries(options.headers)) { - if (is.nullOrUndefined(value)) { - delete returnOptions.headers[key]; - } - } +function customizer(objValue, srcValue) { + if (is.array(srcValue) || is.array(objValue)) { + return cloneDeep(srcValue); } - - // Override these arrays because we don't want to extend them - if (is.object(options.retry)) { - if (Reflect.has(options.retry, 'methods')) { - returnOptions.retry.methods = options.retry.methods; + if (objValue instanceof url.URL) { + return new url.URL(srcValue, objValue); + } + if (![objValue, srcValue].some(is.array) && [objValue, srcValue].every(is.object)) { + // When both args are non-array objects, delete keys for which the source + // value is undefined (null is a significant value places, e.g. `encoding`). + const deleteKeys = []; + for (const key in srcValue) { + if (is.undefined(srcValue[key])) { + deleteKeys.push(key); + } } - - if (Reflect.has(options.retry, 'statusCodes')) { - returnOptions.retry.statusCodes = options.retry.statusCodes; + const result = mergeWith(objValue, srcValue, customizer); + for (const key of deleteKeys) { + delete result[key]; } + return result; } - - return returnOptions; -}; +} diff --git a/source/normalize-arguments.js b/source/normalize-arguments.js index b890f0a97..f21d9dacc 100644 --- a/source/normalize-arguments.js +++ b/source/normalize-arguments.js @@ -67,11 +67,17 @@ module.exports = (url, options, defaults) => { options.headers.accept = 'application/json'; } + const {headers} = options; + for (const [key, value] of Object.entries(headers)) { + if (is.null(value)) { + delete headers[key]; + } + } + const {body} = options; if (is.nullOrUndefined(body)) { options.method = (options.method || 'GET').toUpperCase(); } else { - const {headers} = options; const isObject = is.object(body) && !Buffer.isBuffer(body) && !is.nodeStream(body); if (!is.nodeStream(body) && !is.string(body) && !is.buffer(body) && !(options.form || options.json)) { throw new TypeError('The `body` option must be a stream.Readable, string or Buffer'); diff --git a/test/create.js b/test/create.js index 17ccc2b3f..64d190729 100644 --- a/test/create.js +++ b/test/create.js @@ -1,3 +1,4 @@ +import {URL} from 'url'; import test from 'ava'; import got from '../source'; import {createServer} from './helpers/server'; @@ -76,6 +77,36 @@ test('custom headers (extend)', async t => { t.is(headers.unicorn, 'rainbow'); }); +test('extend overwrites arrays with copy', t => { + const statusCodes = [408]; + const a = got.extend({retry: {statusCodes}}); + t.deepEqual(a.defaults.options.retry.statusCodes, statusCodes); + t.not(a.defaults.options.retry.statusCodes, statusCodes); +}); + +test('extend removes object values set to undefined', t => { + const a = got.extend({ + headers: {foo: 'foo', bar: 'bar', baz: 'baz'} + }); + const b = a.extend({headers: {foo: undefined, bar: null}}); + t.deepEqual( + b.defaults.options.headers, + {...got.defaults.options.headers, ...{baz: 'baz', bar: null}} + ); +}); + +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'); +}); + +test('extend removes object values set to undefined (root keys)', t => { + t.true('headers' in got.defaults.options); + const a = got.extend({headers: undefined}); + t.false('headers' in a.defaults.options); +}); + test('create', async t => { const instance = got.create({ options: {}, From 3bf4693b899e2a6d310dd37a1dbcb2546eec701e Mon Sep 17 00:00:00 2001 From: Jonathan Stewmon Date: Wed, 25 Jul 2018 23:17:45 -0500 Subject: [PATCH 2/7] refactor: use our own merge instead of lodash We have a narrow set of rules for merging objects and lodash's more aggressive merge may have unintended consequences when the values being merged are not plain objects. --- package.json | 3 --- readme.md | 14 ++++++------ source/assign-options.js | 46 +++++++++++++++++----------------------- test/create.js | 4 ++-- 4 files changed, 30 insertions(+), 37 deletions(-) diff --git a/package.json b/package.json index c6652eb11..2062d2bc8 100644 --- a/package.json +++ b/package.json @@ -38,10 +38,7 @@ "cacheable-request": "^4.0.1", "decompress-response": "^3.3.0", "duplexer3": "^0.1.4", - "extend": "^3.0.1", "get-stream": "^3.0.0", - "lodash.clonedeep": "^4.5.0", - "lodash.mergewith": "^4.6.1", "mimic-response": "^1.0.0", "p-cancelable": "^0.5.0", "to-readable-stream": "^1.0.0", diff --git a/readme.md b/readme.md index 8496783d8..4a9a20d03 100644 --- a/readme.md +++ b/readme.md @@ -405,12 +405,14 @@ client.get('/demo'); #### got.assignOptions(parentOptions, newOptions) -Extends parent options. Options are deeply merged to a new object as follows: - -- If either value is `null`, a primitive, or an `Array`, the result is the newOptions value. -- If the parentOptions value is an instance of `URL`, the resulting value is the result of `new URL(new, parent)`. -- If the newOptions value is `undefined` the key is removed. -- If the newOptions value is an `Object`, its values are merged recursively. +Extends parent options. Options are deeply merged to a new object. The value of each key is determined as follows: + +- If the newOptions value is `undefined` the key is omitted from the result. +- If the parentOptions value is an instance of `URL` and the newOptions value is a `string` or `URL`, a new URL instance is created with the p `new URL(new, parent)`. +- If the newOptions value is a plain `Object` + - If the parentOptions value is a plain `Object`, both values are merged recursively into a new `Object`. + - Otherwise, only the newOptions value is merged recurively into a new `Object`. +- Otherwise, the newOptions value is assigned to the key. Avoid using [object spread](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax#Spread_in_object_literals) as it doesn't work recursively: diff --git a/source/assign-options.js b/source/assign-options.js index 6dbacd369..6c369556e 100644 --- a/source/assign-options.js +++ b/source/assign-options.js @@ -1,36 +1,30 @@ const url = require('url'); -const mergeWith = require('lodash.mergewith'); -const cloneDeep = require('lodash.clonedeep'); const is = require('@sindresorhus/is'); module.exports = (defaults, options = {}) => { - return mergeWith( - {opts: cloneDeep(defaults)}, - {opts: options}, - customizer - ).opts; + return merge({}, defaults, options); }; -function customizer(objValue, srcValue) { - if (is.array(srcValue) || is.array(objValue)) { - return cloneDeep(srcValue); - } - if (objValue instanceof url.URL) { - return new url.URL(srcValue, objValue); - } - if (![objValue, srcValue].some(is.array) && [objValue, srcValue].every(is.object)) { - // When both args are non-array objects, delete keys for which the source - // value is undefined (null is a significant value places, e.g. `encoding`). - const deleteKeys = []; - for (const key in srcValue) { - if (is.undefined(srcValue[key])) { - deleteKeys.push(key); +function merge(target = {}, ...sources) { + for (const source of sources) { + for (const [key, sourceValue] of Object.entries(source)) { + const targetValue = target[key]; + if (is.undefined(sourceValue)) { + delete target[key]; + } else if (is.urlInstance(targetValue) && ( + is.urlInstance(sourceValue) || is.string(sourceValue) + )) { + target[key] = new url.URL(sourceValue, targetValue); + } else if (is.plainObject(sourceValue)) { + if (is.plainObject(targetValue)) { + target[key] = merge({}, targetValue, sourceValue); + } else { + target[key] = merge({}, sourceValue); + } + } else { + target[key] = sourceValue; } } - const result = mergeWith(objValue, srcValue, customizer); - for (const key of deleteKeys) { - delete result[key]; - } - return result; } + return target; } diff --git a/test/create.js b/test/create.js index 64d190729..b58845c66 100644 --- a/test/create.js +++ b/test/create.js @@ -77,11 +77,11 @@ test('custom headers (extend)', async t => { t.is(headers.unicorn, 'rainbow'); }); -test('extend overwrites arrays with copy', t => { +test('extend overwrites arrays', t => { const statusCodes = [408]; const a = got.extend({retry: {statusCodes}}); t.deepEqual(a.defaults.options.retry.statusCodes, statusCodes); - t.not(a.defaults.options.retry.statusCodes, statusCodes); + t.is(a.defaults.options.retry.statusCodes, statusCodes); }); test('extend removes object values set to undefined', t => { From a40b948750265ac5fb19fc9fa0843f7d8092ddcc Mon Sep 17 00:00:00 2001 From: Jonathan Stewmon Date: Thu, 26 Jul 2018 09:31:35 -0500 Subject: [PATCH 3/7] refactor: dereference URL on import, remove pointless target default --- source/assign-options.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/source/assign-options.js b/source/assign-options.js index 6c369556e..7b25cf2b7 100644 --- a/source/assign-options.js +++ b/source/assign-options.js @@ -1,11 +1,11 @@ -const url = require('url'); +const {URL} = require('url'); const is = require('@sindresorhus/is'); module.exports = (defaults, options = {}) => { return merge({}, defaults, options); }; -function merge(target = {}, ...sources) { +function merge(target, ...sources) { for (const source of sources) { for (const [key, sourceValue] of Object.entries(source)) { const targetValue = target[key]; @@ -14,7 +14,7 @@ function merge(target = {}, ...sources) { } else if (is.urlInstance(targetValue) && ( is.urlInstance(sourceValue) || is.string(sourceValue) )) { - target[key] = new url.URL(sourceValue, targetValue); + target[key] = new URL(sourceValue, targetValue); } else if (is.plainObject(sourceValue)) { if (is.plainObject(targetValue)) { target[key] = merge({}, targetValue, sourceValue); From ecbb640c05e6b0a6d1a958c7faa65dfdeb59813d Mon Sep 17 00:00:00 2001 From: Jonathan Stewmon Date: Fri, 27 Jul 2018 14:17:31 -0500 Subject: [PATCH 4/7] feat: ignore undefined values in assignOptions --- readme.md | 18 +++++++++--------- source/assign-options.js | 5 +++-- source/normalize-arguments.js | 2 +- test/create.js | 12 ++++++------ test/headers.js | 4 ++-- 5 files changed, 21 insertions(+), 20 deletions(-) diff --git a/readme.md b/readme.md index 4a9a20d03..ae8d9b8e5 100644 --- a/readme.md +++ b/readme.md @@ -405,23 +405,23 @@ client.get('/demo'); #### got.assignOptions(parentOptions, newOptions) -Extends parent options. Options are deeply merged to a new object. The value of each key is determined as follows: +Extends parent options. The options objects are deeply merged to a new object. The value of each property is determined as follows: -- If the newOptions value is `undefined` the key is omitted from the result. -- If the parentOptions value is an instance of `URL` and the newOptions value is a `string` or `URL`, a new URL instance is created with the p `new URL(new, parent)`. -- If the newOptions value is a plain `Object` - - If the parentOptions value is a plain `Object`, both values are merged recursively into a new `Object`. - - Otherwise, only the newOptions value is merged recurively into a new `Object`. -- Otherwise, the newOptions value is assigned to the key. +- If the new value is `undefined` the parent value is preserved. +- If the parent value is an instance of `URL` and the new value is a `string` or `URL`, a new URL instance is created, using the parent value as the base: `new URL(new, parent)`. +- If the new value is a plain `Object` + - If the parent value is a plain `Object`, both values are merged recursively into a new `Object`. + - Otherwise, only the new value is merged recursively into a new `Object`. +- Otherwise, the new value is assigned to the property. Avoid using [object spread](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax#Spread_in_object_literals) as it doesn't work recursively: ```js -const a = {headers: {cat: 'meow', habitat: ['house']}}; +const a = {headers: {cat: 'meow', habitat: ['house', 'alley']}}; const b = {headers: {cow: 'moo', habitat: ['barn']}}; {...a, ...b} // => {headers: {cow: 'moo'}} -got.assignOptions(a, b) // => {headers: {cat: 'meow', dog: 'woof', habitat: ['barn']}} +got.assignOptions(a, b) // => {headers: {cat: 'meow', cow: 'moo', habitat: ['barn']}} ``` ## Errors diff --git a/source/assign-options.js b/source/assign-options.js index 7b25cf2b7..58d0af3fa 100644 --- a/source/assign-options.js +++ b/source/assign-options.js @@ -10,8 +10,9 @@ function merge(target, ...sources) { for (const [key, sourceValue] of Object.entries(source)) { const targetValue = target[key]; if (is.undefined(sourceValue)) { - delete target[key]; - } else if (is.urlInstance(targetValue) && ( + continue; + } + if (is.urlInstance(targetValue) && ( is.urlInstance(sourceValue) || is.string(sourceValue) )) { target[key] = new URL(sourceValue, targetValue); diff --git a/source/normalize-arguments.js b/source/normalize-arguments.js index f21d9dacc..d22436649 100644 --- a/source/normalize-arguments.js +++ b/source/normalize-arguments.js @@ -69,7 +69,7 @@ module.exports = (url, options, defaults) => { const {headers} = options; for (const [key, value] of Object.entries(headers)) { - if (is.null(value)) { + if (is.nullOrUndefined(value)) { delete headers[key]; } } diff --git a/test/create.js b/test/create.js index b58845c66..d95832102 100644 --- a/test/create.js +++ b/test/create.js @@ -84,14 +84,14 @@ test('extend overwrites arrays', t => { t.is(a.defaults.options.retry.statusCodes, statusCodes); }); -test('extend removes object values set to undefined', t => { +test('extend ignores object values set to undefined', t => { const a = got.extend({ - headers: {foo: 'foo', bar: 'bar', baz: 'baz'} + headers: {foo: undefined, 'user-agent': undefined} }); - const b = a.extend({headers: {foo: undefined, bar: null}}); + const b = a.extend({headers: {foo: undefined}}); t.deepEqual( b.defaults.options.headers, - {...got.defaults.options.headers, ...{baz: 'baz', bar: null}} + got.defaults.options.headers ); }); @@ -101,10 +101,10 @@ test('extend merges URL instances', t => { t.is(b.defaults.options.baseUrl.toString(), 'https://example.com/foo'); }); -test('extend removes object values set to undefined (root keys)', t => { +test('extend ignores object values set to undefined (root keys)', t => { t.true('headers' in got.defaults.options); const a = got.extend({headers: undefined}); - t.false('headers' in a.defaults.options); + t.deepEqual(a.defaults.options, got.defaults.options); }); test('create', async t => { diff --git a/test/headers.js b/test/headers.js index 80b39e22d..c95a726d5 100644 --- a/test/headers.js +++ b/test/headers.js @@ -145,9 +145,9 @@ test('remove null value headers', async t => { test('remove undefined value headers', async t => { const {body} = await got(s.url, { headers: { - 'user-agent': undefined + foo: undefined } }); const headers = JSON.parse(body); - t.false(Reflect.has(headers, 'user-agent')); + t.false(Reflect.has(headers, 'foo')); }); From 8acd5a7a4d9fab68fa300a72bab9e41f85fcd86d Mon Sep 17 00:00:00 2001 From: Jonathan Stewmon Date: Fri, 27 Jul 2018 21:51:55 -0500 Subject: [PATCH 5/7] chore: add test for extends null value --- test/create.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/create.js b/test/create.js index d95832102..6a9221f9d 100644 --- a/test/create.js +++ b/test/create.js @@ -84,7 +84,13 @@ test('extend overwrites arrays', t => { t.is(a.defaults.options.retry.statusCodes, statusCodes); }); -test('extend ignores object values set to undefined', t => { +test('extend overwrites null', t => { + const statusCodes = null; + const a = got.extend({retry: {statusCodes}}); + t.is(a.defaults.options.retry.statusCodes, statusCodes); +}); + +test('extend ignores source values set to undefined', t => { const a = got.extend({ headers: {foo: undefined, 'user-agent': undefined} }); From 27788f9e0a6bafa768ff1f534e0d5d667361d3ce Mon Sep 17 00:00:00 2001 From: Jonathan Stewmon Date: Sun, 29 Jul 2018 14:30:26 -0500 Subject: [PATCH 6/7] - recursively merge arrays to a new array - rename assignOptions to mergeOptions - update docs accordingly --- advanced-creation.md | 6 +++--- readme.md | 9 ++++++--- source/create.js | 10 +++++----- source/{assign-options.js => merge-options.js} | 9 +++++++-- test/create.js | 4 ++-- 5 files changed, 23 insertions(+), 15 deletions(-) rename source/{assign-options.js => merge-options.js} (71%) diff --git a/advanced-creation.md b/advanced-creation.md index b7a8993ea..e781a91e9 100644 --- a/advanced-creation.md +++ b/advanced-creation.md @@ -11,7 +11,7 @@ Configure a new `got` instance with the provided settings.
##### [options](readme.md#options) -To inherit from parent, set it as `got.defaults.options` or use [`got.assignOptions(defaults.options, options)`](readme.md#gotassignoptionsparentoptions-newoptions).
+To inherit from parent, set it as `got.defaults.options` or use [`got.mergeOptions(defaults.options, options)`](readme.md#gotmergeOptionsparentoptions-newoptions).
**Note**: Avoid using [object spread](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax#Spread_in_object_literals) as it doesn't work recursively. ##### methods @@ -54,7 +54,7 @@ const settings = { return next(options); }, methods: got.defaults.methods, - options: got.assignOptions(got.defaults.options, { + options: got.mergeOptions(got.defaults.options, { json: true }) }; @@ -99,7 +99,7 @@ const unchangedGot = got.create(defaults); const settings = { handler: got.defaults.handler, methods: got.defaults.methods, - options: got.assignOptions(got.defaults.options, { + options: got.mergeOptions(got.defaults.options, { headers: { unicorn: 'rainbow' } diff --git a/readme.md b/readme.md index ae8d9b8e5..5bfb09395 100644 --- a/readme.md +++ b/readme.md @@ -358,7 +358,7 @@ Sets `options.method` to the method name and makes a request. #### got.extend([options]) -Configure a new `got` instance with default `options`. `options` are merged with the extended instance's `defaults.options` as described in [`got.assignOptions`](#got.assignoptionsparentoptions,-newoptions). +Configure a new `got` instance with default `options`. `options` are merged with the extended instance's `defaults.options` as described in [`got.mergeOptions`](#got.mergeOptionsparentoptions,-newoptions). ```js @@ -403,12 +403,15 @@ client.get('/demo'); *Need more control over the behavior of Got? Check out the [`got.create()`](advanced-creation.md).* -#### got.assignOptions(parentOptions, newOptions) +**Both `got.extend(options)` and `got.create(options)` will freeze the instance's default options. For `got.extend()`, the instance's default options are the result of `got.mergeOptions`, which effectively copies plain `Object` and `Array` values. Therefore, you should treat objects passed to these methods as immutable.** + +#### got.mergeOptions(parentOptions, newOptions) Extends parent options. The options objects are deeply merged to a new object. The value of each property is determined as follows: - If the new value is `undefined` the parent value is preserved. - If the parent value is an instance of `URL` and the new value is a `string` or `URL`, a new URL instance is created, using the parent value as the base: `new URL(new, parent)`. +- If the new value is an `Array`, the new value is recursively merged into an empty array (the source value is discarded). `undefined` elements in the source array are not assigned during the merge, so the resulting array will have an empty item where the source array had an `undefined` item. - If the new value is a plain `Object` - If the parent value is a plain `Object`, both values are merged recursively into a new `Object`. - Otherwise, only the new value is merged recursively into a new `Object`. @@ -421,7 +424,7 @@ const a = {headers: {cat: 'meow', habitat: ['house', 'alley']}}; const b = {headers: {cow: 'moo', habitat: ['barn']}}; {...a, ...b} // => {headers: {cow: 'moo'}} -got.assignOptions(a, b) // => {headers: {cat: 'meow', cow: 'moo', habitat: ['barn']}} +got.mergeOptions(a, b) // => {headers: {cat: 'meow', cow: 'moo', habitat: ['barn']}} ``` ## Errors diff --git a/source/create.js b/source/create.js index 2d09394de..2cca4a7dd 100644 --- a/source/create.js +++ b/source/create.js @@ -1,6 +1,6 @@ 'use strict'; const errors = require('./errors'); -const assignOptions = require('./assign-options'); +const mergeOptions = require('./merge-options'); const asStream = require('./as-stream'); const asPromise = require('./as-promise'); const normalizeArguments = require('./normalize-arguments'); @@ -15,7 +15,7 @@ const create = defaults => { function got(url, options) { try { - options = assignOptions(defaults.options, options); + options = mergeOptions(defaults.options, options); return defaults.handler(normalizeArguments(url, options, defaults), next); } catch (error) { return Promise.reject(error); @@ -24,13 +24,13 @@ const create = defaults => { got.create = create; got.extend = (options = {}) => create({ - options: assignOptions(defaults.options, options), + options: mergeOptions(defaults.options, options), methods: defaults.methods, handler: defaults.handler }); got.stream = (url, options) => { - options = assignOptions(defaults.options, options); + options = mergeOptions(defaults.options, options); options.stream = true; return defaults.handler(normalizeArguments(url, options, defaults), next); }; @@ -40,7 +40,7 @@ const create = defaults => { got.stream[method] = (url, options) => got.stream(url, {...options, method}); } - Object.assign(got, {...errors, assignOptions}); + Object.assign(got, {...errors, mergeOptions}); Object.defineProperty(got, 'defaults', { value: deepFreeze(defaults), writable: false, diff --git a/source/assign-options.js b/source/merge-options.js similarity index 71% rename from source/assign-options.js rename to source/merge-options.js index 58d0af3fa..91d231c68 100644 --- a/source/assign-options.js +++ b/source/merge-options.js @@ -7,12 +7,17 @@ module.exports = (defaults, options = {}) => { function merge(target, ...sources) { for (const source of sources) { - for (const [key, sourceValue] of Object.entries(source)) { + const sourceIter = is.array(source) ? + source.entries() : + Object.entries(source); + for (const [key, sourceValue] of sourceIter) { const targetValue = target[key]; if (is.undefined(sourceValue)) { continue; } - if (is.urlInstance(targetValue) && ( + if (is.array(sourceValue)) { + target[key] = merge(new Array(sourceValue.length), sourceValue); + } else if (is.urlInstance(targetValue) && ( is.urlInstance(sourceValue) || is.string(sourceValue) )) { target[key] = new URL(sourceValue, targetValue); diff --git a/test/create.js b/test/create.js index 6a9221f9d..caab73c52 100644 --- a/test/create.js +++ b/test/create.js @@ -81,7 +81,7 @@ test('extend overwrites arrays', t => { const statusCodes = [408]; const a = got.extend({retry: {statusCodes}}); t.deepEqual(a.defaults.options.retry.statusCodes, statusCodes); - t.is(a.defaults.options.retry.statusCodes, statusCodes); + t.not(a.defaults.options.retry.statusCodes, statusCodes); }); test('extend overwrites null', t => { @@ -142,7 +142,7 @@ test('no tampering with defaults', t => { const instance = got.create({ handler: got.defaults.handler, methods: got.defaults.methods, - options: got.assignOptions(got.defaults.options, { + options: got.mergeOptions(got.defaults.options, { baseUrl: 'example' }) }); From 14526512e7a7204e1c482538908efbb9cb575d29 Mon Sep 17 00:00:00 2001 From: Sindre Sorhus Date: Tue, 31 Jul 2018 17:00:09 +0700 Subject: [PATCH 7/7] Update create.js --- test/create.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/create.js b/test/create.js index caab73c52..4004757b3 100644 --- a/test/create.js +++ b/test/create.js @@ -108,7 +108,7 @@ test('extend merges URL instances', t => { }); test('extend ignores object values set to undefined (root keys)', t => { - t.true('headers' in got.defaults.options); + t.true(Reflect.has(got.defaults.options, 'headers')); const a = got.extend({headers: undefined}); t.deepEqual(a.defaults.options, got.defaults.options); });