fix(merge): change implementation (#716)

* fix(merge): use .toStrictEqual instead of .toEqual in tests

cf. jestjs/jest#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 <yannick.croissant@gmail.com>

* fix: rename isObjectOrArray to isObjectOrArrayOrFunction

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;

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,19 +79,19 @@ 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() {
   var source1 = {a: [[1, 2, 3]]};
   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,21 +153,20 @@ 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',
     'length': 2
   }});
 
   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']});
 });
-