Merge

.gitignore

@@ -31,3 +31,4 @@ tmp
 html-report
 lcov.info
 lcov-report
+.idea

bower.json

@@ -1,7 +1,7 @@
 {
   "name": "seamless-immutable",
   "main": "src/seamless-immutable.js",
-  "version": "4.0.1",
+  "version": "4.0.2",
   "homepage": "https://github.com/rtfeldman/seamless-immutable",
   "authors": [
     "Richard Feldman <[email protected]>"

package.json

@@ -1,6 +1,6 @@
 {
   "name": "seamless-immutable",
-  "version": "4.0.1",
+  "version": "4.0.2",
   "description": "Immutable data structures for JavaScript which are backwards-compatible with normal JS Arrays and Objects.",
   "main": "src/seamless-immutable.js",
   "devDependencies": {

seamless-immutable.development.js

@@ -248,37 +248,51 @@
       throw new TypeError("Immutable#merge can only be invoked with objects or arrays, not " + JSON.stringify(other));
     }
 
-    var anyChanges    = false,
-        result        = quickCopy(this, this.instantiateEmptyObject()), // A shallow clone of this object.
-        receivedArray = (other instanceof Array),
+    var receivedArray = (other instanceof Array),
         deep          = config && config.deep,
         merger        = config && config.merger,
-        key;
+        result;
 
     // Use the given key to extract a value from the given object, then place
     // that value in the result object under the same key. If that resulted
     // in a change from this object's value at that key, set anyChanges = true.
     function addToResult(currentObj, otherObj, key) {
       var immutableValue = Immutable(otherObj[key]);
       var mergerResult = merger && merger(currentObj[key], immutableValue, config);
-      if (merger && mergerResult && mergerResult === currentObj[key]) return;
+      var currentValue = currentObj[key];
 
-      anyChanges = anyChanges ||
-        mergerResult !== undefined ||
+      if ((result !== undefined) ||
+        (mergerResult !== undefined) ||
         (!currentObj.hasOwnProperty(key) ||
-        ((immutableValue !== currentObj[key]) &&
+        ((immutableValue !== currentValue) &&
           // Avoid false positives due to (NaN !== NaN) evaluating to true
-          (immutableValue === immutableValue)));
+          (immutableValue === immutableValue)))) {
 
-      if (mergerResult) {
-        result[key] = mergerResult;
-      } else if (deep && isMergableObject(currentObj[key]) && isMergableObject(immutableValue)) {
-        result[key] = currentObj[key].merge(immutableValue, config);
-      } else {
-        result[key] = immutableValue;
+        var newValue;
+
+        if (mergerResult) {
+          newValue = mergerResult;
+        } else if (deep && isMergableObject(currentValue) && isMergableObject(immutableValue)) {
+          newValue = currentValue.merge(immutableValue, config);
+        } else {
+          newValue = immutableValue;
+        }
+
+        // We check (newValue === newValue) because (NaN !== NaN) in JS
+        if (((currentValue !== newValue) && (newValue === newValue))
+            || !currentObj.hasOwnProperty(key)) {
+          if (result === undefined) {
+            // Make a shallow clone of the current object.
+            result = quickCopy(currentObj, currentObj.instantiateEmptyObject());
+          }
+
+          result[key] = newValue;
+        }
       }
     }
 
+    var key;
+
     // Achieve prioritization by overriding previous values that get in the way.
     if (!receivedArray) {
       // The most common use case: just merge one object into the existing one.
@@ -300,11 +314,11 @@
       }
     }
 
-    if (anyChanges) {
+    if (result === undefined) {
+      return this;
+    } else {
       return makeImmutableObject(result,
         {instantiateEmptyObject: this.instantiateEmptyObject});
-    } else {
-      return this;
     }
   }
 

src/seamless-immutable.js

@@ -248,37 +248,51 @@
       throw new TypeError("Immutable#merge can only be invoked with objects or arrays, not " + JSON.stringify(other));
     }
 
-    var anyChanges    = false,
-        result        = quickCopy(this, this.instantiateEmptyObject()), // A shallow clone of this object.
-        receivedArray = (other instanceof Array),
+    var receivedArray = (other instanceof Array),
         deep          = config && config.deep,
         merger        = config && config.merger,
-        key;
+        result;
 
     // Use the given key to extract a value from the given object, then place
     // that value in the result object under the same key. If that resulted
     // in a change from this object's value at that key, set anyChanges = true.
     function addToResult(currentObj, otherObj, key) {
       var immutableValue = Immutable(otherObj[key]);
       var mergerResult = merger && merger(currentObj[key], immutableValue, config);
-      if (merger && mergerResult && mergerResult === currentObj[key]) return;
+      var currentValue = currentObj[key];
 
-      anyChanges = anyChanges ||
-        mergerResult !== undefined ||
+      if ((result !== undefined) ||
+        (mergerResult !== undefined) ||
         (!currentObj.hasOwnProperty(key) ||
-        ((immutableValue !== currentObj[key]) &&
+        ((immutableValue !== currentValue) &&
           // Avoid false positives due to (NaN !== NaN) evaluating to true
-          (immutableValue === immutableValue)));
+          (immutableValue === immutableValue)))) {
 
-      if (mergerResult) {
-        result[key] = mergerResult;
-      } else if (deep && isMergableObject(currentObj[key]) && isMergableObject(immutableValue)) {
-        result[key] = currentObj[key].merge(immutableValue, config);
-      } else {
-        result[key] = immutableValue;
+        var newValue;
+
+        if (mergerResult) {
+          newValue = mergerResult;
+        } else if (deep && isMergableObject(currentValue) && isMergableObject(immutableValue)) {
+          newValue = currentValue.merge(immutableValue, config);
+        } else {
+          newValue = immutableValue;
+        }
+
+        // We check (newValue === newValue) because (NaN !== NaN) in JS
+        if (((currentValue !== newValue) && (newValue === newValue))
+            || !currentObj.hasOwnProperty(key)) {
+          if (result === undefined) {
+            // Make a shallow clone of the current object.
+            result = quickCopy(currentObj, currentObj.instantiateEmptyObject());
+          }
+
+          result[key] = newValue;
+        }
       }
     }
 
+    var key;
+
     // Achieve prioritization by overriding previous values that get in the way.
     if (!receivedArray) {
       // The most common use case: just merge one object into the existing one.
@@ -300,11 +314,11 @@
       }
     }
 
