Skip to content

Commit c371e1d

Browse files
gkalpakgkalpak
committed
perf(ngClass): avoid deep-watching (if possible) and unnecessary copies
The cases that should benefit most are: 1. When using large objects as keys (e.g.: `{loaded: $ctrl.data}`). 2. When using objects/arrays and there are frequent changes. 3. When there are many `$index` changes (e.g. addition/deletion/reordering in large `ngRepeat`s). The differences in operations per digest include: 1. `Regular expression (when not changed)` **Before:** `equals()` **After:** `toClassString()` 2. `Regular expression (when changed)` **Before:** `copy()` + 2 x `arrayClasses()` + `shallowCopy()` **After:** 2 x `split()` 3. `One-time expression (when not changed)` **Before:** `equals()` **After:** `toFlatValue()` + `equals()`* 4. `One-time expression (when changed)` **Before:** `copy()` + 2 x `arrayClasses()` + `shallowCopy()` **After:** `copy()`* + `toClassString()`* + 2 x `split()` 5. `$index modulo changed` **Before:** `arrayClasses()` **After:** - (*): on flatter structure In large based on angular#14404. Kudos to @drpicox for the initial idea and a big part of the implementation. Closes angular#14404
1 parent 37cec7a commit c371e1d

File tree

2 files changed

+154
-49
lines changed

2 files changed

+154
-49
lines changed

‎src/ng/directive/ngClass.js

