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

perf($compile): remove use of deepEquals and $stateful interceptor from bidi bindings #16550

Closed
wants to merge 1 commit into from
Closed

perf($compile): remove use of deepEquals and $stateful interceptor from bidi bindings #16550

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 13 additions & 18 deletions src/ng/compile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,11 +309,9 @@
* to the local value, since it will be impossible to sync them back to the parent scope.
*
* By default, the {@link ng.$rootScope.Scope#$watch `$watch`}
* method is used for tracking changes, and the equality check is based on object identity.
* However, if an object literal or an array literal is passed as the binding expression, the
* equality check is done by value (using the {@link angular.equals} function). It's also possible
* to watch the evaluated value shallowly with {@link ng.$rootScope.Scope#$watchCollection
* `$watchCollection`}: use `=*` or `=*attr`
* method is used for tracking changes between the local scope property and the expression passed
* via the attribute. It's also possible to watch the evaluated value shallowly with
* {@link ng.$rootScope.Scope#$watchCollection `$watchCollection`}: use `=*` or `=*attr`.
*
* * `<` or `<attr` - set up a one-way (one-directional) binding between a local scope property and an
* expression passed via the attribute `attr`. The expression is evaluated in the context of the
Expand Down Expand Up @@ -3535,7 +3533,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
optional = definition.optional,
mode = definition.mode, // @, =, <, or &
lastValue,
parentGet, parentSet, compare, removeWatch;
parentGet, parentSet, removeWatch;

switch (mode) {

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

parentGet = $parse(attrs[attrName]);
if (parentGet.literal) {
compare = equals;
} else {
compare = simpleCompare;
}
parentSet = parentGet.assign || function() {
// reset the change, or we will throw this exception on every $digest
lastValue = destination[scopeName] = parentGet(scope);
throw $compileMinErr('nonassign',
'Expression \'{0}\' in attribute \'{1}\' used with directive \'{2}\' is non-assignable!',
attrs[attrName], attrName, directive.name);
};
var childGet = function childGet() { return destination[scopeName]; };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the AngularJS coding style to put the var declarations at the top of the function?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think normally, but not 2 lines below this 🤷‍♂️ Preference?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😨


lastValue = destination[scopeName] = parentGet(scope);
var parentValueWatch = function parentValueWatch(parentValue) {
if (!compare(parentValue, destination[scopeName])) {

var bidiWatchAction = function bidiWatchAction(newValues) {
var parentValue = newValues[0];
if (!simpleCompare(parentValue, destination[scopeName])) {
// we are out of sync and need to copy
if (!compare(parentValue, lastValue)) {
if (!simpleCompare(parentValue, lastValue)) {
// parent changed and it has precedence
destination[scopeName] = parentValue;
} else {
Expand All @@ -3601,13 +3598,11 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
}
}
lastValue = parentValue;
return lastValue;
};
parentValueWatch.$stateful = true;
if (definition.collection) {
removeWatch = scope.$watchCollection(attrs[attrName], parentValueWatch);
removeWatch = scope.$watchCollection(parentGet, function bidiCollectionWatchAction(parentValue) { destination[scopeName] = parentValue; });
} else {
removeWatch = scope.$watch($parse(attrs[attrName], parentValueWatch), null, parentGet.literal);
removeWatch = scope.$watchGroup([parentGet, childGet], bidiWatchAction);
}
removeWatchCollection.push(removeWatch);
break;
Expand Down
170 changes: 161 additions & 9 deletions test/ng/compileSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5416,21 +5416,21 @@ describe('$compile', function() {

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

$rootScope.foo = 'foo';
$rootScope.$digest();
expect(element.html()).toBe('1:foo;2:;3:foo;4:');
expect(countWatches($rootScope)).toEqual(4);
expect(countWatches($rootScope)).toEqual(6);

$rootScope.foo = 'baz';
$rootScope.bar = 'bar';
$rootScope.$digest();
expect(element.html()).toBe('1:foo;2:bar;3:foo;4:bar');
expect(countWatches($rootScope)).toEqual(3);
expect(countWatches($rootScope)).toEqual(5);

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

$rootScope.foo = 'foo';
$rootScope.$digest();
expect(element.html()).toBe('1:foo;2:;3:foo;4:');
expect(countWatches($rootScope)).toEqual(4);
expect(countWatches($rootScope)).toEqual(6);

$rootScope.foo = 'baz';
$rootScope.bar = 'bar';
$rootScope.$digest();
expect(element.html()).toBe('1:foo;2:bar;3:foo;4:bar');
expect(countWatches($rootScope)).toEqual(3);
expect(countWatches($rootScope)).toEqual(5);

$rootScope.bar = 'baz';
$rootScope.$digest();
Expand Down Expand Up @@ -5733,6 +5733,37 @@ describe('$compile', function() {
expect(componentScope.reference).toEqual({name: 'b'});
}));

it('should copy parent changes, even if deep equal to old', inject(function() {
compile('<div><span my-component reference="{person: person}">');

$rootScope.person = {name: 'a'};
$rootScope.$apply();
expect(componentScope.reference).toEqual({person: {name: 'a'}});

var instance = componentScope.reference;
$rootScope.person = {name: 'a'};
$rootScope.$apply();
expect(componentScope.reference).not.toBe(instance);
expect(componentScope.reference).toEqual(instance);
}));

it('should not copy on parent changes to nested objects', inject(function() {
compile('<div><span my-component reference="{subObj: subObj}">');

var subObj = {foo: 42};

$rootScope.subObj = subObj;
$rootScope.$apply();
expect(componentScope.reference).toEqual({subObj: subObj});

var firstVal = componentScope.reference;

$rootScope.subObj.foo = true;
$rootScope.$apply();
expect(componentScope.reference).toBe(firstVal);
expect(componentScope.reference).toEqual({subObj: subObj});
}));

it('should not change the component when parent does not change', inject(function() {
compile('<div><span my-component reference="{name: name}">');

Expand Down Expand Up @@ -5771,6 +5802,46 @@ describe('$compile', function() {
}
}));

it('should work with filtered literal objects within array literals', inject(function() {
$rootScope.gabName = 'Gabriel';
$rootScope.tonyName = 'Tony';
$rootScope.query = '';
$rootScope.$apply();

compile('<div><span my-component reference="[{name: gabName}, {name: tonyName}] | filter:query">');

expect(componentScope.reference).toEqual([{name: $rootScope.gabName}, {name: $rootScope.tonyName}]);

$rootScope.query = 'Gab';
$rootScope.$apply();

expect(componentScope.reference).toEqual([{name: $rootScope.gabName}]);

$rootScope.tonyName = 'Gab';
$rootScope.$apply();

expect(componentScope.reference).toEqual([{name: $rootScope.gabName}, {name: $rootScope.tonyName}]);
}));

it('should work with filtered constant literal objects within array literals (constant)', inject(function() {
$rootScope.query = '';
$rootScope.$apply();

compile('<div><span my-component reference="[{name: \'Gabriel\'}, {name: \'Toni\'}] | filter:query">');

expect(componentScope.reference).toEqual([{name: 'Gabriel'}, {name: 'Toni'}]);

$rootScope.query = 'Gab';
$rootScope.$apply();

expect(componentScope.reference).toEqual([{name: 'Gabriel'}]);

$rootScope.query = 'i';
$rootScope.$apply();

expect(componentScope.reference).toEqual([{name: 'Gabriel'}, {name: 'Toni'}]);
}));

});

});
Expand Down Expand Up @@ -5829,8 +5900,48 @@ describe('$compile', function() {
$rootScope.$apply();

expect(componentScope.colref).toEqual([$rootScope.collection[0]]);
expect(componentScope.colrefAlias).toEqual([$rootScope.collection[0]]);
expect(componentScope.$colrefAlias).toEqual([$rootScope.collection[0]]);
expect(componentScope.colrefAlias).toEqual(componentScope.colref);
expect(componentScope.$colrefAlias).toEqual(componentScope.colref);

$rootScope.collection[1].name = 'Gab';
$rootScope.$apply();

expect(componentScope.colref).toEqual($rootScope.collection);
expect(componentScope.colrefAlias).toEqual(componentScope.colref);
expect(componentScope.$colrefAlias).toEqual(componentScope.colref);
}));

it('should work with filtered objects within a literal collection', inject(function() {
$rootScope.gab = {
name: 'Gabriel',
value: 18
};
$rootScope.tony = {
name: 'Tony',
value: 91
};
$rootScope.query = '';
$rootScope.$apply();

compile('<div><span my-component colref="[gab, tony] | filter:query" $colref$="[gab, tony] | filter:query">');

expect(componentScope.colref).toEqual([$rootScope.gab, $rootScope.tony]);
expect(componentScope.colrefAlias).toEqual(componentScope.colref);
expect(componentScope.$colrefAlias).toEqual(componentScope.colref);

$rootScope.query = 'Gab';
$rootScope.$apply();

expect(componentScope.colref).toEqual([$rootScope.gab]);
expect(componentScope.colrefAlias).toEqual(componentScope.colref);
expect(componentScope.$colrefAlias).toEqual(componentScope.colref);

$rootScope.tony.name = 'Gab';
$rootScope.$apply();

expect(componentScope.colref).toEqual([$rootScope.gab, $rootScope.tony]);
expect(componentScope.colrefAlias).toEqual(componentScope.colref);
expect(componentScope.$colrefAlias).toEqual(componentScope.colref);
}));

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


it('should work with filtered literal objects within array literals', inject(function() {
$rootScope.gabName = 'Gabriel';
$rootScope.tonyName = 'Tony';
$rootScope.query = '';
$rootScope.$apply();

compile('<div><span my-component ow-ref="[{name: gabName}, {name: tonyName}] | filter:query">');

expect(componentScope.owRef).toEqual([{name: $rootScope.gabName}, {name: $rootScope.tonyName}]);

$rootScope.query = 'Gab';
$rootScope.$apply();

expect(componentScope.owRef).toEqual([{name: $rootScope.gabName}]);

$rootScope.tonyName = 'Gab';
$rootScope.$apply();

expect(componentScope.owRef).toEqual([{name: $rootScope.gabName}, {name: $rootScope.tonyName}]);
}));

it('should work with filtered constant literal objects within array literals', inject(function() {
$rootScope.query = '';
$rootScope.$apply();

compile('<div><span my-component ow-ref="[{name: \'Gabriel\'}, {name: \'Toni\'}] | filter:query">');

expect(componentScope.owRef).toEqual([{name: 'Gabriel'}, {name: 'Toni'}]);

$rootScope.query = 'Gab';
$rootScope.$apply();

expect(componentScope.owRef).toEqual([{name: 'Gabriel'}]);

$rootScope.query = 'i';
$rootScope.$apply();

expect(componentScope.owRef).toEqual([{name: 'Gabriel'}, {name: 'Toni'}]);
}));


describe('literal objects', function() {
it('should copy parent changes', inject(function() {
compile('<div><span my-component ow-ref="{name: name}">');
Expand Down