fix($compile): don't run unnecessary update to one-way bindings

The watch handler was triggering on its first invocation, even though
its change had already been dealt with when setting up the binding.

Closes angular#14546

Author: petebacondarwin

src/ng/compile.js

@@ -3217,11 +3217,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
             initialChanges[scopeName] = new SimpleChange(_UNINITIALIZED_VALUE, destination[scopeName]);
 
             removeWatch = scope.$watch(parentGet, function parentValueWatchAction(newValue, oldValue) {
-              if (newValue === oldValue) {
-                // If the new and old values are identical then this is the first time the watch has been triggered
-                // So instead we use the current value on the destination as the old value
-                oldValue = destination[scopeName];
-              }
+              if (oldValue === newValue) return;
               recordChanges(scopeName, newValue, oldValue);
               destination[scopeName] = newValue;
             }, parentGet.literal);

test/ng/compileSpec.js

@@ -3686,12 +3686,16 @@ describe('$compile', function() {
           $rootScope.$watch('val', function(val, oldVal) { $rootScope.val2 = val * 2; });
           // Setup the directive with two bindings
           element = $compile('<c1 prop1="val" prop2="val2" other="val3" attr="{{val4}}"></c1>')($rootScope);
+          $rootScope.$apply();
 
           expect(log).toEqual([
             {
               prop1: jasmine.objectContaining({currentValue: undefined}),
               prop2: jasmine.objectContaining({currentValue: undefined}),
               attr: jasmine.objectContaining({currentValue: ''})
+            },
+            {
+              prop2: jasmine.objectContaining({currentValue: NaN})
             }
           ]);
 
@@ -3756,6 +3760,7 @@ describe('$compile', function() {
         module('my');
         inject(function($compile, $rootScope) {
           element = $compile('<c1 prop1="val"></c1>')($rootScope);
+          $rootScope.$apply();
 
           $rootScope.$apply('val = 1');
           expect(log.pop()).toEqual({prop1: jasmine.objectContaining({previousValue: undefined, currentValue: 1})});
@@ -3823,6 +3828,8 @@ describe('$compile', function() {
 
           $rootScope.$apply('a = 7');
           element = $compile('<c1 prop="a" attr="{{a}}"></c1>')($rootScope);
+          $rootScope.$apply();
+
           expect(log).toEqual([
             {
               prop: jasmine.objectContaining({currentValue: 7}),
@@ -3862,6 +3869,8 @@ describe('$compile', function() {
         inject(function($compile, $rootScope) {
           $rootScope.$apply('a = 7');
           element = $compile('<c1 prop="a" attr="{{a}}"></c1>')($rootScope);
+          $rootScope.$apply();
+
           expect(log).toEqual([
             {
               prop: jasmine.objectContaining({currentValue: 7}),
@@ -3911,8 +3920,10 @@ describe('$compile', function() {
 
           // Setup two sibling components with bindings that will change
           element = $compile('<div><c1 prop="val1"></c1><c2 prop="val2"></c2></div>')($rootScope);
+          $rootScope.$apply();
 
           // Clear out initial changes
+          watchCount = 0;
           log = [];
 
           // Update val to trigger the onChanges
@@ -3958,6 +3969,7 @@ describe('$compile', function() {
 
           // Setup the directive with two bindings
           element = $compile('<outer prop1="a"></outer>')($rootScope);
+          $rootScope.$apply();
 
           // Clear out initial changes
           log = [];
@@ -3990,6 +4002,7 @@ describe('$compile', function() {
 
           // Setup the directive with bindings that will keep updating the bound value forever
           element = $compile('<c1 prop="a" on-change="a = -a"></c1>')($rootScope);
+          $rootScope.$apply();
 
           // Update val to trigger the unstable onChanges, which will result in an error
           expect(function() {
@@ -4025,6 +4038,7 @@ describe('$compile', function() {
 
           // Setup the directive with bindings that will keep updating the bound value forever
           element = $compile('<c1 prop="a" on-change="a = -a"></c1>')($rootScope);
+          $rootScope.$apply();
 
           // Update val to trigger the unstable onChanges, which will result in an error
           $rootScope.$apply('a = 42');
@@ -4744,6 +4758,8 @@ describe('$compile', function() {
     describe('one-way binding', function() {
       it('should update isolate when the identity of origin changes', inject(function() {
         compile('<div><span my-component ow-ref="obj">');
+        $rootScope.$apply();
+
         expect(componentScope.owRef).toBeUndefined();
         expect(componentScope.owRefAlias).toBe(componentScope.owRef);
 
@@ -4780,6 +4796,8 @@ describe('$compile', function() {
 
       it('should update isolate when both change', inject(function() {
         compile('<div><span my-component ow-ref="name">');
+        $rootScope.$apply();
+
         $rootScope.name = {mark:123};
         componentScope.owRef = 'misko';
 
@@ -4796,6 +4814,35 @@ describe('$compile', function() {
         expect(componentScope.owRefAlias).toBe($rootScope.name);
       }));
 
+      it('should not update isolate again after $onInit if outer has not changed', function() {
+        var log = [];
+        var component;
+        angular.module('owComponentTest', [])
+          .component('owComponent', {
+            bindings: { input: '<' },
+            controller: function() {
+              component = this;
+              this.input = 'constructor';
+              this.$onInit = function() {
+                this.input = '$onInit';
+              };
+              this.$onChanges = function(changes) {
+                if (changes.input) {
+                  log.push(changes.input);
+                }
+              };
+            }
+          });
+        module('owComponentTest');
+        inject(function() {
+          $rootScope.name = 'outer';
+          compile('<ow-component input="name"></ow-component>');
+          $rootScope.$apply();
+
+          expect($rootScope.name).toEqual('outer');
+          expect(component.input).toEqual('$onInit');
+        })
+      });
 
       it('should not break when isolate and origin both change to the same value', inject(function() {
         $rootScope.name = 'aaa';
@@ -4849,6 +4896,8 @@ describe('$compile', function() {
 
       it('should not throw on non assignable expressions in the parent', inject(function() {
         compile('<div><span my-component ow-ref="\'hello \' + name">');
+        $rootScope.$apply();
+
         $rootScope.name = 'world';
         $rootScope.$apply();
         expect(componentScope.owRef).toBe('hello world');
@@ -4879,6 +4928,8 @@ describe('$compile', function() {
       it('should update isolate scope when "<"-bound NaN changes', inject(function() {
         $rootScope.num = NaN;
         compile('<div my-component ow-ref="num"></div>');
+        $rootScope.$apply();
+
         var isolateScope = element.isolateScope();
         expect(isolateScope.owRef).toBeNaN();
 
@@ -4891,6 +4942,7 @@ describe('$compile', function() {
       describe('literal objects', function() {
         it('should copy parent changes', inject(function() {
           compile('<div><span my-component ow-ref="{name: name}">');
+          $rootScope.$apply();
 
           $rootScope.name = 'a';
           $rootScope.$apply();
@@ -4976,6 +5028,8 @@ describe('$compile', function() {
         describe('optional one-way binding', function() {
           it('should update local when origin changes', inject(function() {
             compile('<div><span my-component ow-optref="name">');
+            $rootScope.$apply();
+
             expect(componentScope.owOptref).toBeUndefined();
             expect(componentScope.owOptrefAlias).toBe(componentScope.owOptref);