+84-49
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,20 @@
99
function classDirective(name, selector) {
1010
name = 'ngClass' + name;
1111

12-
return [function() {
12+
return ['$parse', function($parse) {
1313
return {
1414
restrict: 'AC',
1515
link: function(scope, element, attr) {
16+
var expression = attr[name].trim();
17+
var isOneTime = (expression.charAt(0) === ':') && (expression.charAt(1) === ':');
18+
19+
var watchInterceptor = isOneTime ? toFlatValue : toClassString;
20+
var watchExpression = $parse(expression, watchInterceptor);
21+
var watchAction = isOneTime ? ngClassOneTimeWatchAction : ngClassWatchAction;
22+
1623
var classCounts = element.data('$classCounts');
1724
var oldModulo = true;
18-
var oldVal;
25+
var oldClassString;
1926

2027
if (!classCounts) {
2128
// Use createMap() to prevent class assumptions involving property
@@ -30,34 +37,47 @@ function classDirective(name, selector) {
3037
var newModulo = $index & 1;
3138

3239
if (newModulo !== oldModulo) {
33-
var classes = arrayClasses(oldVal);
3440
if (newModulo === selector) {
35-
addClasses(classes);
41+
addClasses(oldClassString);
3642
} else {
37-
removeClasses(classes);
43+
removeClasses(oldClassString);
3844
}
3945

4046
oldModulo = newModulo;
4147
}
4248
});
4349
}
4450

45-
scope.$watch(attr[name], ngClassWatchAction, true);
51+
scope.$watch(watchExpression, watchAction, isOneTime);
52+
53+
function addClasses(classString) {
54+
classString = digestClassCounts(split(classString), 1);
55+
attr.$addClass(classString);
56+
}
4657

47-
function addClasses(classes) {
48-
var newClasses = digestClassCounts(classes, 1);
49-
attr.$addClass(newClasses);
58+
function removeClasses(classString) {
59+
classString = digestClassCounts(split(classString), -1);
60+
attr.$removeClass(classString);
5061
}
5162

52-
function removeClasses(classes) {
53-
var newClasses = digestClassCounts(classes, -1);
54-
attr.$removeClass(newClasses);
63+
function updateClasses(oldClassString, newClassString) {
64+
var oldClassArray = split(oldClassString);
65+
var newClassArray = split(newClassString);
66+
67+
var toRemoveArray = arrayDifference(oldClassArray, newClassArray);
68+
var toAddArray = arrayDifference(newClassArray, oldClassArray);
69+
70+
var toRemoveString = digestClassCounts(toRemoveArray, -1);
71+
var toAddString = digestClassCounts(toAddArray, 1);
72+
73+
attr.$addClass(toAddString);
74+
attr.$removeClass(toRemoveString);
5575
}
5676

57-
function digestClassCounts(classes, count) {
77+
function digestClassCounts(classArray, count) {
5878
var classesToUpdate = [];
5979

60-
forEach(classes, function(className) {
80+
forEach(classArray, function(className) {
6181
if (count > 0 || classCounts[className]) {
6282
classCounts[className] = (classCounts[className] || 0) + count;
6383
if (classCounts[className] === +(count > 0)) {
@@ -69,31 +89,20 @@ function classDirective(name, selector) {
6989
return classesToUpdate.join(' ');
7090
}
7191

72-
function updateClasses(oldClasses, newClasses) {
73-
var toAdd = arrayDifference(newClasses, oldClasses);
74-
var toRemove = arrayDifference(oldClasses, newClasses);
75-
toAdd = digestClassCounts(toAdd, 1);
76-
toRemove = digestClassCounts(toRemove, -1);
92+
function ngClassOneTimeWatchAction(newClassValue) {
93+
var newClassString = toClassString(newClassValue);
7794

78-
attr.$addClass(toAdd);
79-
attr.$removeClass(toRemove);
95+
if (newClassString !== oldClassString) {
96+
ngClassWatchAction(newClassString);
97+
}
8098
}
8199

82-
function ngClassWatchAction(newVal) {
100+
function ngClassWatchAction(newClassString) {
83101
if (oldModulo === selector) {
84-
var newClasses = arrayClasses(newVal || []);
85-
if (!oldVal) {
86-
addClasses(newClasses);
87-
} else if (!equals(newVal,oldVal)) {
88-
var oldClasses = arrayClasses(oldVal);
89-
updateClasses(oldClasses, newClasses);
90-
}
91-
}
92-
if (isArray(newVal)) {
93-
oldVal = newVal.map(function(v) { return shallowCopy(v); });
94-
} else {
95-
oldVal = shallowCopy(newVal);
102+
updateClasses(oldClassString, newClassString);
96103
}
104+
105+
oldClassString = newClassString;
97106
}
98107
}
99108
};
@@ -118,24 +127,50 @@ function classDirective(name, selector) {
118127
return values;
119128
}
120129

121-
function arrayClasses(classVal) {
122-
var classes = [];
123-
if (isArray(classVal)) {
124-
forEach(classVal, function(v) {
125-
classes = classes.concat(arrayClasses(v));
126-
});
127-
return classes;
128-
} else if (isString(classVal)) {
129-
return classVal.split(' ');
130-
} else if (isObject(classVal)) {
131-
forEach(classVal, function(v, k) {
132-
if (v) {
133-
classes = classes.concat(k.split(' '));
130+
function split(classString) {
131+
return classString && classString.split(' ');
132+
}
133+
134+
function toClassString(classValue) {
135+
var classString = classValue;
136+
137+
if (isArray(classValue)) {
138+
classString = classValue.map(toClassString).join(' ');
139+
} else if (isObject(classValue)) {
140+
classString = Object.keys(classValue).
141+
filter(function(key) { return classValue[key]; }).
142+
join(' ');
143+
}
144+
145+
return classString;
146+
}
147+
148+
function toFlatValue(classValue) {
149+
var flatValue = classValue;
150+
151+
if (isArray(classValue)) {
152+
flatValue = classValue.map(toFlatValue);
153+
} else if (isObject(classValue)) {
154+
var hasUndefined = false;
155+
156+
flatValue = Object.keys(classValue).filter(function(key) {
157+
var value = classValue[key];
158+
159+
if (!hasUndefined && isUndefined(value)) {
160+
hasUndefined = true;
134161
}
162+
163+
return value;
135164
});
136-
return classes;
165+
166+
if (hasUndefined) {
167+
// Prevent the `oneTimeLiteralWatchInterceptor` from unregistering
168+
// the watcher, by including at least one `undefined` value.
169+
flatValue.push(undefined);
170+
}
137171
}
138-
return classVal;
172+
173+
return flatValue;
139174
}
140175
}
141176

‎test/ng/directive/ngClassSpec.js

+70
Original file line numberDiff line numberDiff line change
@@ -574,6 +574,76 @@ fdescribe('ngClass', function() {
574574
expect(element).not.toHaveClass('orange');
575575
})
576576
);
577+
578+
describe('large objects', function() {
579+
var getProp;
580+
var veryLargeObj;
581+
582+
beforeEach(function() {
583+
getProp = jasmine.createSpy('getProp');
584+
veryLargeObj = {};
585+
586+
Object.defineProperty(veryLargeObj, 'prop', {
587+
get: getProp,
588+
enumerable: true
589+
});
590+
});
591+
592+
it('should not be copied when using an expression', inject(function($compile, $rootScope) {
593+
element = $compile('<div ng-class="fooClass"></div>')($rootScope);
594+
$rootScope.fooClass = {foo: veryLargeObj};
595+
$rootScope.$digest();
596+
597+
expect(element).toHaveClass('foo');
598+
expect(getProp).not.toHaveBeenCalled();
599+
}));
600+
601+
it('should not be copied when using a literal', inject(function($compile, $rootScope) {
602+
element = $compile('<div ng-class="{foo: veryLargeObj}"></div>')($rootScope);
603+
$rootScope.veryLargeObj = veryLargeObj;
604+
$rootScope.$digest();
605+
606+
expect(element).toHaveClass('foo');
607+
expect(getProp).not.toHaveBeenCalled();
608+
}));
609+
610+
it('should not be copied when inside an array', inject(function($compile, $rootScope) {
611+
element = $compile('<div ng-class="[{foo: veryLargeObj}]"></div>')($rootScope);
612+
$rootScope.veryLargeObj = veryLargeObj;
613+
$rootScope.$digest();
614+
615+
expect(element).toHaveClass('foo');
616+
expect(getProp).not.toHaveBeenCalled();
617+
}));
618+
619+
it('should not be copied when using one-time binding', inject(function($compile, $rootScope) {
620+
element = $compile('<div ng-class="::{foo: veryLargeObj, bar: bar}"></div>')($rootScope);
621+
$rootScope.veryLargeObj = veryLargeObj;
622+
$rootScope.$digest();
623+
624+
expect(element).toHaveClass('foo');
625+
expect(element).not.toHaveClass('bar');
626+
expect(getProp).not.toHaveBeenCalled();
627+
628+
$rootScope.$apply('veryLargeObj.bar = "bar"');
629+
630+
expect(element).toHaveClass('foo');
631+
expect(element).not.toHaveClass('bar');
632+
expect(getProp).not.toHaveBeenCalled();
633+
634+
$rootScope.$apply('bar = "bar"');
635+
636+
expect(element).toHaveClass('foo');
637+
expect(element).toHaveClass('bar');
638+
expect(getProp).not.toHaveBeenCalled();
639+
640+
$rootScope.$apply('veryLargeObj.bar = "qux"');
641+
642+
expect(element).toHaveClass('foo');
643+
expect(element).toHaveClass('bar');
644+
expect(getProp).not.toHaveBeenCalled();
645+
}));
646+
});
577647
});
578648

579649
describe('ngClass animations', function() {

0 commit comments

Comments
 (0)