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

Improve jqLiteRemoveClass performance #16078

Closed
aresn opened this issue Jun 30, 2017 · 1 comment
Closed

Improve jqLiteRemoveClass performance #16078

aresn opened this issue Jun 30, 2017 · 1 comment

Comments

@aresn
Copy link

aresn commented Jun 30, 2017

I would like to propose a slight modification to jqLiteRemoveClass function to increase it's performance.

Current implementation constantly reads the "class" value from DOM and writes it back to the DOM inside a for loop.

Here is what I have created for our local branch of angular:

function jqLiteRemoveClass(element, cssClasses) {
  if (cssClasses && element.setAttribute) {
    var currentClass = element.getAttribute('class');

    if (currentClass) {
      var cssClass;
      var curValue;
      curValue = currentClass = trim(currentClass);
      var classNames = cssClasses.split(' ');

      for (var i = 0, len = classNames.length; i < len; i++) {
        cssClass = trim(classNames[i]);
        currentClass = trim((' ' + currentClass + ' ')
          .replace(/[\n\t]/g, ' ')
          .replace(' ' + cssClass + ' ', ' '));
      }
      if (currentClass !== curValue) {
        element.setAttribute('class',currentClass);
      }
    }
  }
}

Here is a link to a jsPerf that I have created that compares the two functions :
https://jsperf.com/compare-angular-remove-class-with-improved-version/1

@aresn aresn changed the title Improve jgLiteRemoveClass performance Improve jqLiteRemoveClass performance Jun 30, 2017
@Narretz
Copy link
Contributor

Narretz commented Jul 2, 2017

Hi, perf improvements are definitely welcome, although I wonder if you specifically experienced perf problems related to removeClass in your apps?
And it does make sense to open this as a PR, so it's easier to see the change in the code.
I also wonder how jquery does it and if we should follow their implementation?

@Narretz Narretz added this to the Purgatory milestone Jul 2, 2017
jbedard added a commit to jbedard/angular.js that referenced this issue Jul 26, 2017
jbedard added a commit to jbedard/angular.js that referenced this issue Jul 27, 2017
jbedard added a commit to jbedard/angular.js that referenced this issue Jul 27, 2017
jbedard added a commit to jbedard/angular.js that referenced this issue Aug 3, 2017
jbedard added a commit to jbedard/angular.js that referenced this issue Aug 9, 2017
jbedard added a commit to jbedard/angular.js that referenced this issue Sep 9, 2017
jbedard added a commit to jbedard/angular.js that referenced this issue Sep 10, 2017
jbedard added a commit to jbedard/angular.js that referenced this issue Sep 12, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants