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

Commit b3b514e

Browse files
author
Jason Bedard
committed
perf($compile): remove use of deepEquals and $stateful interceptor from bidi bindings
Previously bidirectional bindings were implemented using a custom watch interceptor which stored the component/directive inner value and therefore had to be marked as `$stateful`. The use of a `$stateful` interceptor prevented some optimizations from being used. As a result of using a `$stateful` interceptor the bindings would be executed on each and every digest. For literal expressions this would recreate the literal every digest and therefore require `objectEquality = true`, causing a `angular.equals` call on every digest.
1 parent fb2d3fa commit b3b514e

File tree

2 files changed

+174
-27
lines changed

2 files changed

+174
-27
lines changed

‎src/ng/compile.js

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -309,11 +309,9 @@
309309
* to the local value, since it will be impossible to sync them back to the parent scope.
310310
*
311311
* By default, the {@link ng.$rootScope.Scope#$watch `$watch`}
312-
* method is used for tracking changes, and the equality check is based on object identity.
313-
* However, if an object literal or an array literal is passed as the binding expression, the
314-
* equality check is done by value (using the {@link angular.equals} function). It's also possible
315-
* to watch the evaluated value shallowly with {@link ng.$rootScope.Scope#$watchCollection
316-
* `$watchCollection`}: use `=*` or `=*attr`
312+
* method is used for tracking changes between the local scope property and the expression passed
313+
* via the attribute. It's also possible to watch the evaluated value shallowly with
314+
* {@link ng.$rootScope.Scope#$watchCollection `$watchCollection`}: use `=*` or `=*attr`.
317315
*
318316
* * `<` or `<attr` - set up a one-way (one-directional) binding between a local scope property and an
319317
* expression passed via the attribute `attr`. The expression is evaluated in the context of the
@@ -3535,7 +3533,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
35353533
optional = definition.optional,
35363534
mode = definition.mode, // @, =, <, or &
35373535
lastValue,
3538-
parentGet, parentSet, compare, removeWatch;
3536+
parentGet, parentSet, removeWatch;
35393537

35403538
switch (mode) {
35413539

@@ -3576,23 +3574,22 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
35763574
if (optional && !attrs[attrName]) break;
35773575

35783576
parentGet = $parse(attrs[attrName]);
3579-
if (parentGet.literal) {
3580-
compare = equals;
3581-
} else {
3582-
compare = simpleCompare;
3583-
}
35843577
parentSet = parentGet.assign || function() {
35853578
// reset the change, or we will throw this exception on every $digest
35863579
lastValue = destination[scopeName] = parentGet(scope);
35873580
throw $compileMinErr('nonassign',
35883581
'Expression \'{0}\' in attribute \'{1}\' used with directive \'{2}\' is non-assignable!',
35893582
attrs[attrName], attrName, directive.name);
35903583
};
3584+
var childGet = function childGet() { return destination[scopeName]; };
3585+
35913586
lastValue = destination[scopeName] = parentGet(scope);
3592-
var parentValueWatch = function parentValueWatch(parentValue) {
3593-
if (!compare(parentValue, destination[scopeName])) {
3587+
3588+
var bidiWatchAction = function bidiWatchAction(newValues) {
3589+
var parentValue = newValues[0];
3590+
if (!simpleCompare(parentValue, destination[scopeName])) {
35943591
// we are out of sync and need to copy
3595-
if (!compare(parentValue, lastValue)) {
3592+
if (!simpleCompare(parentValue, lastValue)) {
35963593
// parent changed and it has precedence
35973594
destination[scopeName] = parentValue;
35983595
} else {
@@ -3601,13 +3598,11 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
36013598
}
36023599
}
36033600
lastValue = parentValue;
3604-
return lastValue;
36053601
};
3606-
parentValueWatch.$stateful = true;
36073602
if (definition.collection) {
3608-
removeWatch = scope.$watchCollection(attrs[attrName], parentValueWatch);
3603+
removeWatch = scope.$watchCollection(parentGet, function bidiCollectionWatchAction(parentValue) { destination[scopeName] = parentValue; });
36093604
} else {
3610-
removeWatch = scope.$watch($parse(attrs[attrName], parentValueWatch), null, parentGet.literal);
3605+
removeWatch = scope.$watchGroup([parentGet, childGet], bidiWatchAction);
36113606
}
36123607
removeWatchCollection.push(removeWatch);
36133608
break;

‎test/ng/compileSpec.js

Lines changed: 161 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5416,21 +5416,21 @@ describe('$compile', function() {
54165416

54175417
inject(function($rootScope) {
54185418
compile('<div other-tpl-dir param1="::foo" param2="bar"></div>');
5419-
expect(countWatches($rootScope)).toEqual(6); // 4 -> template watch group, 2 -> '='
5419+
expect(countWatches($rootScope)).toEqual(8); // 4 -> template watch group, 4 -> '='
54205420
$rootScope.$digest();
54215421
expect(element.html()).toBe('1:;2:;3:;4:');
5422-
expect(countWatches($rootScope)).toEqual(6);
5422+
expect(countWatches($rootScope)).toEqual(8);
54235423

54245424
$rootScope.foo = 'foo';
54255425
$rootScope.$digest();
54265426
expect(element.html()).toBe('1:foo;2:;3:foo;4:');
5427-
expect(countWatches($rootScope)).toEqual(4);
5427+
expect(countWatches($rootScope)).toEqual(6);
54285428

54295429
$rootScope.foo = 'baz';
54305430
$rootScope.bar = 'bar';
54315431
$rootScope.$digest();
54325432
expect(element.html()).toBe('1:foo;2:bar;3:foo;4:bar');
5433-
expect(countWatches($rootScope)).toEqual(3);
5433+
expect(countWatches($rootScope)).toEqual(5);
54345434

54355435
$rootScope.bar = 'baz';
54365436
$rootScope.$digest();
@@ -5487,18 +5487,18 @@ describe('$compile', function() {
54875487
compile('<div other-tpl-dir param1="::foo" param2="bar"></div>');
54885488
$rootScope.$digest();
54895489
expect(element.html()).toBe('1:;2:;3:;4:');
5490-
expect(countWatches($rootScope)).toEqual(6); // 4 -> template watch group, 2 -> '='
5490+
expect(countWatches($rootScope)).toEqual(8); // 4 -> template watch group, 4 -> '='
54915491

54925492
$rootScope.foo = 'foo';
54935493
$rootScope.$digest();
54945494
expect(element.html()).toBe('1:foo;2:;3:foo;4:');
5495-
expect(countWatches($rootScope)).toEqual(4);
5495+
expect(countWatches($rootScope)).toEqual(6);
54965496

54975497
$rootScope.foo = 'baz';
54985498
$rootScope.bar = 'bar';
54995499
$rootScope.$digest();
55005500
expect(element.html()).toBe('1:foo;2:bar;3:foo;4:bar');
5501-
expect(countWatches($rootScope)).toEqual(3);
5501+
expect(countWatches($rootScope)).toEqual(5);
55025502

55035503
$rootScope.bar = 'baz';
55045504
$rootScope.$digest();
@@ -5733,6 +5733,37 @@ describe('$compile', function() {
57335733
expect(componentScope.reference).toEqual({name: 'b'});
57345734
}));
57355735

5736+
it('should copy parent changes, even if deep equal to old', inject(function() {
5737+
compile('<div><span my-component reference="{person: person}">');
5738+
5739+
$rootScope.person = {name: 'a'};
5740+
$rootScope.$apply();
5741+
expect(componentScope.reference).toEqual({person: {name: 'a'}});
5742+
5743+
var instance = componentScope.reference;
5744+
$rootScope.person = {name: 'a'};
5745+
$rootScope.$apply();
5746+
expect(componentScope.reference).not.toBe(instance);
5747+
expect(componentScope.reference).toEqual(instance);
5748+
}));
5749+
5750+
it('should not copy on parent changes to nested objects', inject(function() {
5751+
compile('<div><span my-component reference="{subObj: subObj}">');
5752+
5753+
var subObj = {foo: 42};
5754+
5755+
$rootScope.subObj = subObj;
5756+
$rootScope.$apply();
5757+
expect(componentScope.reference).toEqual({subObj: subObj});
5758+
5759+
var firstVal = componentScope.reference;
5760+
5761+
$rootScope.subObj.foo = true;
5762+
$rootScope.$apply();
5763+
expect(componentScope.reference).toBe(firstVal);
5764+
expect(componentScope.reference).toEqual({subObj: subObj});
5765+
}));
5766+
57365767
it('should not change the component when parent does not change', inject(function() {
57375768
compile('<div><span my-component reference="{name: name}">');
57385769

@@ -5771,6 +5802,46 @@ describe('$compile', function() {
57715802
}
57725803
}));
57735804

5805+
it('should work with filtered literal objects within array literals', inject(function() {
5806+
$rootScope.gabName = 'Gabriel';
5807+
$rootScope.tonyName = 'Tony';
5808+
$rootScope.query = '';
5809+
$rootScope.$apply();
5810+
5811+
compile('<div><span my-component reference="[{name: gabName}, {name: tonyName}] | filter:query">');
5812+
5813+
expect(componentScope.reference).toEqual([{name: $rootScope.gabName}, {name: $rootScope.tonyName}]);
5814+
5815+
$rootScope.query = 'Gab';
5816+
$rootScope.$apply();
5817+
5818+
expect(componentScope.reference).toEqual([{name: $rootScope.gabName}]);
5819+
5820+
$rootScope.tonyName = 'Gab';
5821+
$rootScope.$apply();
5822+
5823+
expect(componentScope.reference).toEqual([{name: $rootScope.gabName}, {name: $rootScope.tonyName}]);
5824+
}));
5825+
5826+
it('should work with filtered constant literal objects within array literals (constant)', inject(function() {
5827+
$rootScope.query = '';
5828+
$rootScope.$apply();
5829+
5830+
compile('<div><span my-component reference="[{name: \'Gabriel\'}, {name: \'Toni\'}] | filter:query">');
5831+
5832+
expect(componentScope.reference).toEqual([{name: 'Gabriel'}, {name: 'Toni'}]);
5833+
5834+
$rootScope.query = 'Gab';
5835+
$rootScope.$apply();
5836+
5837+
expect(componentScope.reference).toEqual([{name: 'Gabriel'}]);
5838+
5839+
$rootScope.query = 'i';
5840+
$rootScope.$apply();
5841+
5842+
expect(componentScope.reference).toEqual([{name: 'Gabriel'}, {name: 'Toni'}]);
5843+
}));
5844+
57745845
});
57755846

57765847
});
@@ -5829,8 +5900,48 @@ describe('$compile', function() {
58295900
$rootScope.$apply();
58305901

58315902
expect(componentScope.colref).toEqual([$rootScope.collection[0]]);
5832-
expect(componentScope.colrefAlias).toEqual([$rootScope.collection[0]]);
5833-
expect(componentScope.$colrefAlias).toEqual([$rootScope.collection[0]]);
5903+
expect(componentScope.colrefAlias).toEqual(componentScope.colref);
5904+
expect(componentScope.$colrefAlias).toEqual(componentScope.colref);
5905+
5906+
$rootScope.collection[1].name = 'Gab';
5907+
$rootScope.$apply();
5908+
5909+
expect(componentScope.colref).toEqual($rootScope.collection);
5910+
expect(componentScope.colrefAlias).toEqual(componentScope.colref);
5911+
expect(componentScope.$colrefAlias).toEqual(componentScope.colref);
5912+
}));
5913+
5914+
it('should work with filtered objects within a literal collection', inject(function() {
5915+
$rootScope.gab = {
5916+
name: 'Gabriel',
5917+
value: 18
5918+
};
5919+
$rootScope.tony = {
5920+
name: 'Tony',
5921+
value: 91
5922+
};
5923+
$rootScope.query = '';
5924+
$rootScope.$apply();
5925+
5926+
compile('<div><span my-component colref="[gab, tony] | filter:query" $colref$="[gab, tony] | filter:query">');
5927+
5928+
expect(componentScope.colref).toEqual([$rootScope.gab, $rootScope.tony]);
5929+
expect(componentScope.colrefAlias).toEqual(componentScope.colref);
5930+
expect(componentScope.$colrefAlias).toEqual(componentScope.colref);
5931+
5932+
$rootScope.query = 'Gab';
5933+
$rootScope.$apply();
5934+
5935+
expect(componentScope.colref).toEqual([$rootScope.gab]);
5936+
expect(componentScope.colrefAlias).toEqual(componentScope.colref);
5937+
expect(componentScope.$colrefAlias).toEqual(componentScope.colref);
5938+
5939+
$rootScope.tony.name = 'Gab';
5940+
$rootScope.$apply();
5941+
5942+
expect(componentScope.colref).toEqual([$rootScope.gab, $rootScope.tony]);
5943+
expect(componentScope.colrefAlias).toEqual(componentScope.colref);
5944+
expect(componentScope.$colrefAlias).toEqual(componentScope.colref);
58345945
}));
58355946

58365947
it('should update origin scope when isolate scope changes', inject(function() {
@@ -6187,6 +6298,47 @@ describe('$compile', function() {
61876298
}));
61886299

61896300

6301+
it('should work with filtered literal objects within array literals', inject(function() {
6302+
$rootScope.gabName = 'Gabriel';
6303+
$rootScope.tonyName = 'Tony';
6304+
$rootScope.query = '';
6305+
$rootScope.$apply();
6306+
6307+
compile('<div><span my-component ow-ref="[{name: gabName}, {name: tonyName}] | filter:query">');
6308+
6309+
expect(componentScope.owRef).toEqual([{name: $rootScope.gabName}, {name: $rootScope.tonyName}]);
6310+
6311+
$rootScope.query = 'Gab';
6312+
$rootScope.$apply();
6313+
6314+
expect(componentScope.owRef).toEqual([{name: $rootScope.gabName}]);
6315+
6316+
$rootScope.tonyName = 'Gab';
6317+
$rootScope.$apply();
6318+
6319+
expect(componentScope.owRef).toEqual([{name: $rootScope.gabName}, {name: $rootScope.tonyName}]);
6320+
}));
6321+
6322+
it('should work with filtered constant literal objects within array literals', inject(function() {
6323+
$rootScope.query = '';
6324+
$rootScope.$apply();
6325+
6326+
compile('<div><span my-component ow-ref="[{name: \'Gabriel\'}, {name: \'Toni\'}] | filter:query">');
6327+
6328+
expect(componentScope.owRef).toEqual([{name: 'Gabriel'}, {name: 'Toni'}]);
6329+
6330+
$rootScope.query = 'Gab';
6331+
$rootScope.$apply();
6332+
6333+
expect(componentScope.owRef).toEqual([{name: 'Gabriel'}]);
6334+
6335+
$rootScope.query = 'i';
6336+
$rootScope.$apply();
6337+
6338+
expect(componentScope.owRef).toEqual([{name: 'Gabriel'}, {name: 'Toni'}]);
6339+
}));
6340+
6341+
61906342
describe('literal objects', function() {
61916343
it('should copy parent changes', inject(function() {
61926344
compile('<div><span my-component ow-ref="{name: name}">');

0 commit comments

Comments
 (0)