-    if (anyChanges) {
+    if (result === undefined) {
+      return this;
+    } else {
       return makeImmutableObject(result,
         {instantiateEmptyObject: this.instantiateEmptyObject});
-    } else {
-      return this;
     }
   }
 

test/ImmutableObject/test-merge.js

@@ -95,7 +95,7 @@ module.exports = function(config) {
 
           _.each(mutables, function(mutable, index) {
             _.each(mutable, function (value, key) {
-              assert(result.hasOwnProperty(key), "Result " + JSON.stringify(result) + " did not have property " + key);
+              assert(result.hasOwnProperty(key), "Result " + JSON.stringify(result) + " did not have property " + key + " with value " + value + " from " + JSON.stringify(mutable));
             });
           });
         });
@@ -111,6 +111,23 @@ module.exports = function(config) {
         });
       });
 
+      it("does not reproduce #70", function() {
+        var c = Immutable({a: {b: 1}});
+
+        assert.strictEqual(c, c.merge({a: {b: 1}}, {deep: true}));
+      });
+
+      it("does nothing when merging an identical object", function() {
+        checkMultiple(function(immutable, mutables, runMerge) {
+          _.each(mutables, function(mutable, index) {
+            var identicalImmutable = Immutable(mutable);
+
+            assert.strictEqual(identicalImmutable,
+              identicalImmutable.merge(mutable, {deep: true}));
+          });
+        });
+      });
+
       it("does nothing when passed a merge that will result in no changes", function() {
         checkMultiple(function(immutable, mutables, runMerge) {
           // Make sure all the changes will be no-ops.