From 97833ed7336370cb2fad81a35699994e3ed18fa3 Mon Sep 17 00:00:00 2001 From: Aaron Stone Date: Wed, 28 Oct 2020 21:12:08 -0600 Subject: [PATCH 01/10] Apply clone option to target object --- index.js | 3 ++- test/merge.js | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/index.js b/index.js index 0ac71be..6f9ca17 100644 --- a/index.js +++ b/index.js @@ -52,7 +52,8 @@ function propertyIsUnsafe(target, key) { } function mergeObject(target, source, options) { - var destination = {} + // Modify target object if clone set to false + var destination = options.clone === false ? target : {} if (options.isMergeableObject(target)) { getKeys(target).forEach(function(key) { destination[key] = cloneUnlessOtherwiseSpecified(target[key], options) diff --git a/test/merge.js b/test/merge.js index fa2ac5b..e4b6032 100644 --- a/test/merge.js +++ b/test/merge.js @@ -1,6 +1,30 @@ var merge = require('../') var test = require('tape') +test('result should retain target type information when not cloning', function(t) { + var src = { key1: 'value1', key2: 'value2' } + class CustomType {} + var target = new CustomType() + + var res = merge(target, src, {clone: false}) + t.not(src instanceof CustomType) + t.assert(target instanceof CustomType) + t.assert(res instanceof CustomType) + t.end() +}) + +test('modify target object if clone set to false', function(t) { + var src = { key1: 'value1', key2: 'value2' } + var target = { key3: 'value3'} + + var clonedRes = merge(target, src, {clone: true}) + var notClonedRes = merge(target, src, {clone: false}) + + t.assert(clonedRes !== target, 'result should be cloned') + t.assert(notClonedRes === target, 'result should maintain target reference') + t.end() +}) + test('add keys in target that do not exist at the root', function(t) { var src = { key1: 'value1', key2: 'value2' } var target = {} From b9020e000cf7146217dce9da604ccf39608d9982 Mon Sep 17 00:00:00 2001 From: Aaron Stone Date: Thu, 29 Oct 2020 11:08:54 -0600 Subject: [PATCH 02/10] Apply target mutation to merge.all first array entry --- index.js | 18 +++++++++++++++--- test/merge.js | 12 ++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index 6f9ca17..0794239 100644 --- a/index.js +++ b/index.js @@ -4,6 +4,10 @@ function emptyTarget(val) { return Array.isArray(val) ? [] : {} } +function firstArrayEntry(arr) { + return arr[0] && arr[0].length ? arr : [] +} + function cloneUnlessOtherwiseSpecified(value, options) { return (options.clone !== false && options.isMergeableObject(value)) ? deepmerge(emptyTarget(value), value, options) @@ -51,9 +55,17 @@ function propertyIsUnsafe(target, key) { && Object.propertyIsEnumerable.call(target, key)) // and also unsafe if they're nonenumerable. } +// Retrieves either a new object or the appropriate target object to mutate. +function getDestinationObject(target, options) { + if (options && options.clone !== false) { + return {} + } + + return Array.isArray(target) ? firstArrayEntry(target) : target +} + function mergeObject(target, source, options) { - // Modify target object if clone set to false - var destination = options.clone === false ? target : {} + var destination = getDestinationObject(target, options) if (options.isMergeableObject(target)) { getKeys(target).forEach(function(key) { destination[key] = cloneUnlessOtherwiseSpecified(target[key], options) @@ -101,7 +113,7 @@ deepmerge.all = function deepmergeAll(array, options) { return array.reduce(function(prev, next) { return deepmerge(prev, next, options) - }, {}) + }, getDestinationObject(array, options)) } module.exports = deepmerge diff --git a/test/merge.js b/test/merge.js index e4b6032..5737f08 100644 --- a/test/merge.js +++ b/test/merge.js @@ -25,6 +25,18 @@ test('modify target object if clone set to false', function(t) { t.end() }) +test('merge.all mutates target object', function(t) { + var src = { key1: 'value1', key2: 'value2' } + var target = { key3: 'value3'} + + var clonedRes = merge.all([target, src], {clone: true}) + var notClonedRes = merge.all([target, src], {clone: false}) + + t.assert(clonedRes !== target, 'result should be cloned') + t.assert(notClonedRes === target, 'result should maintain first array entry reference') + t.end() +}) + test('add keys in target that do not exist at the root', function(t) { var src = { key1: 'value1', key2: 'value2' } var target = {} From 10deb3548cc6ec521baa030adebdb2bf497919a5 Mon Sep 17 00:00:00 2001 From: Aaron Stone Date: Thu, 29 Oct 2020 11:15:16 -0600 Subject: [PATCH 03/10] Missing index on return --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index 0794239..7a4530d 100644 --- a/index.js +++ b/index.js @@ -5,7 +5,7 @@ function emptyTarget(val) { } function firstArrayEntry(arr) { - return arr[0] && arr[0].length ? arr : [] + return arr[0] && arr[0].length ? arr[0] : [] } function cloneUnlessOtherwiseSpecified(value, options) { From 38d54b2dc7cbd22b5aceb36b1b60a9ffe601a997 Mon Sep 17 00:00:00 2001 From: Aaron Stone Date: Thu, 29 Oct 2020 11:16:32 -0600 Subject: [PATCH 04/10] Index madness. Oops. --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index 7a4530d..0df2a22 100644 --- a/index.js +++ b/index.js @@ -5,7 +5,7 @@ function emptyTarget(val) { } function firstArrayEntry(arr) { - return arr[0] && arr[0].length ? arr[0] : [] + return arr && arr.length ? arr[0] : [] } function cloneUnlessOtherwiseSpecified(value, options) { From cf8ef25861507b192722743beaffafa440110fca Mon Sep 17 00:00:00 2001 From: Aaron Stone Date: Thu, 5 Nov 2020 20:58:33 -0700 Subject: [PATCH 05/10] Add cloneWithTarget flag --- index.js | 6 +++--- test/merge.js | 16 ++++++++-------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/index.js b/index.js index 0df2a22..8d3d025 100644 --- a/index.js +++ b/index.js @@ -57,11 +57,11 @@ function propertyIsUnsafe(target, key) { // Retrieves either a new object or the appropriate target object to mutate. function getDestinationObject(target, options) { - if (options && options.clone !== false) { - return {} + if (options && !!options.cloneWithTarget) { + return Array.isArray(target) ? firstArrayEntry(target) : target } - return Array.isArray(target) ? firstArrayEntry(target) : target + return {} } function mergeObject(target, source, options) { diff --git a/test/merge.js b/test/merge.js index 5737f08..f0ebb25 100644 --- a/test/merge.js +++ b/test/merge.js @@ -1,36 +1,36 @@ var merge = require('../') var test = require('tape') -test('result should retain target type information when not cloning', function(t) { +test('result should retain target type information when cloneWithTarget set to true', function(t) { var src = { key1: 'value1', key2: 'value2' } class CustomType {} var target = new CustomType() - var res = merge(target, src, {clone: false}) + var res = merge(target, src, {cloneWithTarget: true}) t.not(src instanceof CustomType) t.assert(target instanceof CustomType) t.assert(res instanceof CustomType) t.end() }) -test('modify target object if clone set to false', function(t) { +test('modify target object if cloneWithTarget set to true', function(t) { var src = { key1: 'value1', key2: 'value2' } var target = { key3: 'value3'} - var clonedRes = merge(target, src, {clone: true}) - var notClonedRes = merge(target, src, {clone: false}) + var clonedRes = merge(target, src) + var notClonedRes = merge(target, src, {cloneWithTarget: true}) t.assert(clonedRes !== target, 'result should be cloned') t.assert(notClonedRes === target, 'result should maintain target reference') t.end() }) -test('merge.all mutates target object', function(t) { +test('merge.all mutates target object when cloneWithTarget set to true', function(t) { var src = { key1: 'value1', key2: 'value2' } var target = { key3: 'value3'} - var clonedRes = merge.all([target, src], {clone: true}) - var notClonedRes = merge.all([target, src], {clone: false}) + var clonedRes = merge.all([target, src]) + var notClonedRes = merge.all([target, src], {cloneWithTarget: true}) t.assert(clonedRes !== target, 'result should be cloned') t.assert(notClonedRes === target, 'result should maintain first array entry reference') From 971f5e6f71b6ba655db6b23fde66fbc963b876d0 Mon Sep 17 00:00:00 2001 From: Aaron Stone Date: Thu, 5 Nov 2020 21:01:03 -0700 Subject: [PATCH 06/10] merge with target, not clone with target... --- index.js | 2 +- test/merge.js | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/index.js b/index.js index 8d3d025..fdb44ff 100644 --- a/index.js +++ b/index.js @@ -57,7 +57,7 @@ function propertyIsUnsafe(target, key) { // Retrieves either a new object or the appropriate target object to mutate. function getDestinationObject(target, options) { - if (options && !!options.cloneWithTarget) { + if (options && !!options.mergeWithTarget) { return Array.isArray(target) ? firstArrayEntry(target) : target } diff --git a/test/merge.js b/test/merge.js index f0ebb25..c37b1fc 100644 --- a/test/merge.js +++ b/test/merge.js @@ -1,36 +1,36 @@ var merge = require('../') var test = require('tape') -test('result should retain target type information when cloneWithTarget set to true', function(t) { +test('result should retain target type information when mergeWithTarget set to true', function(t) { var src = { key1: 'value1', key2: 'value2' } class CustomType {} var target = new CustomType() - var res = merge(target, src, {cloneWithTarget: true}) + var res = merge(target, src, {mergeWithTarget: true}) t.not(src instanceof CustomType) t.assert(target instanceof CustomType) t.assert(res instanceof CustomType) t.end() }) -test('modify target object if cloneWithTarget set to true', function(t) { +test('modify target object if mergeWithTarget set to true', function(t) { var src = { key1: 'value1', key2: 'value2' } var target = { key3: 'value3'} var clonedRes = merge(target, src) - var notClonedRes = merge(target, src, {cloneWithTarget: true}) + var notClonedRes = merge(target, src, {mergeWithTarget: true}) t.assert(clonedRes !== target, 'result should be cloned') t.assert(notClonedRes === target, 'result should maintain target reference') t.end() }) -test('merge.all mutates target object when cloneWithTarget set to true', function(t) { +test('merge.all mutates target object when mergeWithTarget set to true', function(t) { var src = { key1: 'value1', key2: 'value2' } var target = { key3: 'value3'} var clonedRes = merge.all([target, src]) - var notClonedRes = merge.all([target, src], {cloneWithTarget: true}) + var notClonedRes = merge.all([target, src], {mergeWithTarget: true}) t.assert(clonedRes !== target, 'result should be cloned') t.assert(notClonedRes === target, 'result should maintain first array entry reference') From c5688b7180e486436c480e253e0381e6cc8522a3 Mon Sep 17 00:00:00 2001 From: Aaron Stone Date: Thu, 5 Nov 2020 21:52:06 -0700 Subject: [PATCH 07/10] Remove unnecessary boolean cast --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index fdb44ff..1f614f1 100644 --- a/index.js +++ b/index.js @@ -57,7 +57,7 @@ function propertyIsUnsafe(target, key) { // Retrieves either a new object or the appropriate target object to mutate. function getDestinationObject(target, options) { - if (options && !!options.mergeWithTarget) { + if (options && options.mergeWithTarget) { return Array.isArray(target) ? firstArrayEntry(target) : target } From 3b66e4cad6c5e9f6b1e0b8d7c3ec8a085a7c0dd1 Mon Sep 17 00:00:00 2001 From: Aaron Stone Date: Thu, 12 Nov 2020 12:42:49 -0700 Subject: [PATCH 08/10] Handle undefined target --- index.js | 14 +++++++++----- test/merge.js | 12 ++++++++++++ 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/index.js b/index.js index 1f614f1..962a6d9 100644 --- a/index.js +++ b/index.js @@ -56,16 +56,20 @@ function propertyIsUnsafe(target, key) { } // Retrieves either a new object or the appropriate target object to mutate. -function getDestinationObject(target, options) { - if (options && options.mergeWithTarget) { +function getDestinationObject(target, source, options) { + const targetDefined = typeof target !== 'undefined' + const isArray = Array.isArray(target) || Array.isArray(source) + const doMerge = options && (options.mergeWithTarget || options.clone === false) + + if (targetDefined && doMerge) { return Array.isArray(target) ? firstArrayEntry(target) : target } - return {} + return isArray ? [] : {}; } function mergeObject(target, source, options) { - var destination = getDestinationObject(target, options) + var destination = getDestinationObject(target, source, options) if (options.isMergeableObject(target)) { getKeys(target).forEach(function(key) { destination[key] = cloneUnlessOtherwiseSpecified(target[key], options) @@ -113,7 +117,7 @@ deepmerge.all = function deepmergeAll(array, options) { return array.reduce(function(prev, next) { return deepmerge(prev, next, options) - }, getDestinationObject(array, options)) + }, getDestinationObject(array, undefined, options)) } module.exports = deepmerge diff --git a/test/merge.js b/test/merge.js index c37b1fc..9cd3801 100644 --- a/test/merge.js +++ b/test/merge.js @@ -1,6 +1,18 @@ var merge = require('../') var test = require('tape') +test('should handle an undefined value in the target object when merging', function(t) { + var src = { key1: 'value1', key2: { key4: 'value4'}, key3: ['value3'] } + var target = { key1: 'value', key2: undefined, key3: undefined } + + var notClonedRes = merge(target, src, {mergeWithTarget: true}) + + t.assert(notClonedRes === target, 'should successfully merge mutating target'); + t.assert(notClonedRes.key2 instanceof Object, 'should successfully merge object'); + t.assert(Array.isArray(notClonedRes.key3), 'should successfully merge array'); + t.end() +}) + test('result should retain target type information when mergeWithTarget set to true', function(t) { var src = { key1: 'value1', key2: 'value2' } class CustomType {} From dadd37e8abac9d7b22accb2c419ce45c613ead62 Mon Sep 17 00:00:00 2001 From: Aaron Stone Date: Fri, 13 Nov 2020 16:32:51 -0700 Subject: [PATCH 09/10] Improve undefined source / target testing --- test/merge.js | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/test/merge.js b/test/merge.js index 9cd3801..71858fd 100644 --- a/test/merge.js +++ b/test/merge.js @@ -2,14 +2,23 @@ var merge = require('../') var test = require('tape') test('should handle an undefined value in the target object when merging', function(t) { - var src = { key1: 'value1', key2: { key4: 'value4'}, key3: ['value3'] } - var target = { key1: 'value', key2: undefined, key3: undefined } + var src = { key1: 'value1', key2: { key4: 'value4'}, key3: ['value3'], key5: undefined } + var target = { key1: 'value', key2: undefined, key3: undefined, key5: ['value5'], key6: ['value6'], key7: { key8: 'value8'} } var notClonedRes = merge(target, src, {mergeWithTarget: true}) - t.assert(notClonedRes === target, 'should successfully merge mutating target'); - t.assert(notClonedRes.key2 instanceof Object, 'should successfully merge object'); - t.assert(Array.isArray(notClonedRes.key3), 'should successfully merge array'); + // Undefined target + t.assert(notClonedRes.key2 === target.key2, 'should merge object source into undefined value'); + t.assert(notClonedRes.key3 === target.key3, 'should merge array source into undefined target'); + t.assert(typeof notClonedRes.key2 === 'object', 'should retain object type when merging into undefined target'); + t.assert(Array.isArray(notClonedRes.key3), 'should retain array type when merging into undefined target'); + + // Explicit undefined source + t.assert(typeof key5 === 'undefined', 'should overwrite value with explicitly undefined value'); + + // Not defined source props + t.assert(Array.isArray(notClonedRes.key6), 'should preserve target property value when no source value exists'); + t.assert(typeof notClonedRes.key7 === 'object', 'should preserve target property value when'); t.end() }) From 4c0d549597aaa88210d7466f92f8003b465d64b8 Mon Sep 17 00:00:00 2001 From: Aaron Stone Date: Fri, 13 Nov 2020 16:34:05 -0700 Subject: [PATCH 10/10] Incomplete assertion definitions --- test/merge.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/merge.js b/test/merge.js index 71858fd..c34d2b5 100644 --- a/test/merge.js +++ b/test/merge.js @@ -14,11 +14,11 @@ test('should handle an undefined value in the target object when merging', funct t.assert(Array.isArray(notClonedRes.key3), 'should retain array type when merging into undefined target'); // Explicit undefined source - t.assert(typeof key5 === 'undefined', 'should overwrite value with explicitly undefined value'); + t.assert(typeof key5 === 'undefined', 'should overwrite target value with explicitly undefined source value'); // Not defined source props t.assert(Array.isArray(notClonedRes.key6), 'should preserve target property value when no source value exists'); - t.assert(typeof notClonedRes.key7 === 'object', 'should preserve target property value when'); + t.assert(typeof notClonedRes.key7 === 'object', 'should preserve target property value when no source value exists'); t.end() })