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

Commit 3047964

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 #14546 Closes #14580
1 parent 225afb9 commit 3047964

File tree

2 files changed

+109
-11
lines changed

2 files changed

+109
-11
lines changed

‎src/ng/compile.js‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3226,14 +3226,13 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
32263226

32273227
parentGet = $parse(attrs[attrName]);
32283228

3229-
destination[scopeName] = parentGet(scope);
3229+
var initialValue = destination[scopeName] = parentGet(scope);
32303230
initialChanges[scopeName] = new SimpleChange(_UNINITIALIZED_VALUE, destination[scopeName]);
32313231

32323232
removeWatch = scope.$watch(parentGet, function parentValueWatchAction(newValue, oldValue) {
3233-
if (newValue === oldValue) {
3234-
// If the new and old values are identical then this is the first time the watch has been triggered
3235-
// So instead we use the current value on the destination as the old value
3236-
oldValue = destination[scopeName];
3233+
if (oldValue === newValue) {
3234+
if (oldValue === initialValue) return;
3235+
oldValue = initialValue;
32373236
}
32383237
recordChanges(scopeName, newValue, oldValue);
32393238
destination[scopeName] = newValue;

‎test/ng/compileSpec.js‎

Lines changed: 105 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3822,6 +3822,7 @@ describe('$compile', function() {
38223822

38233823
$rootScope.$apply('a = 7');
38243824
element = $compile('<c1 prop="a" attr="{{a}}"></c1>')($rootScope);
3825+
38253826
expect(log).toEqual([
38263827
{
38273828
prop: jasmine.objectContaining({currentValue: 7}),
@@ -3861,6 +3862,7 @@ describe('$compile', function() {
38613862
inject(function($compile, $rootScope) {
38623863
$rootScope.$apply('a = 7');
38633864
element = $compile('<c1 prop="a" attr="{{a}}"></c1>')($rootScope);
3865+
38643866
expect(log).toEqual([
38653867
{
38663868
prop: jasmine.objectContaining({currentValue: 7}),
@@ -4743,6 +4745,7 @@ describe('$compile', function() {
47434745
describe('one-way binding', function() {
47444746
it('should update isolate when the identity of origin changes', inject(function() {
47454747
compile('<div><span my-component ow-ref="obj">');
4748+
47464749
expect(componentScope.owRef).toBeUndefined();
47474750
expect(componentScope.owRefAlias).toBe(componentScope.owRef);
47484751

@@ -4779,6 +4782,7 @@ describe('$compile', function() {
47794782

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

@@ -4795,6 +4799,101 @@ describe('$compile', function() {
47954799
expect(componentScope.owRefAlias).toBe($rootScope.name);
47964800
}));
47974801

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

47994898
it('should not break when isolate and origin both change to the same value', inject(function() {
48004899
$rootScope.name = 'aaa';
@@ -4818,7 +4917,6 @@ describe('$compile', function() {
48184917
$rootScope.name = {mark:123};
48194918
compile('<div><span my-component ow-ref="name">');
48204919

4821-
$rootScope.$apply();
48224920
expect($rootScope.name).toEqual({mark:123});
48234921
expect(componentScope.owRef).toBe($rootScope.name);
48244922
expect(componentScope.owRefAlias).toBe($rootScope.name);
@@ -4835,7 +4933,6 @@ describe('$compile', function() {
48354933
$rootScope.obj = {mark:123};
48364934
compile('<div><span my-component ow-ref="obj">');
48374935

4838-
$rootScope.$apply();
48394936
expect($rootScope.obj).toEqual({mark:123});
48404937
expect(componentScope.owRef).toBe($rootScope.obj);
48414938

@@ -4848,6 +4945,7 @@ describe('$compile', function() {
48484945

48494946
it('should not throw on non assignable expressions in the parent', inject(function() {
48504947
compile('<div><span my-component ow-ref="\'hello \' + name">');
4948+
48514949
$rootScope.name = 'world';
48524950
$rootScope.$apply();
48534951
expect(componentScope.owRef).toBe('hello world');
@@ -4864,7 +4962,7 @@ describe('$compile', function() {
48644962

48654963
it('should not throw when assigning to undefined', inject(function() {
48664964
compile('<div><span my-component>');
4867-
$rootScope.$apply();
4965+
48684966
expect(componentScope.owRef).toBeUndefined();
48694967

48704968
componentScope.owRef = 'ignore me';
@@ -4878,6 +4976,7 @@ describe('$compile', function() {
48784976
it('should update isolate scope when "<"-bound NaN changes', inject(function() {
48794977
$rootScope.num = NaN;
48804978
compile('<div my-component ow-ref="num"></div>');
4979+
48814980
var isolateScope = element.isolateScope();
48824981
expect(isolateScope.owRef).toBeNaN();
48834982

@@ -4916,7 +5015,7 @@ describe('$compile', function() {
49165015
$rootScope.name = 'georgios';
49175016
$rootScope.obj = {name: 'pete'};
49185017
compile('<div><span my-component ow-ref="[{name: name}, obj]">');
4919-
$rootScope.$apply();
5018+
49205019
expect(componentScope.owRef).toEqual([{name: 'georgios'}, {name: 'pete'}]);
49215020

49225021
$rootScope.name = 'lucas';
@@ -4930,7 +5029,7 @@ describe('$compile', function() {
49305029
$rootScope.name = 'georgios';
49315030
$rootScope.obj = {name: 'pete'};
49325031
compile('<div><span my-component ow-ref="{name: name, item: obj}">');
4933-
$rootScope.$apply();
5032+
49345033
expect(componentScope.owRef).toEqual({name: 'georgios', item: {name: 'pete'}});
49355034

49365035
$rootScope.name = 'lucas';
@@ -4966,7 +5065,6 @@ describe('$compile', function() {
49665065
function test(literalString, literalValue) {
49675066
compile('<div><span my-component ow-ref="' + literalString + '">');
49685067

4969-
$rootScope.$apply();
49705068
expect(componentScope.owRef).toBe(literalValue);
49715069
dealoc(element);
49725070
}
@@ -4975,6 +5073,7 @@ describe('$compile', function() {
49755073
describe('optional one-way binding', function() {
49765074
it('should update local when origin changes', inject(function() {
49775075
compile('<div><span my-component ow-optref="name">');
5076+
49785077
expect(componentScope.owOptref).toBeUndefined();
49795078
expect(componentScope.owOptrefAlias).toBe(componentScope.owOptref);
49805079

0 commit comments

Comments
 (0)