Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit 60909c8

Browse files
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 #14546
1 parent 824ce30 commit 60909c8

File tree

2 files changed

+121
-11
lines changed

2 files changed

+121
-11
lines changed

src/ng/compile.js

+4-5
Original file line numberDiff line numberDiff line change
@@ -3213,14 +3213,13 @@ 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 (newValue === oldValue) {
3221-
// If the new and old values are identical then this is the first time the watch has been triggered
3222-
// So instead we use the current value on the destination as the old value
3223-
oldValue = destination[scopeName];
3220+
if (oldValue === newValue) {
3221+
if (oldValue === initialValue) return;
3222+
oldValue = initialValue;
32243223
}
32253224
recordChanges(scopeName, newValue, oldValue);
32263225
destination[scopeName] = newValue;

test/ng/compileSpec.js

+117-6
Original file line numberDiff line numberDiff line change
@@ -3823,6 +3823,7 @@ describe('$compile', function() {
38233823

38243824
$rootScope.$apply('a = 7');
38253825
element = $compile('<c1 prop="a" attr="{{a}}"></c1>')($rootScope);
3826+
38263827
expect(log).toEqual([
38273828
{
38283829
prop: jasmine.objectContaining({currentValue: 7}),
@@ -3862,6 +3863,7 @@ describe('$compile', function() {
38623863
inject(function($compile, $rootScope) {
38633864
$rootScope.$apply('a = 7');
38643865
element = $compile('<c1 prop="a" attr="{{a}}"></c1>')($rootScope);
3866+
38653867
expect(log).toEqual([
38663868
{
38673869
prop: jasmine.objectContaining({currentValue: 7}),
@@ -4744,6 +4746,7 @@ describe('$compile', function() {
47444746
describe('one-way binding', function() {
47454747
it('should update isolate when the identity of origin changes', inject(function() {
47464748
compile('<div><span my-component ow-ref="obj">');
4749+
47474750
expect(componentScope.owRef).toBeUndefined();
47484751
expect(componentScope.owRefAlias).toBe(componentScope.owRef);
47494752

@@ -4780,6 +4783,7 @@ describe('$compile', function() {
47804783

47814784
it('should update isolate when both change', inject(function() {
47824785
compile('<div><span my-component ow-ref="name">');
4786+
47834787
$rootScope.name = {mark:123};
47844788
componentScope.owRef = 'misko';
47854789

@@ -4796,6 +4800,113 @@ describe('$compile', function() {
47964800
expect(componentScope.owRefAlias).toBe($rootScope.name);
47974801
}));
47984802

4803+
it('should not update isolate again after $onInit if outer has not changed', function() {
4804+
var log = [];
4805+
var component;
4806+
angular.module('owComponentTest', [])
4807+
.component('owComponent', {
4808+
bindings: { input: '<' },
4809+
controller: function() {
4810+
component = this;
4811+
this.input = 'constructor';
4812+
log.push('constructor');
4813+
4814+
this.$onInit = function() {
4815+
this.input = '$onInit';
4816+
log.push('$onInit');
4817+
};
4818+
4819+
this.$onChanges = function(changes) {
4820+
if (changes.input) {
4821+
log.push(['$onChanges', changes.input]);
4822+
}
4823+
};
4824+
}
4825+
});
4826+
module('owComponentTest');
4827+
inject(function() {
4828+
$rootScope.name = 'outer';
4829+
compile('<ow-component input="name"></ow-component>');
4830+
4831+
expect($rootScope.name).toEqual('outer');
4832+
expect(component.input).toEqual('$onInit');
4833+
expect(log).toEqual([
4834+
'constructor',
4835+
['$onChanges', jasmine.objectContaining({ currentValue: 'outer' })],
4836+
'$onInit'
4837+
]);
4838+
});
4839+
});
4840+
4841+
4842+
it('should update isolate again after $onInit if outer has changed (before initial watchAction call)', function() {
4843+
var log = [];
4844+
var component;
4845+
angular.module('owComponentTest', [])
4846+
.component('owComponent', {
4847+
bindings: { input: '<' },
4848+
controller: function() {
4849+
component = this;
4850+
this.input = 'constructor';
4851+
this.$onInit = function() {
4852+
this.input = '$onInit';
4853+
};
4854+
this.$onChanges = function(changes) {
4855+
if (changes.input) {
4856+
log.push(changes.input);
4857+
}
4858+
};
4859+
}
4860+
});
4861+
module('owComponentTest');
4862+
inject(function() {
4863+
$rootScope.name = 'outer1';
4864+
compile('<ow-component input="name"></ow-component>');
4865+
4866+
expect(component.input).toEqual('$onInit');
4867+
$rootScope.$apply('name = "outer2"');
4868+
4869+
expect($rootScope.name).toEqual('outer2');
4870+
expect(component.input).toEqual('outer2');
4871+
});
4872+
});
4873+
4874+
it('should update isolate again after $onInit if outer has changed (before initial watchAction call)', function() {
4875+
var log = [];
4876+
var component;
4877+
angular.module('owComponentTest', [])
4878+
.directive('changeInput', function() {
4879+
return function(scope, elem, attrs) {
4880+
scope.name = 'outer2';
4881+
};
4882+
})
4883+
.component('owComponent', {
4884+
bindings: { input: '<' },
4885+
controller: function() {
4886+
component = this;
4887+
this.input = 'constructor';
4888+
this.$onInit = function() {
4889+
this.input = '$onInit';
4890+
};
4891+
this.$onChanges = function(changes) {
4892+
if (changes.input) {
4893+
log.push(changes.input);
4894+
}
4895+
};
4896+
}
4897+
});
4898+
module('owComponentTest');
4899+
inject(function() {
4900+
$rootScope.name = 'outer1';
4901+
compile('<ow-component input="name" change-input></ow-component>');
4902+
4903+
expect(component.input).toEqual('$onInit');
4904+
$rootScope.$digest();
4905+
4906+
expect($rootScope.name).toEqual('outer2');
4907+
expect(component.input).toEqual('outer2');
4908+
});
4909+
});
47994910

48004911
it('should not break when isolate and origin both change to the same value', inject(function() {
48014912
$rootScope.name = 'aaa';
@@ -4819,7 +4930,6 @@ describe('$compile', function() {
48194930
$rootScope.name = {mark:123};
48204931
compile('<div><span my-component ow-ref="name">');
48214932

4822-
$rootScope.$apply();
48234933
expect($rootScope.name).toEqual({mark:123});
48244934
expect(componentScope.owRef).toBe($rootScope.name);
48254935
expect(componentScope.owRefAlias).toBe($rootScope.name);
@@ -4836,7 +4946,6 @@ describe('$compile', function() {
48364946
$rootScope.obj = {mark:123};
48374947
compile('<div><span my-component ow-ref="obj">');
48384948

4839-
$rootScope.$apply();
48404949
expect($rootScope.obj).toEqual({mark:123});
48414950
expect(componentScope.owRef).toBe($rootScope.obj);
48424951

@@ -4849,6 +4958,7 @@ describe('$compile', function() {
48494958

48504959
it('should not throw on non assignable expressions in the parent', inject(function() {
48514960
compile('<div><span my-component ow-ref="\'hello \' + name">');
4961+
48524962
$rootScope.name = 'world';
48534963
$rootScope.$apply();
48544964
expect(componentScope.owRef).toBe('hello world');
@@ -4865,7 +4975,7 @@ describe('$compile', function() {
48654975

48664976
it('should not throw when assigning to undefined', inject(function() {
48674977
compile('<div><span my-component>');
4868-
$rootScope.$apply();
4978+
48694979
expect(componentScope.owRef).toBeUndefined();
48704980

48714981
componentScope.owRef = 'ignore me';
@@ -4879,6 +4989,7 @@ describe('$compile', function() {
48794989
it('should update isolate scope when "<"-bound NaN changes', inject(function() {
48804990
$rootScope.num = NaN;
48814991
compile('<div my-component ow-ref="num"></div>');
4992+
48824993
var isolateScope = element.isolateScope();
48834994
expect(isolateScope.owRef).toBeNaN();
48844995

@@ -4917,7 +5028,7 @@ describe('$compile', function() {
49175028
$rootScope.name = 'georgios';
49185029
$rootScope.obj = {name: 'pete'};
49195030
compile('<div><span my-component ow-ref="[{name: name}, obj]">');
4920-
$rootScope.$apply();
5031+
49215032
expect(componentScope.owRef).toEqual([{name: 'georgios'}, {name: 'pete'}]);
49225033

49235034
$rootScope.name = 'lucas';
@@ -4931,7 +5042,7 @@ describe('$compile', function() {
49315042
$rootScope.name = 'georgios';
49325043
$rootScope.obj = {name: 'pete'};
49335044
compile('<div><span my-component ow-ref="{name: name, item: obj}">');
4934-
$rootScope.$apply();
5045+
49355046
expect(componentScope.owRef).toEqual({name: 'georgios', item: {name: 'pete'}});
49365047

49375048
$rootScope.name = 'lucas';
@@ -4967,7 +5078,6 @@ describe('$compile', function() {
49675078
function test(literalString, literalValue) {
49685079
compile('<div><span my-component ow-ref="' + literalString + '">');
49695080

4970-
$rootScope.$apply();
49715081
expect(componentScope.owRef).toBe(literalValue);
49725082
dealoc(element);
49735083
}
@@ -4976,6 +5086,7 @@ describe('$compile', function() {
49765086
describe('optional one-way binding', function() {
49775087
it('should update local when origin changes', inject(function() {
49785088
compile('<div><span my-component ow-optref="name">');
5089+
49795090
expect(componentScope.owOptref).toBeUndefined();
49805091
expect(componentScope.owOptrefAlias).toBe(componentScope.owOptref);
49815092

0 commit comments

Comments
 (0)