Skip to content

Commit f49b9e9

Browse files
petebacondarwinpetebacondarwin
committed
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
1 parent 87a4a71 commit f49b9e9

File tree

2 files changed

+78
-15
lines changed

2 files changed

+78
-15
lines changed

‎src/ng/compile.js

+5-2
Original file line numberDiff line numberDiff line change
@@ -3213,11 +3213,14 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
32133213

32143214
parentGet = $parse(attrs[attrName]);
32153215

3216-
destination[scopeName] = parentGet(scope);
3216+
var initialValue = destination[scopeName] = parentGet(scope);
32173217
initialChanges[scopeName] = new SimpleChange(_UNINITIALIZED_VALUE, destination[scopeName]);
32183218

32193219
removeWatch = scope.$watch(parentGet, function parentValueWatchAction(newValue, oldValue) {
3220-
if (oldValue === newValue) return;
3220+
if (oldValue === newValue) {
3221+
if (oldValue === initialValue) return;
3222+
oldValue = initialValue;
3223+
}
32213224
recordChanges(scopeName, newValue, oldValue);
32223225
destination[scopeName] = newValue;
32233226
}, parentGet.literal);

‎test/ng/compileSpec.js

+73-13
Original file line numberDiff line numberDiff line change
@@ -4758,7 +4758,6 @@ describe('$compile', function() {
47584758
describe('one-way binding', function() {
47594759
it('should update isolate when the identity of origin changes', inject(function() {
47604760
compile('<div><span my-component ow-ref="obj">');
4761-
$rootScope.$apply();
47624761

47634762
expect(componentScope.owRef).toBeUndefined();
47644763
expect(componentScope.owRefAlias).toBe(componentScope.owRef);
@@ -4796,7 +4795,6 @@ describe('$compile', function() {
47964795

47974796
it('should update isolate when both change', inject(function() {
47984797
compile('<div><span my-component ow-ref="name">');
4799-
$rootScope.$apply();
48004798

48014799
$rootScope.name = {mark:123};
48024800
componentScope.owRef = 'misko';
@@ -4841,7 +4839,6 @@ describe('$compile', function() {
48414839
inject(function() {
48424840
$rootScope.name = 'outer';
48434841
compile('<ow-component input="name"></ow-component>');
4844-
$rootScope.$apply();
48454842

48464843
expect($rootScope.name).toEqual('outer');
48474844
expect(component.input).toEqual('$onInit');
@@ -4853,6 +4850,76 @@ describe('$compile', function() {
48534850
});
48544851
});
48554852

4853+
4854+
it('should update isolate again after $onInit if outer has changed (before initial watchAction call)', function() {
4855+
var log = [];
4856+
var component;
4857+
angular.module('owComponentTest', [])
4858+
.component('owComponent', {
4859+
bindings: { input: '<' },
4860+
controller: function() {
4861+
component = this;
4862+
this.input = 'constructor';
4863+
this.$onInit = function() {
4864+
this.input = '$onInit';
4865+
};
4866+
this.$onChanges = function(changes) {
4867+
if (changes.input) {
4868+
log.push(changes.input);
4869+
}
4870+
};
4871+
}
4872+
});
4873+
module('owComponentTest');
4874+
inject(function() {
4875+
$rootScope.name = 'outer1';
4876+
compile('<ow-component input="name"></ow-component>');
4877+
4878+
expect(component.input).toEqual('$onInit');
4879+
$rootScope.$apply('name = "outer2"');
4880+
4881+
expect($rootScope.name).toEqual('outer2');
4882+
expect(component.input).toEqual('outer2');
4883+
});
4884+
});
4885+
4886+
it('should update isolate again after $onInit if outer has changed (before initial watchAction call)', function() {
4887+
var log = [];
4888+
var component;
4889+
angular.module('owComponentTest', [])
4890+
.directive('changeInput', function() {
4891+
return function(scope, elem, attrs) {
4892+
scope.name = 'outer2';
4893+
};
4894+
})
4895+
.component('owComponent', {
4896+
bindings: { input: '<' },
4897+
controller: function() {
4898+
component = this;
4899+
this.input = 'constructor';
4900+
this.$onInit = function() {
4901+
this.input = '$onInit';
4902+
};
4903+
this.$onChanges = function(changes) {
4904+
if (changes.input) {
4905+
log.push(changes.input);
4906+
}
4907+
};
4908+
}
4909+
});
4910+
module('owComponentTest');
4911+
inject(function() {
4912+
$rootScope.name = 'outer1';
4913+
compile('<ow-component input="name" change-input></ow-component>');
4914+
4915+
expect(component.input).toEqual('$onInit');
4916+
$rootScope.$digest();
4917+
4918+
expect($rootScope.name).toEqual('outer2');
4919+
expect(component.input).toEqual('outer2');
4920+
});
4921+
});
4922+
48564923
it('should not break when isolate and origin both change to the same value', inject(function() {
48574924
$rootScope.name = 'aaa';
48584925
compile('<div><span my-component ow-ref="name">');
@@ -4875,7 +4942,6 @@ describe('$compile', function() {
48754942
$rootScope.name = {mark:123};
48764943
compile('<div><span my-component ow-ref="name">');
48774944

4878-
$rootScope.$apply();
48794945
expect($rootScope.name).toEqual({mark:123});
48804946
expect(componentScope.owRef).toBe($rootScope.name);
48814947
expect(componentScope.owRefAlias).toBe($rootScope.name);
@@ -4892,7 +4958,6 @@ describe('$compile', function() {
48924958
$rootScope.obj = {mark:123};
48934959
compile('<div><span my-component ow-ref="obj">');
48944960

4895-
$rootScope.$apply();
48964961
expect($rootScope.obj).toEqual({mark:123});
48974962
expect(componentScope.owRef).toBe($rootScope.obj);
48984963

@@ -4905,7 +4970,6 @@ describe('$compile', function() {
49054970

49064971
it('should not throw on non assignable expressions in the parent', inject(function() {
49074972
compile('<div><span my-component ow-ref="\'hello \' + name">');
4908-
$rootScope.$apply();
49094973

49104974
$rootScope.name = 'world';
49114975
$rootScope.$apply();
@@ -4923,7 +4987,7 @@ describe('$compile', function() {
49234987

49244988
it('should not throw when assigning to undefined', inject(function() {
49254989
compile('<div><span my-component>');
4926-
$rootScope.$apply();
4990+
49274991
expect(componentScope.owRef).toBeUndefined();
49284992

49294993
componentScope.owRef = 'ignore me';
@@ -4937,7 +5001,6 @@ describe('$compile', function() {
49375001
it('should update isolate scope when "<"-bound NaN changes', inject(function() {
49385002
$rootScope.num = NaN;
49395003
compile('<div my-component ow-ref="num"></div>');
4940-
$rootScope.$apply();
49415004

49425005
var isolateScope = element.isolateScope();
49435006
expect(isolateScope.owRef).toBeNaN();
@@ -4951,7 +5014,6 @@ describe('$compile', function() {
49515014
describe('literal objects', function() {
49525015
it('should copy parent changes', inject(function() {
49535016
compile('<div><span my-component ow-ref="{name: name}">');
4954-
$rootScope.$apply();
49555017

49565018
$rootScope.name = 'a';
49575019
$rootScope.$apply();
@@ -4978,7 +5040,7 @@ describe('$compile', function() {
49785040
$rootScope.name = 'georgios';
49795041
$rootScope.obj = {name: 'pete'};
49805042
compile('<div><span my-component ow-ref="[{name: name}, obj]">');
4981-
$rootScope.$apply();
5043+
49825044
expect(componentScope.owRef).toEqual([{name: 'georgios'}, {name: 'pete'}]);
49835045

49845046
$rootScope.name = 'lucas';
@@ -4992,7 +5054,7 @@ describe('$compile', function() {
49925054
$rootScope.name = 'georgios';
49935055
$rootScope.obj = {name: 'pete'};
49945056
compile('<div><span my-component ow-ref="{name: name, item: obj}">');
4995-
$rootScope.$apply();
5057+
49965058
expect(componentScope.owRef).toEqual({name: 'georgios', item: {name: 'pete'}});
49975059

49985060
$rootScope.name = 'lucas';
@@ -5028,7 +5090,6 @@ describe('$compile', function() {
50285090
function test(literalString, literalValue) {
50295091
compile('<div><span my-component ow-ref="' + literalString + '">');
50305092

5031-
$rootScope.$apply();
50325093
expect(componentScope.owRef).toBe(literalValue);
50335094
dealoc(element);
50345095
}
@@ -5037,7 +5098,6 @@ describe('$compile', function() {
50375098
describe('optional one-way binding', function() {
50385099
it('should update local when origin changes', inject(function() {
50395100
compile('<div><span my-component ow-optref="name">');
5040-
$rootScope.$apply();
50415101

50425102
expect(componentScope.owOptref).toBeUndefined();
50435103
expect(componentScope.owOptrefAlias).toBe(componentScope.owOptref);

0 commit comments

Comments
 (0)