Make got.mergeOptions() behavior more obvious and document its behavior (#538)

Author: jstewmon

advanced-creation.md

@@ -11,7 +11,7 @@ Configure a new `got` instance with the provided settings.<br>
 
 ##### [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).<br>
+To inherit from parent, set it as `got.defaults.options` or use [`got.mergeOptions(defaults.options, options)`](readme.md#gotmergeOptionsparentoptions-newoptions).<br>
 **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'
 		}

package.json

@@ -38,7 +38,6 @@
 		"cacheable-request": "^4.0.1",
 		"decompress-response": "^3.3.0",
 		"duplexer3": "^0.1.4",
-		"extend": "^3.0.1",
 		"get-stream": "^3.0.0",
 		"mimic-response": "^1.0.0",
 		"p-cancelable": "^0.5.0",

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.mergeOptions`](#gotmergeoptionsparentoptions-newoptions).
 
-**Note:** You can extend another extended instance. `got.defaults` provides settings used by that instance.<br>
-Check out the [unchanged default values](source/index.js).
 
 ```js
 const client = got.extend({
@@ -405,16 +403,28 @@ 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.**
 
-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:
+#### 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`.
+- 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'}};
-const b = {headers: {dog: 'woof'}};
+const a = {headers: {cat: 'meow', habitat: ['house', 'alley']}};
+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.mergeOptions(a, b) // => {headers: {cat: 'meow', cow: 'moo', habitat: ['barn']}}
 ```
 
 ## Errors

source/assign-options.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,

source/create.js

@@ -0,0 +1,36 @@
+const {URL} = require('url');
+const is = require('@sindresorhus/is');
+
+module.exports = (defaults, options = {}) => {
+	return merge({}, defaults, options);
+};
+
+function merge(target, ...sources) {
+	for (const source of sources) {
+		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.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);
+			} else if (is.plainObject(sourceValue)) {
+				if (is.plainObject(targetValue)) {
+					target[key] = merge({}, targetValue, sourceValue);
+				} else {
+					target[key] = merge({}, sourceValue);
+				}
+			} else {
+				target[key] = sourceValue;
+			}
+		}
+	}
+	return target;
+}

source/merge-options.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.nullOrUndefined(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');

source/normalize-arguments.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,42 @@ test('custom headers (extend)', async t => {
 	t.is(headers.unicorn, 'rainbow');
 });
 
+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);
+});
+
+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}
+	});
+	const b = a.extend({headers: {foo: undefined}});
+	t.deepEqual(
+		b.defaults.options.headers,
+		got.defaults.options.headers
+	);
+});
+
+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 ignores object values set to undefined (root keys)', t => {
+	t.true(Reflect.has(got.defaults.options, 'headers'));
+	const a = got.extend({headers: undefined});
+	t.deepEqual(a.defaults.options, got.defaults.options);
+});
+
 test('create', async t => {
 	const instance = got.create({
 		options: {},
@@ -105,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'
 		})
 	});

test/create.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'));
 });