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:
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
+};