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

Commit d01e0c9

Browse files
drpicoxdrpicox
committed
perf(ngClass): optimize the case of static map of classes with large objects by refactor
1 parent 83bc247 commit d01e0c9

File tree

2 files changed

+205
-81
lines changed

2 files changed

+205
-81
lines changed

‎src/ng/directive/ngClass.js

+100-81
Original file line numberDiff line numberDiff line change
@@ -2,90 +2,91 @@
22

33
function classDirective(name, selector) {
44
name = 'ngClass' + name;
5-
return ['$animate', function($animate) {
5+
return ['$animate','$parse', function($animate,$parse) {
66
return {
77
restrict: 'AC',
8-
link: function(scope, element, attr) {
9-
var oldVal;
10-
11-
scope.$watch(attr[name], ngClassWatchAction, true);
12-
13-
attr.$observe('class', function(value) {
14-
ngClassWatchAction(scope.$eval(attr[name]));
8+
compile: function(tElement, tAttr) {
9+
var classFn = $parse(tAttr[name], function(value) {
10+
return arrayClassesL1(value);
1511
});
12+
return function(scope, element, attr) {
13+
var oldClasses;
1614

1715

18-
if (name !== 'ngClass') {
19-
scope.$watch('$index', function($index, old$index) {
20-
// jshint bitwise: false
21-
var mod = $index & 1;
22-
if (mod !== (old$index & 1)) {
23-
var classes = arrayClasses(scope.$eval(attr[name]));
24-
mod === selector ?
25-
addClasses(classes) :
26-
removeClasses(classes);
27-
}
28-
});
29-
}
16+
var unwatch = scope.$watch(classFn, ngClassWatchAction, classArrayEquals);
3017

31-
function addClasses(classes) {
32-
var newClasses = digestClassCounts(classes, 1);
33-
attr.$addClass(newClasses);
34-
}
18+
attr.$observe('class', function(value) {
19+
ngClassWatchAction(classFn(scope));
20+
});
3521

36-
function removeClasses(classes) {
37-
var newClasses = digestClassCounts(classes, -1);
38-
attr.$removeClass(newClasses);
39-
}
4022

41-
function digestClassCounts(classes, count) {
42-
// Use createMap() to prevent class assumptions involving property
43-
// names in Object.prototype
44-
var classCounts = element.data('$classCounts') || createMap();
45-
var classesToUpdate = [];
46-
forEach(classes, function(className) {
47-
if (count > 0 || classCounts[className]) {
48-
classCounts[className] = (classCounts[className] || 0) + count;
49-
if (classCounts[className] === +(count > 0)) {
50-
classesToUpdate.push(className);
23+
if (name !== 'ngClass') {
24+
scope.$watch('$index', function($index, old$index) {
25+
// jshint bitwise: false
26+
var mod = $index & 1;
27+
if (mod !== (old$index & 1)) {
28+
var classes = classFn(scope).join(' ').split(' ');
29+
mod === selector ?
30+
addClasses(classes) :
31+
removeClasses(classes);
5132
}
52-
}
53-
});
54-
element.data('$classCounts', classCounts);
55-
return classesToUpdate.join(' ');
56-
}
33+
});
34+
}
5735

58-
function updateClasses(oldClasses, newClasses) {
59-
var toAdd = arrayDifference(newClasses, oldClasses);
60-
var toRemove = arrayDifference(oldClasses, newClasses);
61-
toAdd = digestClassCounts(toAdd, 1);
62-
toRemove = digestClassCounts(toRemove, -1);
63-
if (toAdd && toAdd.length) {
64-
$animate.addClass(element, toAdd);
36+
function addClasses(classes) {
37+
var newClasses = digestClassCounts(classes, 1);
38+
attr.$addClass(newClasses);
6539
}
66-
if (toRemove && toRemove.length) {
67-
$animate.removeClass(element, toRemove);
40+
41+
function removeClasses(classes) {
42+
var newClasses = digestClassCounts(classes, -1);
43+
attr.$removeClass(newClasses);
44+
}
45+
46+
function digestClassCounts(classes, count) {
47+
// Use createMap() to prevent class assumptions involving property
48+
// names in Object.prototype
49+
var classCounts = element.data('$classCounts') || createMap();
50+
var classesToUpdate = [];
51+
forEach(classes, function(className) {
52+
if (count > 0 || classCounts[className]) {
53+
classCounts[className] = (classCounts[className] || 0) + count;
54+
if (classCounts[className] === +(count > 0)) {
55+
classesToUpdate.push(className);
56+
}
57+
}
58+
});
59+
element.data('$classCounts', classCounts);
60+
return classesToUpdate.join(' ');
6861
}
69-
}
7062

71-
function ngClassWatchAction(newVal) {
72-
// jshint bitwise: false
73-
if (selector === true || (scope.$index & 1) === selector) {
74-
// jshint bitwise: true
75-
var newClasses = arrayClasses(newVal || []);
76-
if (!oldVal) {
77-
addClasses(newClasses);
78-
} else if (!equals(newVal,oldVal)) {
79-
var oldClasses = arrayClasses(oldVal);
80-
updateClasses(oldClasses, newClasses);
63+
function updateClasses(oldClasses, newClasses) {
64+
var toAdd = arrayDifference(newClasses, oldClasses);
65+
var toRemove = arrayDifference(oldClasses, newClasses);
66+
toAdd = digestClassCounts(toAdd, 1);
67+
toRemove = digestClassCounts(toRemove, -1);
68+
if (toAdd && toAdd.length) {
69+
$animate.addClass(element, toAdd);
70+
}
71+
if (toRemove && toRemove.length) {
72+
$animate.removeClass(element, toRemove);
8173
}
8274
}
83-
if (isArray(newVal)) {
84-
oldVal = newVal.map(function(v) { return shallowCopy(v); });
85-
} else {
86-
oldVal = shallowCopy(newVal);
75+
76+
function ngClassWatchAction(newVal) {
77+
// jshint bitwise: false
78+
if (selector === true || (scope.$index & 1) === selector) {
79+
// jshint bitwise: true
80+
var newClasses = (newVal || []).join(' ').split(' ');
81+
if (!oldClasses) {
82+
addClasses(newClasses);
83+
} else if (!classArrayEquals(oldClasses, newClasses)) {
84+
updateClasses(oldClasses, newClasses);
85+
}
86+
oldClasses = shallowCopy(newClasses);
87+
}
8788
}
88-
}
89+
};
8990
}
9091
};
9192

@@ -103,25 +104,43 @@ function classDirective(name, selector) {
103104
return values;
104105
}
105106

106-
function arrayClasses(classVal) {
107-
var classes = [];
107+
function arrayClassesL1(classVal) {
108+
var classes;
108109
if (isArray(classVal)) {
109-
forEach(classVal, function(v) {
110-
classes = classes.concat(arrayClasses(v));
111-
});
112-
return classes;
113-
} else if (isString(classVal)) {
114-
return classVal.split(' ');
110+
classes = classVal.map(arrayClassesL2);
115111
} else if (isObject(classVal)) {
116-
forEach(classVal, function(v, k) {
117-
if (v) {
118-
classes = classes.concat(k.split(' '));
112+
classes = [];
113+
forEach(classVal, function(val, key) {
114+
if (val) {
115+
classes.push(key);
116+
} else if (isUndefined(val)) {
117+
classes.push(val);
119118
}
120119
});
121-
return classes;
120+
} else if (isString(classVal)) {
121+
classes = [classVal];
122+
} else {
123+
classes = classVal;
124+
}
125+
126+
return classes;
127+
}
128+
129+
function arrayClassesL2(values) {
130+
if (isObject(values)) {
131+
return Object.keys(values).filter(function(k) {
132+
return values[k];
133+
}).join(' ');
134+
} else {
135+
return values;
122136
}
123-
return classVal;
124137
}
138+
139+
function classArrayEquals(a1, a2) {
140+
return a1 === a2 ||
141+
isArray(a1) && isArray(a2) && a1.join(' ') === a2.join(' ');
142+
}
143+
125144
}];
126145
}
127146

‎test/ng/directive/ngClassSpec.js

+105
Original file line numberDiff line numberDiff line change
@@ -425,6 +425,111 @@ describe('ngClass', function() {
425425
})
426426
);
427427

428+
it('should do value stabilization as expected when one-time binding',
429+
inject(function($rootScope, $compile) {
430+
element = $compile('<div ng-class=" :: className"></div>')($rootScope);
431+
$rootScope.$digest();
432+
433+
$rootScope.className = 'foo';
434+
$rootScope.$digest();
435+
expect(element).toHaveClass('foo');
436+
437+
$rootScope.className = 'bar';
438+
$rootScope.$digest();
439+
expect(element).toHaveClass('foo');
440+
})
441+
);
442+
443+
it('should do value remove the watcher when static array one-time binding',
444+
inject(function($rootScope, $compile) {
445+
element = $compile('<div ng-class="::[className]"></div>')($rootScope);
446+
$rootScope.$digest();
447+
448+
$rootScope.className = 'foo';
449+
$rootScope.$digest();
450+
expect(element).toHaveClass('foo');
451+
452+
$rootScope.className = 'bar';
453+
$rootScope.$digest();
454+
expect(element).toHaveClass('foo');
455+
expect(element).not.toHaveClass('bar');
456+
})
457+
);
458+
459+
it('should do value remove the watcher when static map one-time binding',
460+
inject(function($rootScope, $compile) {
461+
element = $compile('<div ng-class="::{foo: fooPresent}"></div>')($rootScope);
462+
$rootScope.$digest();
463+
464+
$rootScope.fooPresent = true;
465+
$rootScope.$digest();
466+
expect(element).toHaveClass('foo');
467+
468+
$rootScope.fooPresent = false;
469+
$rootScope.$digest();
470+
expect(element).toHaveClass('foo');
471+
})
472+
);
473+
474+
it('should track changes of mutating object inside an array',
475+
inject(function($rootScope, $compile) {
476+
$rootScope.classVar = [{orange: true}];
477+
element = $compile('<div ng-class="classVar"></div>')($rootScope);
478+
$rootScope.$digest();
479+
480+
$rootScope.classVar[0].orange = false;
481+
$rootScope.$digest();
482+
483+
expect(element).not.toHaveClass('orange');
484+
})
485+
);
486+
487+
describe('large objects', function() {
488+
489+
var verylargeobject, getProp;
490+
beforeEach(function() {
491+
getProp = jasmine.createSpy('getProp');
492+
verylargeobject = {};
493+
Object.defineProperty(verylargeobject, 'prop', {
494+
get: getProp,
495+
enumerable: true
496+
});
497+
});
498+
499+
it('should not be copied if static', inject(function($rootScope, $compile) {
500+
element = $compile('<div ng-class="{foo: verylargeobject}"></div>')($rootScope);
501+
$rootScope.verylargeobject = verylargeobject;
502+
$rootScope.$digest();
503+
504+
expect(getProp).not.toHaveBeenCalled();
505+
}));
506+
507+
it('should not be copied if dynamic', inject(function($rootScope, $compile) {
508+
$rootScope.fooClass = {foo: verylargeobject};
509+
element = $compile('<div ng-class="fooClass"></div>')($rootScope);
510+
$rootScope.$digest();
511+
512+
expect(getProp).not.toHaveBeenCalled();
513+
}));
514+
515+
it('should not be copied if inside an array', inject(function($rootScope, $compile) {
516+
element = $compile('<div ng-class="[{foo: verylargeobject}]"></div>')($rootScope);
517+
$rootScope.verylargeobject = verylargeobject;
518+
$rootScope.$digest();
519+
520+
expect(getProp).not.toHaveBeenCalled();
521+
}));
522+
523+
it('should not be copied when one-time binding', inject(function($rootScope, $compile) {
524+
element = $compile('<div ng-class="::{foo: verylargeobject}"></div>')($rootScope);
525+
$rootScope.verylargeobject = verylargeobject;
526+
$rootScope.$digest();
527+
528+
expect(getProp).not.toHaveBeenCalled();
529+
}));
530+
531+
});
532+
428533
});
429534

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

0 commit comments

Comments
 (0)