perf(jqLite): avoid repeated add/removeAttribute in jqLiteRemoveClass #16131
Conversation
src/jqLite.js
Outdated
| }); | ||
|
|
||
| element.setAttribute('class', trim(existingClasses)); |
There was a problem hiding this comment.
Should you do a check to ensure that we don't set this unnecessarily which potentially causes unnecessary renders?
Ref: https://github.com/jquery/jquery/blob/master/src/attributes/classes.js#L41-L45
There was a problem hiding this comment.
I thought that should be done along with addClass in its own commit...
|
Passed now. However I decided to add one more commit like @dcherman suggested to only actually set the |
|
@jbedard can you please rebase this? The tests currently don't run because of a Travis problem at the branch point. |
|
Rebased. The tests had to be put in |
test/jqLiteSpec.js
Outdated
| @@ -914,6 +914,37 @@ describe('jqLite', function() { | |||
| }); | |||
|
|
|||
|
|
|||
| //JQLite specific implementation/performance tests | |||
test/jqLiteSpec.js
Outdated
| @@ -1031,6 +1062,37 @@ describe('jqLite', function() { | |||
| jqA.removeClass('foo baz noexistent'); | |||
| expect(a.className).toBe('bar'); | |||
| }); | |||
|
|
|||
|
|
|||
| //JQLite specific implementation/performance tests | |||
test/jqLiteSpec.js
Outdated
| @@ -914,6 +914,37 @@ describe('jqLite', function() { | |||
| }); | |||
|
|
|||
|
|
|||
| //JQLite specific implementation/performance tests | |||
This does something similar to what was suggested in #16078, although keeps it a little more like
jqLiteAddClassfor now.