Repository: rtfeldman/seamless-immutable
Pull request #71 (2015-10-19 13:55:56)

Fix deep merge strict equality
Fixes https://github.com/rtfeldman/seamless-immutable/issues/70


Patch:
diff --git a/seamless-immutable.development.js b/seamless-immutable.development.js
index 26bb03c..e5dccc8 100644
--- a/seamless-immutable.development.js
+++ b/seamless-immutable.development.js
@@ -248,12 +248,10 @@
       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
@@ -261,24 +259,40 @@
     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;
     }
   }
 
diff --git a/seamless-immutable.production.min.js b/seamless-immutable.production.min.js
index 1ad16b8..7e5e2d0 100644
--- a/seamless-immutable.production.min.js
+++ b/seamless-immutable.production.min.js
@@ -1,1 +1,1 @@
-!function(){"use strict";function a(a,b,c){Object.defineProperty(a,b,{enumerable:!1,configurable:!1,writable:!1,value:c})}function b(b){a(b,t,!0)}function c(a){return"object"==typeof a?null===a||a.hasOwnProperty(t):!0}function d(a){return!(null===a||"object"!=typeof a||a instanceof Array||a instanceof Date)}function e(a){var b=new Error(a);return b.__proto__=e,b}function f(a,c){b(a);return a}function g(b,c){var d=b[c];a(b,c,function(){return s(d.apply(b,arguments))})}function h(b){for(var c in x)if(x.hasOwnProperty(c)){var d=x[c];g(b,d)}a(b,"flatMap",i),a(b,"asObject",l),a(b,"asMutable",k);for(var e=0,h=b.length;h>e;e++)b[e]=s(b[e]);return f(b,w)}function i(a){if(0===arguments.length)return this;var b,c=[],d=this.length;for(b=0;d>b;b++){var e=a(this[b],b,this);e instanceof Array?c.push.apply(c,e):c.push(e)}return h(c)}function j(a){if(0===arguments.length)return this;a instanceof Array||(a=Array.prototype.slice.call(arguments));var b=this.instantiateEmptyObject();for(var c in this)this.hasOwnProperty(c)&&-1===a.indexOf(c)&&(b[c]=this[c]);return r(b,{instantiateEmptyObject:this.instantiateEmptyObject})}function k(a){var b,c,d=[];if(a&&a.deep)for(b=0,c=this.length;c>b;b++)d.push(m(this[b]));else for(b=0,c=this.length;c>b;b++)d.push(this[b]);return d}function l(a){"function"!=typeof a&&(a=function(a){return a});var b,c={},d=this.length;for(b=0;d>b;b++){var e=a(this[b],b,this),f=e[0],g=e[1];c[f]=g}return r(c)}function m(a){return!a||!a.hasOwnProperty(t)||a instanceof Date?a:a.asMutable({deep:!0})}function n(a,b){for(var c in a)a.hasOwnProperty(c)&&(b[c]=a[c]);return b}function o(a,b){function c(a,c,e){var h=s(c[e]),k=j&&j(a[e],h,b);j&&k&&k===a[e]||(f=f||void 0!==k||!a.hasOwnProperty(e)||h!==a[e]&&h===h,k?g[e]=k:i&&d(a[e])&&d(h)?g[e]=a[e].merge(h,b):g[e]=h)}if(0===arguments.length)return this;if(null===a||"object"!=typeof a)throw new TypeError("Immutable#merge can only be invoked with objects or arrays, not "+JSON.stringify(a));var e,f=!1,g=n(this,this.instantiateEmptyObject()),h=a instanceof Array,i=b&&b.deep,j=b&&b.merger;if(h)for(var k=0;k<a.length;k++){var l=a[k];for(e in l)l.hasOwnProperty(e)&&c(this,l,e)}else for(e in a)a.hasOwnProperty(e)&&c(this,a,e);return f?r(g,{instantiateEmptyObject:this.instantiateEmptyObject}):this}function p(a){var b,c=this.instantiateEmptyObject();if(a&&a.deep)for(b in this)this.hasOwnProperty(b)&&(c[b]=m(this[b]));else for(b in this)this.hasOwnProperty(b)&&(c[b]=this[b]);return c}function q(){return{}}function r(b,c){var d=c&&c.instantiateEmptyObject?c.instantiateEmptyObject:q;return a(b,"merge",o),a(b,"without",j),a(b,"asMutable",p),a(b,"instantiateEmptyObject",d),f(b,u)}function s(a,b){if(c(a))return a;if(a instanceof Array)return h(a.slice());if(a instanceof Date)return f(new Date(a.getTime()));var d=b&&b.prototype,e=d&&d!==Object.prototype?function(){return Object.create(d)}:q,g=e();for(var i in a)a.hasOwnProperty(i)&&(g[i]=s(a[i]));return r(g,{instantiateEmptyObject:e})}var t="__immutable_invariants_hold",u=["setPrototypeOf"],v=["keys"],w=u.concat(["push","pop","sort","splice","shift","unshift","reverse"]),x=v.concat(["map","filter","slice","concat","reduce","reduceRight"]);e.prototype=Error.prototype,s.isImmutable=c,s.ImmutableError=e,Object.freeze(s),"object"==typeof module?module.exports=s:"object"==typeof exports?exports.Immutable=s:"object"==typeof window?window.Immutable=s:"object"==typeof global&&(global.Immutable=s)}();
\ No newline at end of file
+!function(){"use strict";function a(a,b,c){Object.defineProperty(a,b,{enumerable:!1,configurable:!1,writable:!1,value:c})}function b(b){a(b,t,!0)}function c(a){return"object"==typeof a?null===a||a.hasOwnProperty(t):!0}function d(a){return!(null===a||"object"!=typeof a||a instanceof Array||a instanceof Date)}function e(a){var b=new Error(a);return b.__proto__=e,b}function f(a,c){b(a);return a}function g(b,c){var d=b[c];a(b,c,function(){return s(d.apply(b,arguments))})}function h(b){for(var c in x)if(x.hasOwnProperty(c)){var d=x[c];g(b,d)}a(b,"flatMap",i),a(b,"asObject",l),a(b,"asMutable",k);for(var e=0,h=b.length;h>e;e++)b[e]=s(b[e]);return f(b,w)}function i(a){if(0===arguments.length)return this;var b,c=[],d=this.length;for(b=0;d>b;b++){var e=a(this[b],b,this);e instanceof Array?c.push.apply(c,e):c.push(e)}return h(c)}function j(a){if(0===arguments.length)return this;a instanceof Array||(a=Array.prototype.slice.call(arguments));var b=this.instantiateEmptyObject();for(var c in this)this.hasOwnProperty(c)&&-1===a.indexOf(c)&&(b[c]=this[c]);return r(b,{instantiateEmptyObject:this.instantiateEmptyObject})}function k(a){var b,c,d=[];if(a&&a.deep)for(b=0,c=this.length;c>b;b++)d.push(m(this[b]));else for(b=0,c=this.length;c>b;b++)d.push(this[b]);return d}function l(a){"function"!=typeof a&&(a=function(a){return a});var b,c={},d=this.length;for(b=0;d>b;b++){var e=a(this[b],b,this),f=e[0],g=e[1];c[f]=g}return r(c)}function m(a){return!a||!a.hasOwnProperty(t)||a instanceof Date?a:a.asMutable({deep:!0})}function n(a,b){for(var c in a)a.hasOwnProperty(c)&&(b[c]=a[c]);return b}function o(a,b){function c(a,c,f){var g=s(c[f]),j=i&&i(a[f],g,b),k=a[f];if(void 0!==e||void 0!==j||!a.hasOwnProperty(f)||g!==k&&g===g){var l;l=j?j:h&&d(k)&&d(g)?k.merge(g,b):g,(k!==l&&l===l||!a.hasOwnProperty(f))&&(void 0===e&&(e=n(a,a.instantiateEmptyObject())),e[f]=l)}}if(0===arguments.length)return this;if(null===a||"object"!=typeof a)throw new TypeError("Immutable#merge can only be invoked with objects or arrays, not "+JSON.stringify(a));var e,f,g=a instanceof Array,h=b&&b.deep,i=b&&b.merger;if(g)for(var j=0;j<a.length;j++){var k=a[j];for(f in k)k.hasOwnProperty(f)&&c(this,k,f)}else for(f in a)a.hasOwnProperty(f)&&c(this,a,f);return void 0===e?this:r(e,{instantiateEmptyObject:this.instantiateEmptyObject})}function p(a){var b,c=this.instantiateEmptyObject();if(a&&a.deep)for(b in this)this.hasOwnProperty(b)&&(c[b]=m(this[b]));else for(b in this)this.hasOwnProperty(b)&&(c[b]=this[b]);return c}function q(){return{}}function r(b,c){var d=c&&c.instantiateEmptyObject?c.instantiateEmptyObject:q;return a(b,"merge",o),a(b,"without",j),a(b,"asMutable",p),a(b,"instantiateEmptyObject",d),f(b,u)}function s(a,b){if(c(a))return a;if(a instanceof Array)return h(a.slice());if(a instanceof Date)return f(new Date(a.getTime()));var d=b&&b.prototype,e=d&&d!==Object.prototype?function(){return Object.create(d)}:q,g=e();for(var i in a)a.hasOwnProperty(i)&&(g[i]=s(a[i]));return r(g,{instantiateEmptyObject:e})}var t="__immutable_invariants_hold",u=["setPrototypeOf"],v=["keys"],w=u.concat(["push","pop","sort","splice","shift","unshift","reverse"]),x=v.concat(["map","filter","slice","concat","reduce","reduceRight"]);e.prototype=Error.prototype,s.isImmutable=c,s.ImmutableError=e,Object.freeze(s),"object"==typeof module?module.exports=s:"object"==typeof exports?exports.Immutable=s:"object"==typeof window?window.Immutable=s:"object"==typeof global&&(global.Immutable=s)}();
\ No newline at end of file
diff --git a/src/seamless-immutable.js b/src/seamless-immutable.js
index ceb58ed..1b6cc75 100644
--- a/src/seamless-immutable.js
+++ b/src/seamless-immutable.js
@@ -248,12 +248,10 @@
       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
@@ -261,24 +259,40 @@
     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 patch:
diff --git a/test/ImmutableObject/test-merge.js b/test/ImmutableObject/test-merge.js
index 3316eb9..342ccb7 100644
--- a/test/ImmutableObject/test-merge.js
+++ b/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.