From 29c213853de49e4f2a36d9c95886ddb13237047e Mon Sep 17 00:00:00 2001 From: Youcef Mammar Date: Fri, 21 Jun 2019 16:25:12 +0200 Subject: [PATCH] fix(merge): change implementation (#716) * fix(merge): use .toStrictEqual instead of .toEqual in tests cf. https://github.com/facebook/jest/issues/711 `undefined` values being critical to in assessing merge behaviour this is necessary * fix(merge): specify description add meaning of array-like * fix(merge): change implementation - split the implem into merge(target, ...sources) and _merge(target, source), I think it's more simple this way. - I reorganized some of the conditions The description and the test suit should not change. Implementation has changed but behaviour and limitation is the same (Note that the lodash inspired test suite for merge should be passing). * fix: extra-space Co-Authored-By: Yannick Croissant * fix: rename isObjectOrArray to isObjectOrArrayOrFunction --- src/functions/merge.js | 75 ++++++++++++++++++++++-------------- test/spec/functions/merge.js | 37 +++++++++--------- 2 files changed, 64 insertions(+), 48 deletions(-) diff --git a/src/functions/merge.js b/src/functions/merge.js index 1567277cc..b8b95695e 100644 --- a/src/functions/merge.js +++ b/src/functions/merge.js @@ -2,11 +2,45 @@ function clone(value) { if (typeof value === 'object' && value !== null) { - return merge(Array.isArray(value) ? [] : {}, value); + return _merge(Array.isArray(value) ? [] : {}, value); } return value; } +function isObjectOrArrayOrFunction(value) { + return ( + typeof value === 'function' || + Array.isArray(value) || + Object.prototype.toString.call(value) === '[object Object]' + ); +} + +function _merge(target, source) { + if (target === source) { + return target; + } + + for (var key in source) { + if (!Object.prototype.hasOwnProperty.call(source, key)) { + continue; + } + + var sourceVal = source[key]; + var targetVal = target[key]; + + if (typeof targetVal !== 'undefined' && typeof sourceVal === 'undefined') { + continue; + } + + if (isObjectOrArrayOrFunction(targetVal) && isObjectOrArrayOrFunction(sourceVal)) { + target[key] = _merge(targetVal, sourceVal); + } else { + target[key] = clone(sourceVal); + } + } + return target; +} + /** * This method is like Object.assign, but recursively merges own and inherited * enumerable keyed properties of source objects into the destination object. @@ -16,43 +50,26 @@ function clone(value) { * - treats non-plain objects as plain * - does not work for circular objects * - treats sparse arrays as sparse - * - does not convert Array-like objects to arrays + * - does not convert Array-like objects (Arguments, NodeLists, etc.) to arrays * * @param {Object} object The destination object. * @param {...Object} [sources] The source objects. * @returns {Object} Returns `object`. */ -function merge() { - var sources = Array.prototype.slice.call(arguments); - var target = sources.shift(); - - return sources.reduce(function(acc, source) { - if (acc === source) { - return acc; - } - if (source === undefined) { - return acc; - } - - if (acc === undefined) { - return clone(source); - } +function merge(target) { + if (!isObjectOrArrayOrFunction(target)) { + target = {}; + } - if (typeof acc !== 'object' && typeof acc !== 'function') { - return clone(source); - } + for (var i = 1, l = arguments.length; i < l; i++) { + var source = arguments[i]; - if (typeof source !== 'object' && typeof source !== 'function') { - return acc; + if (isObjectOrArrayOrFunction(source)) { + _merge(target, source); } - - Object.keys(source).forEach(function(key) { - acc[key] = merge(acc[key], source[key]); - }); - - return acc; - }, target); + } + return target; } module.exports = merge; diff --git a/test/spec/functions/merge.js b/test/spec/functions/merge.js index f6da8c129..e7f7015e0 100644 --- a/test/spec/functions/merge.js +++ b/test/spec/functions/merge.js @@ -21,14 +21,14 @@ it('should merge `source` into `object`', function() { ] }; - expect(merge(names, ages, heights)).toEqual(expected); + expect(merge(names, ages, heights)).toStrictEqual(expected); }); it('should work with four arguments', function() { var expected = {a: 4}; var actual = merge({a: 1}, {a: 2}, {a: 3}, expected); - expect(actual).toEqual(expected); + expect(actual).toStrictEqual(expected); }); it('should merge onto function `object` values', function() { @@ -37,8 +37,8 @@ it('should merge onto function `object` values', function() { var source = {a: 1}; var actual = merge(Foo, source); - expect(actual).toEqual(Foo); - expect(Foo.a).toEqual(1); + expect(actual).toStrictEqual(Foo); + expect(Foo.a).toStrictEqual(1); }); it('should merge first source object properties to function', function() { @@ -79,9 +79,9 @@ it('should not augment source objects for inner objects', function() { var source2 = {a: [{b: 2}]}; var actual = merge({}, source1, source2); - expect(source1.a).toEqual([{a: 1}]); - expect(source2.a).toEqual([{b: 2}]); - expect(actual.a).toEqual([{a: 1, b: 2}]); + expect(source1.a).toStrictEqual([{a: 1}]); + expect(source2.a).toStrictEqual([{b: 2}]); + expect(actual.a).toStrictEqual([{a: 1, b: 2}]); }); it('should not augment source objects for inner arrays', function() { @@ -89,9 +89,9 @@ it('should not augment source objects for inner arrays', function() { var source2 = {a: [[3, 4]]}; var actual = merge({}, source1, source2); - expect(source1.a).toEqual([[1, 2, 3]]); - expect(source2.a).toEqual([[3, 4]]); - expect(actual.a).toEqual([[3, 4, 3]]); + expect(source1.a).toStrictEqual([[1, 2, 3]]); + expect(source2.a).toStrictEqual([[3, 4]]); + expect(actual.a).toStrictEqual([[3, 4, 3]]); }); it('should merge plain objects onto non-plain objects', function() { @@ -103,16 +103,16 @@ it('should merge plain objects onto non-plain objects', function() { var actual = merge(new Foo(), object); expect(actual instanceof Foo).toBe(true); - expect(actual).toEqual(new Foo(object)); + expect(actual).toStrictEqual(new Foo(object)); actual = merge([new Foo()], [object]); expect(actual[0] instanceof Foo).toBe(true); - expect(actual).toEqual([new Foo(object)]); + expect(actual).toStrictEqual([new Foo(object)]); }); it('should not overwrite existing values with `undefined` values of object sources', function() { var actual = merge({a: 1}, {a: undefined, b: undefined}); - expect(actual).toEqual({a: 1, b: undefined}); + expect(actual).toStrictEqual({a: 1, b: undefined}); }); it('should not overwrite existing values with `undefined` values of array sources', function() { @@ -122,12 +122,12 @@ it('should not overwrite existing values with `undefined` values of array source var actual = merge([4, 5, 6], array); var expected = [1, 5, 3]; - expect(actual).toEqual(expected); + expect(actual).toStrictEqual(expected); array = [1, undefined, 3]; actual = merge([4, 5, 6], array); - expect(actual).toEqual(expected); + expect(actual).toStrictEqual(expected); }); it('should skip merging when `object` and `source` are the same value', function() { @@ -153,7 +153,7 @@ it('should not convert objects to arrays when merging arrays of `source`', funct var object = {a: {'1': 'y', 'b': 'z', 'length': 2}}; var actual = merge(object, {a: ['x']}); - expect(actual).toEqual({a: { + expect(actual).toStrictEqual({a: { '0': 'x', '1': 'y', 'b': 'z', @@ -161,13 +161,12 @@ it('should not convert objects to arrays when merging arrays of `source`', funct }}); actual = merge({a: {}}, {a: []}); - expect(actual).toEqual({a: {}}); + expect(actual).toStrictEqual({a: {}}); }); it('should not convert strings to arrays when merging arrays of `source`', function() { var object = {a: 'abcde'}; var actual = merge(object, {a: ['x', 'y', 'z']}); - expect(actual).toEqual({a: ['x', 'y', 'z']}); + expect(actual).toStrictEqual({a: ['x', 'y', 'z']}); }); -