fix($compile): avoid calling $onChanges() twice for NaN initial values

gkalpak · petebacondarwin · commit 7d7efbf545c8 · 2016-09-07T12:19:27.000+01:00
Closes angular#15098
diff --git a/src/ng/compile.js b/src/ng/compile.js
@@ -3536,7 +3536,9 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
       });
 
       function recordChanges(key, currentValue, previousValue) {
-        if (isFunction(destination.$onChanges) && currentValue !== previousValue) {
+        if (isFunction(destination.$onChanges) && currentValue !== previousValue &&
+            // eslint-disable-next-line no-self-compare
+            (currentValue === currentValue || previousValue === previousValue)) {
           // If we have not already scheduled the top level onChangesQueue handler then do so now
           if (!onChangesQueue) {
             scope.$$postDigest(flushOnChangesQueue);
diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js
@@ -4387,6 +4387,40 @@ describe('$compile', function() {
           });
 
 
+          it('should not call `$onChanges` twice even when the initial value is `NaN`', function() {
+            var onChangesSpy = jasmine.createSpy('$onChanges');
+
+            module(function($compileProvider) {
+              $compileProvider.component('test', {
+                bindings: {prop: '<', attr: '@'},
+                controller: function TestController() {
+                  this.$onChanges = onChangesSpy;
+                }
+              });
+          });
+
+            inject(function($compile, $rootScope) {
+              var template = '<test prop="a" attr="{{a}}"></test>' +
+                             '<test prop="b" attr="{{b}}"></test>';
+              $rootScope.a = 'foo';
+              $rootScope.b = NaN;
+
+              element = $compile(template)($rootScope);
+              $rootScope.$digest();
+
+              expect(onChangesSpy).toHaveBeenCalledTimes(2);
+              expect(onChangesSpy.calls.argsFor(0)[0]).toEqual({
+                prop: jasmine.objectContaining({currentValue: 'foo'}),
+                attr: jasmine.objectContaining({currentValue: 'foo'})
+              });
+              expect(onChangesSpy.calls.argsFor(1)[0]).toEqual({
+                prop: jasmine.objectContaining({currentValue: NaN}),
+                attr: jasmine.objectContaining({currentValue: 'NaN'})
+              });
+            });
+          });
+
+
           it('should only trigger one extra digest however many controllers have changes', function() {
             var log = [];
             function TestController1() { }
@@ -4433,7 +4467,6 @@ describe('$compile', function() {
           it('should cope with changes occuring inside `$onChanges()` hooks', function() {
             var log = [];
             function OuterController() {}
-
             OuterController.prototype.$onChanges = function(change) {
               log.push(['OuterController', change]);
               // Make a change to the inner component
@@ -4468,7 +4501,6 @@ describe('$compile', function() {
 
               expect(log).toEqual([
                 ['OuterController', {prop1: jasmine.objectContaining({previousValue: undefined, currentValue: 42})}],
-                ['InnerController', {prop2: jasmine.objectContaining({previousValue: NaN, currentValue: NaN})}],
                 ['InnerController', {prop2: jasmine.objectContaining({previousValue: NaN, currentValue: 84})}]
               ]);
             });
