Skip to content

Commit

Permalink
use lodash to merge options and document assignOptions behaviors
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Jonathan Stewmon committed Jul 25, 2018
1 parent da7f055 commit 2a9611f
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 28 deletions.
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
21 changes: 13 additions & 8 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -359,10 +359,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.<br>
Check out the [unchanged default values](source/index.js).

```js
const client = got.extend({
Expand Down Expand Up @@ -408,14 +406,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
Expand Down
47 changes: 28 additions & 19 deletions source/assign-options.js
Original file line number Diff line number Diff line change
@@ -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;
};
}
8 changes: 7 additions & 1 deletion source/normalize-arguments.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
31 changes: 31 additions & 0 deletions test/create.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import {URL} from 'url';
import test from 'ava';
import got from '../source';
import {createServer} from './helpers/server';
Expand Down Expand Up @@ -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: {},
Expand Down

0 comments on commit 2a9611f

Please sign in to comment.