Repository: rtfeldman/seamless-immutable (js)

Issue:
Add function for deep merge?
I did this a few days ago in a fork:
https://github.com/crudh/seamless-immutable/commit/8552c98fd177cdf48d560758caa1ab8bee7df92f

mergeDeep is like the normal merge function but it merges all children objects too. Right now there is no code reuse from the normal merge (except for 2 lines it is a copy and paste of merge) and the tests are simple copies from the normal merge.

Before cleaning it up for my fork I just wanted to know if there is any interest to pull it back into master? If so, are there any preferences for naming, code reuse and so on?

Pull request description:
deep merge support in the merge function
merge now only accepts 1 or 2 params. The first is the object to merge or an array of objects to merge. The second is an optional config object that right now only supports `{deep: true}`. If supplied it will perform a deep merge. See details in #19.

I am not sure that the test coverage is enough?

Fixes #19.

Patch:
diff --git a/seamless-immutable.js b/seamless-immutable.js
index a9d1f56..f10f509 100644
--- a/seamless-immutable.js
+++ b/seamless-immutable.js
@@ -215,9 +215,10 @@
    * this object and the provided object, prioritizing the provided object's
    * values whenever the same key is present in both objects.
    *
-   * @param {object} other - The other object to merge. Multiple objects can be passed, either as an array or as extra arguments. In such a case, the later an object appears in that list, the higher its priority.
+   * @param {object} other - The other object to merge. Multiple objects can be passed as an array. In such a case, the later an object appears in that list, the higher its priority.
+   * @param {object} config - Optional config object what contains settings. Right now only {deep: true} is supported for a merge deep
    */
-  function merge(arg) {
+  function merge(other, config) {
     // Calling .merge() with no arguments is a no-op. Don't bother cloning.
     if (arguments.length === 0) {
       return this;
@@ -225,7 +226,8 @@
 
     var anyChanges    = false,
         result        = quickCopy(this, {}), // A shallow clone of this object.
-        receivedArray = (arg instanceof Array),
+        receivedArray = (other instanceof Array),
+        deep          = config && config.deep,
         key;
 
     // Use the given key to extract a value from the given object, then place
@@ -240,28 +242,26 @@
           // Avoid false positives due to (NaN !== NaN) evaluating to true
           (immutableValue === immutableValue)));
 
-      result[key] = immutableValue;
+      if (deep && currentObj[key] !== null && typeof currentObj[key] === "object" && typeof immutableValue === "object" ) {
+        result[key] = currentObj[key].merge(immutableValue, config);
+      } else {
+        result[key] = immutableValue;
+      }
     }
 
     // Achieve prioritization by overriding previous values that get in the way.
-    if (!receivedArray && arguments.length === 1) {
+    if (!receivedArray) {
       // The most common use case: just merge one object into the existing one.
-      for (key in arg) {
-        addToResult(this, arg, key);
+      for (key in other) {
+        addToResult(this, other, key);
       }
     } else {
-      // We also accept either an Array or multiple arguments.
-      var others = receivedArray ? arg :
-        Array.prototype.slice.call(arguments);
-        // Note: If we don't convert arguments into an Array, IE9 will end up
-        // including the arguments object in the final result object.
-        // Can't make this stuff up.
-
-      for (var index in others) {
-        var other = others[index];
-
-        for (key in other) {
-          addToResult(this, other, key);
+      // We also accept an Array
+      for (var index in other) {
+        var otherFromArray = other[index];
+
+        for (key in otherFromArray) {
+          addToResult(this, otherFromArray, key);
         }
       }
     }
@@ -334,4 +334,4 @@
   } else if (typeof global === "object") {
     global.Immutable = Immutable;
   }
-})();
\ No newline at end of file
+})();


Test patch:
diff --git a/test/ImmutableObject.spec.js b/test/ImmutableObject.spec.js
index b12aa54..9483322 100644
--- a/test/ImmutableObject.spec.js
+++ b/test/ImmutableObject.spec.js
@@ -8,4 +8,4 @@ describe("ImmutableObject", function() {
   testMerge();
   testWithout();
   testToMutable();
-});
\ No newline at end of file
+});
diff --git a/test/ImmutableObject/test-merge.js b/test/ImmutableObject/test-merge.js
index 3e1bd6f..d1e570d 100644
--- a/test/ImmutableObject/test-merge.js
+++ b/test/ImmutableObject/test-merge.js
@@ -11,18 +11,12 @@ function generateArrayOfObjects() {
 
 module.exports = function() {
   describe("#merge", function() {
-    function generateMergeTestsFor(specifiers) {
+    function generateMergeTestsFor(specifiers, config) {
       var runs = 100;
 
       function checkMultiple(callback) {
         check(runs, specifiers, function(list) {
-          var useVarArgs = !(list instanceof Array);
-
-          if (arguments.length > 1) {
-            list = Array.prototype.slice.call(arguments);
-          } else if (useVarArgs) {
-            list = [list]
-          }
+          list = list instanceof Array ? list : [list];
 
           assert.notStrictEqual(list.length, 0, "Can't usefully check merge() with no objects");
 
@@ -31,9 +25,7 @@ module.exports = function() {
           function runMerge(others) {
             others = others || list;
 
-            return useVarArgs ?
-              immutable.merge.apply(immutable, others) :
-              immutable.merge(others);
+            return immutable.merge(others, config);
           }
 
           callback(immutable, list, runMerge);
@@ -60,7 +52,7 @@ module.exports = function() {
 
                 // Record that we expect this to end up in the final result.
                 expectedChanges[key] = newValue;
-                assert(Immutable.isImmutable(expectedChanges[key]))
+                assert(Immutable.isImmutable(expectedChanges[key]));
 
                 mutable[key]  = newValue;
               } else if (mutable.hasOwnProperty(key) && mutable[key] !== immutable[key]) {
@@ -163,16 +155,35 @@ module.exports = function() {
       });
     });
 
+    it("merges deep when the config tells it to", function() {
+      var original = Immutable({all: "your base", are: {belong: "to us", you: {have: "x", make: "your time"}}});
+      var toMerge  = {are: {you: {have: "no chance to survive"}}};
+
+      var expectedShallow = Immutable({all: "your base", are: {you: {have: "no chance to survive"}}});
+      var actualShallow   = original.merge(toMerge);
+
+      assert.deepEqual(actualShallow, expectedShallow);
+
+      var expectedDeep = Immutable({all: "your base", are: {belong: "to us", you: {have: "no chance to survive", make: "your time"}}});
+      var actualDeep   = original.merge(toMerge, {deep: true});
+
+      assert.deepEqual(actualDeep, expectedDeep);
+    });
+
     describe("when passed a single object", function() {
       generateMergeTestsFor([TestUtils.ComplexObjectSpecifier()]);
     });
 
-    describe("when passed multiple objects", function() {
-      generateMergeTestsFor([TestUtils.ComplexObjectSpecifier(), TestUtils.ComplexObjectSpecifier(), TestUtils.ComplexObjectSpecifier()]);
+    describe("when passed a single object with deep set to true", function() {
+      generateMergeTestsFor([TestUtils.ComplexObjectSpecifier()], {deep: true});
     });
 
     describe("when passed an array of objects", function() {
       generateMergeTestsFor([generateArrayOfObjects]);
     });
+
+    describe("when passed an array of objects with deep set to true", function() {
+      generateMergeTestsFor([generateArrayOfObjects], {deep: true});
+    });
   });
-};
\ No newline at end of file
+};