Skip to content

Commit 007a7ef

Browse files
committed
perf(jqLite): avoid repeated add/removeAttribute in jqLiteRemoveClass
Fixes angular#16078 Closes angular#16131
1 parent 21a2f4b commit 007a7ef

File tree

2 files changed

+35
-5
lines changed

2 files changed

+35
-5
lines changed

src/jqLite.js

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -420,13 +420,15 @@ function jqLiteHasClass(element, selector) {
420420

421421
function jqLiteRemoveClass(element, cssClasses) {
422422
if (cssClasses && element.setAttribute) {
423+
var existingClasses = (' ' + (element.getAttribute('class') || '') + ' ')
424+
.replace(/[\n\t]/g, ' ');
425+
423426
forEach(cssClasses.split(' '), function(cssClass) {
424-
element.setAttribute('class', trim(
425-
(' ' + (element.getAttribute('class') || '') + ' ')
426-
.replace(/[\n\t]/g, ' ')
427-
.replace(' ' + trim(cssClass) + ' ', ' '))
428-
);
427+
cssClass = trim(cssClass);
428+
existingClasses = existingClasses.replace(' ' + cssClass + ' ', ' ');
429429
});
430+
431+
element.setAttribute('class', trim(existingClasses));
430432
}
431433
}
432434

test/jqLiteSpec.js

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -914,6 +914,20 @@ describe('jqLite', function() {
914914
});
915915

916916

917+
it('should only get/set the attribute once when multiple classes added', function() {
918+
var fakeElement = {
919+
nodeType: 1,
920+
setAttribute: jasmine.createSpy(),
921+
getAttribute: jasmine.createSpy().and.returnValue('')
922+
};
923+
var jqA = jqLite(fakeElement);
924+
925+
jqA.addClass('foo bar baz');
926+
expect(fakeElement.getAttribute).toHaveBeenCalledOnceWith('class');
927+
expect(fakeElement.setAttribute).toHaveBeenCalledOnceWith('class', 'foo bar baz');
928+
});
929+
930+
917931
it('should not add duplicate classes', function() {
918932
var jqA = jqLite(a);
919933
expect(a.className).toBe('');
@@ -1031,6 +1045,20 @@ describe('jqLite', function() {
10311045
jqA.removeClass('foo baz noexistent');
10321046
expect(a.className).toBe('bar');
10331047
});
1048+
1049+
1050+
it('should get/set the attribute once when removing multiple classes', function() {
1051+
var fakeElement = {
1052+
nodeType: 1,
1053+
setAttribute: jasmine.createSpy(),
1054+
getAttribute: jasmine.createSpy().and.returnValue('foo bar baz')
1055+
};
1056+
var jqA = jqLite(fakeElement);
1057+
1058+
jqA.removeClass('foo baz noexistent');
1059+
expect(fakeElement.getAttribute).toHaveBeenCalledOnceWith('class');
1060+
expect(fakeElement.setAttribute).toHaveBeenCalledOnceWith('class', 'bar');
1061+
});
10341062
});
10351063
});
10361064

0 commit comments

Comments
 (0)