Skip to content

Commit

Permalink
refactor: use our own merge instead of lodash
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jstewmon committed Jul 26, 2018
1 parent 2a9611f commit d9e04cd
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 36 deletions.
2 changes: 0 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@
"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
14 changes: 8 additions & 6 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -406,12 +406,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:

Expand Down
46 changes: 20 additions & 26 deletions source/assign-options.js
Original file line number Diff line number Diff line change
@@ -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;
}
4 changes: 2 additions & 2 deletions test/create.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down

0 comments on commit d9e04cd

Please sign in to comment.