Skip to content

Commit 120f60b

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 values (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 7319bd3 commit 120f60b

File tree

2 files changed

+135
-70
lines changed

2 files changed

+135
-70
lines changed

‎src/ng/directive/ngClass.js

+95-55
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,16 @@ function classDirective(name, selector) {
1414
return {
1515
restrict: 'AC',
1616
link: function(scope, element, attr) {
17+
var expression = attr[name].trim();
18+
var isOneTime = (expression.charAt(0) === ':') && (expression.charAt(1) === ':');
19+
20+
var watchInterceptor = isOneTime ? toFlatValue : toClassString;
21+
var watchExpression = $parse(expression, watchInterceptor);
22+
var watchAction = isOneTime ? ngClassOneTimeWatchAction : ngClassWatchAction;
23+
1724
var classCounts = element.data('$classCounts');
1825
var oldModulo = true;
19-
var oldVal;
26+
var oldClassString;
2027

2128
if (!classCounts) {
2229
// Use createMap() to prevent class assumptions involving property
@@ -33,34 +40,39 @@ function classDirective(name, selector) {
3340
});
3441
}
3542

36-
scope.$watch(indexWatchExpression, function(newModulo) {
37-
var classes = arrayClasses(oldVal);
38-
if (newModulo === selector) {
39-
addClasses(classes);
40-
} else {
41-
removeClasses(classes);
42-
}
43-
44-
oldModulo = newModulo;
45-
});
43+
scope.$watch(indexWatchExpression, ngClassIndexWatchAction);
4644
}
4745

48-
scope.$watch(attr[name], ngClassWatchAction, true);
46+
scope.$watch(watchExpression, watchAction, isOneTime);
4947

50-
function addClasses(classes) {
51-
var newClasses = digestClassCounts(classes, 1);
52-
attr.$addClass(newClasses);
48+
function addClasses(classString) {
49+
classString = digestClassCounts(split(classString), 1);
50+
attr.$addClass(classString);
5351
}
5452

55-
function removeClasses(classes) {
56-
var newClasses = digestClassCounts(classes, -1);
57-
attr.$removeClass(newClasses);
53+
function removeClasses(classString) {
54+
classString = digestClassCounts(split(classString), -1);
55+
attr.$removeClass(classString);
56+
}
57+
58+
function updateClasses(oldClassString, newClassString) {
59+
var oldClassArray = split(oldClassString);
60+
var newClassArray = split(newClassString);
61+
62+
var toRemoveArray = arrayDifference(oldClassArray, newClassArray);
63+
var toAddArray = arrayDifference(newClassArray, oldClassArray);
64+
65+
var toRemoveString = digestClassCounts(toRemoveArray, -1);
66+
var toAddString = digestClassCounts(toAddArray, 1);
67+
68+
attr.$addClass(toAddString);
69+
attr.$removeClass(toRemoveString);
5870
}
5971

60-
function digestClassCounts(classes, count) {
72+
function digestClassCounts(classArray, count) {
6173
var classesToUpdate = [];
6274

63-
forEach(classes, function(className) {
75+
forEach(classArray, function(className) {
6476
if (count > 0 || classCounts[className]) {
6577
classCounts[className] = (classCounts[className] || 0) + count;
6678
if (classCounts[className] === +(count > 0)) {
@@ -72,31 +84,33 @@ function classDirective(name, selector) {
7284
return classesToUpdate.join(' ');
7385
}
7486

75-
function updateClasses(oldClasses, newClasses) {
76-
var toAdd = arrayDifference(newClasses, oldClasses);
77-
var toRemove = arrayDifference(oldClasses, newClasses);
78-
toAdd = digestClassCounts(toAdd, 1);
79-
toRemove = digestClassCounts(toRemove, -1);
87+
function ngClassIndexWatchAction(newModulo) {
88+
// This watch-action should run before the `ngClass[OneTime]WatchAction()`, thus it
89+
// adds/removes `oldClassString`. If the `ngClass` expression has changed as well, the
90+
// `ngClass[OneTime]WatchAction()` will update the classes.
91+
if (newModulo === selector) {
92+
addClasses(oldClassString);
93+
} else {
94+
removeClasses(oldClassString);
95+
}
8096

81-
attr.$addClass(toAdd);
82-
attr.$removeClass(toRemove);
97+
oldModulo = newModulo;
8398
}
8499

85-
function ngClassWatchAction(newVal) {
86-
if (oldModulo === selector) {
87-
var newClasses = arrayClasses(newVal || []);
88-
if (!oldVal) {
89-
addClasses(newClasses);
90-
} else if (!equals(newVal,oldVal)) {
91-
var oldClasses = arrayClasses(oldVal);
92-
updateClasses(oldClasses, newClasses);
93-
}
100+
function ngClassOneTimeWatchAction(newClassValue) {
101+
var newClassString = toClassString(newClassValue);
102+
103+
if (newClassString !== oldClassString) {
104+
ngClassWatchAction(newClassString);
94105
}
95-
if (isArray(newVal)) {
96-
oldVal = newVal.map(function(v) { return shallowCopy(v); });
97-
} else {
98-
oldVal = shallowCopy(newVal);
106+
}
107+
108+
function ngClassWatchAction(newClassString) {
109+
if (oldModulo === selector) {
110+
updateClasses(oldClassString, newClassString);
99111
}
112+
113+
oldClassString = newClassString;
100114
}
101115
}
102116
};
@@ -121,24 +135,50 @@ function classDirective(name, selector) {
121135
return values;
122136
}
123137

124-
function arrayClasses(classVal) {
125-
var classes = [];
126-
if (isArray(classVal)) {
127-
forEach(classVal, function(v) {
128-
classes = classes.concat(arrayClasses(v));
129-
});
130-
return classes;
131-
} else if (isString(classVal)) {
132-
return classVal.split(' ');
133-
} else if (isObject(classVal)) {
134-
forEach(classVal, function(v, k) {
135-
if (v) {
136-
classes = classes.concat(k.split(' '));
138+
function split(classString) {
139+
return classString && classString.split(' ');
140+
}
141+
142+
function toClassString(classValue) {
143+
var classString = classValue;
144+
145+
if (isArray(classValue)) {
146+
classString = classValue.map(toClassString).join(' ');
147+
} else if (isObject(classValue)) {
148+
classString = Object.keys(classValue).
149+
filter(function(key) { return classValue[key]; }).
150+
join(' ');
151+
}
152+
153+
return classString;
154+
}
155+
156+
function toFlatValue(classValue) {
157+
var flatValue = classValue;
158+
159+
if (isArray(classValue)) {
160+
flatValue = classValue.map(toFlatValue);
161+
} else if (isObject(classValue)) {
162+
var hasUndefined = false;
163+
164+
flatValue = Object.keys(classValue).filter(function(key) {
165+
var value = classValue[key];
166+
167+
if (!hasUndefined && isUndefined(value)) {
168+
hasUndefined = true;
137169
}
170+
171+
return value;
138172
});
139-
return classes;
173+
174+
if (hasUndefined) {
175+
// Prevent the `oneTimeLiteralWatchInterceptor` from unregistering
176+
// the watcher, by including at least one `undefined` value.
177+
flatValue.push(undefined);
178+
}
140179
}
141-
return classVal;
180+
181+
return flatValue;
142182
}
143183
}
144184

‎test/ng/directive/ngClassSpec.js

+40-15
Original file line numberDiff line numberDiff line change
@@ -568,46 +568,71 @@ describe('ngClass', function() {
568568
);
569569

570570
describe('large objects', function() {
571+
var getProp;
572+
var veryLargeObj;
571573

572-
var verylargeobject, getProp;
573574
beforeEach(function() {
574575
getProp = jasmine.createSpy('getProp');
575-
verylargeobject = {};
576-
Object.defineProperty(verylargeobject, 'prop', {
576+
veryLargeObj = {};
577+
578+
Object.defineProperty(veryLargeObj, 'prop', {
577579
get: getProp,
578580
enumerable: true
579581
});
580582
});
581583

582-
it('should not be copied if static', inject(function($rootScope, $compile) {
583-
element = $compile('<div ng-class="{foo: verylargeobject}"></div>')($rootScope);
584-
$rootScope.verylargeobject = verylargeobject;
584+
it('should not be copied when using an expression', inject(function($compile, $rootScope) {
585+
element = $compile('<div ng-class="fooClass"></div>')($rootScope);
586+
$rootScope.fooClass = {foo: veryLargeObj};
585587
$rootScope.$digest();
586588

589+
expect(element).toHaveClass('foo');
587590
expect(getProp).not.toHaveBeenCalled();
588591
}));
589592

590-
it('should not be copied if dynamic', inject(function($rootScope, $compile) {
591-
$rootScope.fooClass = {foo: verylargeobject};
592-
element = $compile('<div ng-class="fooClass"></div>')($rootScope);
593+
it('should not be copied when using a literal', inject(function($compile, $rootScope) {
594+
element = $compile('<div ng-class="{foo: veryLargeObj}"></div>')($rootScope);
595+
$rootScope.veryLargeObj = veryLargeObj;
593596
$rootScope.$digest();
594597

598+
expect(element).toHaveClass('foo');
595599
expect(getProp).not.toHaveBeenCalled();
596600
}));
597601

598-
it('should not be copied if inside an array', inject(function($rootScope, $compile) {
599-
element = $compile('<div ng-class="[{foo: verylargeobject}]"></div>')($rootScope);
600-
$rootScope.verylargeobject = verylargeobject;
602+
it('should not be copied when inside an array', inject(function($compile, $rootScope) {
603+
element = $compile('<div ng-class="[{foo: veryLargeObj}]"></div>')($rootScope);
604+
$rootScope.veryLargeObj = veryLargeObj;
601605
$rootScope.$digest();
602606

607+
expect(element).toHaveClass('foo');
603608
expect(getProp).not.toHaveBeenCalled();
604609
}));
605610

606-
it('should not be copied when one-time binding', inject(function($rootScope, $compile) {
607-
element = $compile('<div ng-class="::{foo: verylargeobject}"></div>')($rootScope);
608-
$rootScope.verylargeobject = verylargeobject;
611+
it('should not be copied when using one-time binding', inject(function($compile, $rootScope) {
612+
element = $compile('<div ng-class="::{foo: veryLargeObj, bar: bar}"></div>')($rootScope);
613+
$rootScope.veryLargeObj = veryLargeObj;
609614
$rootScope.$digest();
610615

616+
expect(element).toHaveClass('foo');
617+
expect(element).not.toHaveClass('bar');
618+
expect(getProp).not.toHaveBeenCalled();
619+
620+
$rootScope.$apply('veryLargeObj.bar = "bar"');
621+
622+
expect(element).toHaveClass('foo');
623+
expect(element).not.toHaveClass('bar');
624+
expect(getProp).not.toHaveBeenCalled();
625+
626+
$rootScope.$apply('bar = "bar"');
627+
628+
expect(element).toHaveClass('foo');
629+
expect(element).toHaveClass('bar');
630+
expect(getProp).not.toHaveBeenCalled();
631+
632+
$rootScope.$apply('veryLargeObj.bar = "qux"');
633+
634+
expect(element).toHaveClass('foo');
635+
expect(element).toHaveClass('bar');
611636
expect(getProp).not.toHaveBeenCalled();
612637
}));
613638
});

0 commit comments

Comments
 (0)