Merge pull request #1811 from Polymer/handle-nan-in-bindings

Handle NaN correctly in bindings
Polymer · Jun 11, 2015 · 1cad7b0 · 1cad7b0
2 parents f76ff64 + 48edb32
commit 1cad7b0
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 4 deletions.
diff --git a/src/lib/bind/accessors.html b/src/lib/bind/accessors.html
@@ -48,7 +48,10 @@
 
       _propertySet: function(property, value, effects) {
         var old = this.__data__[property];
-        if (old !== value) {
+        // NaN is always not equal to itself,
+        // if old and value are both NaN we treat them as equal
+        // x === x is 10x faster, and equivalent to !isNaN(x)
+        if (old !== value && (old === old || value === value)) {
           this.__data__[property] = value;
           if (typeof value == 'object') {
             this._clearPath(property);

diff --git a/src/standard/notify-path.html b/src/standard/notify-path.html
@@ -76,7 +76,10 @@
       notifyPath: function(path, value, fromAbove) {
         var old = this._propertySet(path, value);
         // manual dirty checking for now...
-        if (old !== value) {
+        // NaN is always not equal to itself,
+        // if old and value are both NaN we treat them as equal
+        // x === x is 10x faster, and equivalent to !isNaN(x)
+        if (old !== value && (old === old || value === value)) {
           // console.group((this.localName || this.dataHost.id + '-' + this.dataHost.dataHost.index) + '#' + (this.id || this.index) + ' ' + path, value);
           // Take path effects at this level for exact path matches,
           // and notify down for any bindings to a subset of this path
@@ -463,4 +466,4 @@
   })();
 
 
-</script>
+</script>
diff --git a/test/unit/bind-elements.html b/test/unit/bind-elements.html
@@ -72,6 +72,12 @@
           notify: true,
           observer: 'readonlyvalueChanged'
         },
+        notifierWithoutComputing: {
+          type: Number,
+          observer: 'notifierWithoutComputingChanged',
+          notify: true,
+          value: 0
+        },
         add: {
           value: 20
         },
@@ -112,7 +118,8 @@
           computedFromMultipleValuesChanged: 0,
           multipleDepChangeHandler: 0,
           customEventValueChanged: 0,
-          customEventObjectValueChanged: 0
+          customEventObjectValueChanged: 0,
+          notifierWithoutComputingChanged: 0
         };
       },
       clearObserverCounts: function() {
@@ -158,6 +165,9 @@
         assert.equal(old, this._readonlyvalue, 'observer old argument wrong');
         this._readonlyvalue = val;
       },
+      notifierWithoutComputingChanged: function() {
+        this.observerCounts.notifierWithoutComputingChanged++;
+      },
       computeNotifyingValue: function(val) {
         return val + 2;
       },

diff --git a/test/unit/bind.html b/test/unit/bind.html
@@ -123,6 +123,19 @@
     assert.equal(el.observerCounts.computedvalueChanged, 1, 'observer not called');
   });
 
+  test('NaN does not loop observers', function() {
+    el.clearObserverCounts();
+    el.addEventListener('notifierwithoutcomputing-changed', function(e) {
+      if (el.observerCounts.notifierWithoutComputingChanged >= 3) {
+        throw new Error('infinite loop!');
+      }
+    });
+    el.notifierWithoutComputing = NaN;
+    assert.equal(el.observerCounts.notifierWithoutComputingChanged, 1,
+                 'NaN was not handled as expected');
+  });
+
+
   test('computed notification sent', function() {
     var notified = 0;
     el.addEventListener('computednotifyingvalue-changed', function(e) {