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

perf(ngClass): optimize the case of static map of classes of large objects #14394

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 8 additions & 2 deletions src/ng/directive/ngClass.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
'use strict';

function classDirective(name, selector) {
var staticMapClassRegEx = /^\s*(::)?\s*\{/;
name = 'ngClass' + name;
return ['$animate', function($animate) {
return {
restrict: 'AC',
link: function(scope, element, attr) {
var oldVal;

scope.$watch(attr[name], ngClassWatchAction, true);
// shortcut: if it is clearly a map of classes do not copy values, they are supposed to be boolean (truly/falsy)
scope[staticMapClassRegEx.test(attr[name]) ? '$watchCollection' : '$watch'](attr[name], ngClassWatchAction, true);

attr.$observe('class', function(value) {
ngClassWatchAction(scope.$eval(attr[name]));
Expand Down Expand Up @@ -78,7 +80,11 @@ function classDirective(name, selector) {
updateClasses(oldClasses, newClasses);
}
}
oldVal = shallowCopy(newVal);
if (isArray(newVal)) {
oldVal = newVal.map(function(v) { return shallowCopy(v); });
} else {
oldVal = shallowCopy(newVal);
}
}
}
};
Expand Down
41 changes: 41 additions & 0 deletions test/ng/directive/ngClassSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,47 @@ describe('ngClass', function() {
expect(e2.hasClass('even')).toBeTruthy();
expect(e2.hasClass('odd')).toBeFalsy();
}));

it('should support mixed array/object variable with a mutating object', inject(function($rootScope, $compile) {
$rootScope.classVar = ['', {orange: true}];
element = $compile('<div class="existing" ng-class="classVar"></div>')($rootScope);
$rootScope.$digest();

$rootScope.classVar[1].orange = false;
$rootScope.$digest();

expect(element.hasClass('orange')).toBeFalsy();
}));

describe('large objects', function() {

var verylargeobject, getProp;
beforeEach(function() {
getProp = jasmine.createSpy('getProp');
verylargeobject = {};
Object.defineProperty(verylargeobject, 'prop', {
get: getProp,
enumerable: true
});
});

it('should not copy large objects via hard map of classes', inject(function($rootScope, $compile) {
element = $compile('<div ng-class="{foo: verylargeobject}"></div>')($rootScope);
$rootScope.verylargeobject = verylargeobject;
$rootScope.$digest();

expect(getProp).not.toHaveBeenCalled();
}));

it('should not copy large objects via hard map of classes in one-time binding', inject(function($rootScope, $compile) {
element = $compile('<div ng-class="::{foo: verylargeobject}"></div>')($rootScope);
$rootScope.verylargeobject = verylargeobject;
$rootScope.$digest();

expect(getProp).not.toHaveBeenCalled();
}));
});

});

describe('ngClass animations', function() {
Expand Down