From aafb9d9a3b39a12761aff003731d931f71acd79e Mon Sep 17 00:00:00 2001
From: David Rodenas Pico <david.rodenas@gmail.com>
Date: Fri, 8 Apr 2016 11:40:28 +0200
Subject: [PATCH 1/2] perf(ngClass): optimize the case of static map of classes
 with large objects by refactor

---
 src/ng/directive/ngClass.js | 78 +++++++++++++++++++++++--------------
 1 file changed, 48 insertions(+), 30 deletions(-)

diff --git a/src/ng/directive/ngClass.js b/src/ng/directive/ngClass.js
index 7e138b065dc2..1e6c282249a4 100644
--- a/src/ng/directive/ngClass.js
+++ b/src/ng/directive/ngClass.js
@@ -8,32 +8,33 @@
 
 function classDirective(name, selector) {
   name = 'ngClass' + name;
-  return ['$animate', function($animate) {
+  return ['$animate','$parse', function($animate,$parse) {
     return {
       restrict: 'AC',
       link: function(scope, element, attr) {
-        var oldVal;
+        var oldClasses;
 
-        scope.$watch(attr[name], ngClassWatchAction, true);
+        var classFn = $parse(attr[name], arrayClasses);
+        scope.$watch(classFn, ngClassWatchAction, true);
 
         attr.$observe('class', function(value) {
-          ngClassWatchAction(scope.$eval(attr[name]));
+          ngClassWatchAction(classFn(scope));
         });
 
 
         if (name !== 'ngClass') {
           scope.$watch('$index', function($index, old$index) {
-            /* eslint-disable no-bitwise */
+            // eslint-disable-next-line no-bitwise
             var mod = $index & 1;
+            // eslint-disable-next-line no-bitwise
             if (mod !== (old$index & 1)) {
-              var classes = arrayClasses(scope.$eval(attr[name]));
               if (mod === selector) {
-                addClasses(classes);
+                ngClassWatchAction(classFn(scope));
               } else {
-                removeClasses(classes);
+                removeClasses(oldClasses);
+                oldClasses = [];
               }
             }
-            /* eslint-enable */
           });
         }
 
@@ -80,18 +81,13 @@ function classDirective(name, selector) {
         function ngClassWatchAction(newVal) {
           // eslint-disable-next-line no-bitwise
           if (selector === true || (scope.$index & 1) === selector) {
-            var newClasses = arrayClasses(newVal || []);
-            if (!oldVal) {
+            var newClasses = (newVal || []).join(' ').split(' ');
+            if (!oldClasses) {
               addClasses(newClasses);
-            } else if (!equals(newVal,oldVal)) {
-              var oldClasses = arrayClasses(oldVal);
+            } else if (!arrayEqualClasses(oldClasses, newClasses)) {
               updateClasses(oldClasses, newClasses);
             }
-          }
-          if (isArray(newVal)) {
-            oldVal = newVal.map(function(v) { return shallowCopy(v); });
-          } else {
-            oldVal = shallowCopy(newVal);
+            oldClasses = shallowCopy(newClasses);
           }
         }
       }
@@ -112,24 +108,46 @@ function classDirective(name, selector) {
     }
 
     function arrayClasses(classVal) {
-      var classes = [];
+      var classes;
       if (isArray(classVal)) {
-        forEach(classVal, function(v) {
-          classes = classes.concat(arrayClasses(v));
-        });
-        return classes;
-      } else if (isString(classVal)) {
-        return classVal.split(' ');
+        classes = classVal.map(classNames);
       } else if (isObject(classVal)) {
-        forEach(classVal, function(v, k) {
-          if (v) {
-            classes = classes.concat(k.split(' '));
+        classes = [];
+        forEach(classVal, function(val, key) {
+          if (val) {
+            classes.push(key);
+          } else if (isUndefined(val)) {
+            // This case is for one time binding:
+            // ::expression are evaluated until no undefineds are found
+            // if no undefined is placed inside the array,
+            // it would assume that the value is fully computed and will fail
+            classes.push(val);
           }
         });
-        return classes;
+      } else if (isString(classVal)) {
+        classes = [classVal];
+      } else {
+        classes = classVal;
       }
-      return classVal;
+
+      return classes;
     }
+
+    function classNames(values) {
+      if (isObject(values)) {
+        return Object.keys(values).filter(function(k) {
+          return values[k];
+        }).join(' ');
+      } else {
+        return values;
+      }
+    }
+
+    function arrayEqualClasses(a1, a2) {
+      return a1 === a2 ||
+        isArray(a1) && isArray(a2) && a1.join(' ') === a2.join(' ');
+    }
+
   }];
 }
 

From 01e395a5a720958ec42c7ca3d904437c33e53103 Mon Sep 17 00:00:00 2001
From: David Rodenas Pico <david.rodenas@gmail.com>
Date: Mon, 10 Oct 2016 20:58:01 +0200
Subject: [PATCH 2/2] test(ngClass): test large objects, ::one time
 expressions, and $index bug

---
 test/ng/directive/ngClassSpec.js | 154 +++++++++++++++++++++++++++++++
 1 file changed, 154 insertions(+)

diff --git a/test/ng/directive/ngClassSpec.js b/test/ng/directive/ngClassSpec.js
index b30d1318317c..7c4acaec9d02 100644
--- a/test/ng/directive/ngClassSpec.js
+++ b/test/ng/directive/ngClassSpec.js
@@ -299,6 +299,54 @@ describe('ngClass', function() {
     expect(e2.hasClass('D')).toBeFalsy();
   }));
 
+  it('should compute class names when changing row parity and class value',
+    inject(function($rootScope, $compile) {
+      element = $compile('<ul>' +
+        '<li ng-repeat="i in arr track by i" ng-class-even="cls"></li>' +
+        '</ul>')($rootScope);
+
+      $rootScope.$apply(function() {
+        $rootScope.arr = ['a','b'];
+        $rootScope.cls = 'even';
+      });
+      var e3 = jqLite(element[0].childNodes[3]);
+      expect(e3).toHaveClass('even');
+
+      $rootScope.$apply(function() {
+        $rootScope.arr = ['b'];
+        $rootScope.cls = 'off';
+      });
+      var e1 = jqLite(element[0].childNodes[1]);
+      expect(e3[0]).toBe(e1[0]); // make sure same instance of ngClassEven
+      expect(e3).not.toHaveClass('even');
+      expect(e3).not.toHaveClass('off');
+
+      $rootScope.$apply(function() {
+        $rootScope.arr = ['a','b'];
+        $rootScope.cls = 'on';
+      });
+      expect(e3).not.toHaveClass('even');
+      expect(e3).not.toHaveClass('off');
+      expect(e3).toHaveClass('on');
+
+      // activate both $watch functions (class and $index)
+      // see https://plnkr.co/edit/W1ck8dS08TCJpirWiGVA
+      $rootScope.$apply(function() {
+        $rootScope.arr = ['b'];
+        $rootScope.cls = 'off';
+      });
+      expect(e3).not.toHaveClass('even');
+      expect(e3).not.toHaveClass('off');
+      expect(e3).not.toHaveClass('on');
+
+      $rootScope.$apply(function() {
+        $rootScope.arr = ['a','b'];
+      });
+      expect(e3).not.toHaveClass('even');
+      expect(e3).toHaveClass('off');
+      expect(e3).not.toHaveClass('on');
+    })
+  );
 
   it('should reapply ngClass when interpolated class attribute changes', inject(function($rootScope, $compile) {
     element = $compile('<div class="one {{cls}} three" ng-class="{four: four}"></div>')($rootScope);
@@ -424,6 +472,112 @@ describe('ngClass', function() {
     })
   );
 
+  it('should do value stabilization as expected when one-time binding',
+    inject(function($rootScope, $compile) {
+      element = $compile('<div ng-class=" :: className"></div>')($rootScope);
+      $rootScope.$digest();
+
+      $rootScope.className = 'foo';
+      $rootScope.$digest();
+      expect(element).toHaveClass('foo');
+
+      $rootScope.className = 'bar';
+      $rootScope.$digest();
+      expect(element).toHaveClass('foo');
+    })
+  );
+
+  it('should remove the watcher when static array one-time binding',
+    inject(function($rootScope, $compile) {
+      element = $compile('<div ng-class="::[className]"></div>')($rootScope);
+      $rootScope.$digest();
+
+      $rootScope.className = 'foo';
+      $rootScope.$digest();
+      expect(element).toHaveClass('foo');
+
+      $rootScope.className = 'bar';
+      $rootScope.$digest();
+      expect(element).toHaveClass('foo');
+      expect(element).not.toHaveClass('bar');
+    })
+  );
+
+  it('should remove the watcher when static map one-time binding',
+    inject(function($rootScope, $compile) {
+      element = $compile('<div ng-class="::{foo: fooPresent}"></div>')($rootScope);
+      $rootScope.$digest();
+
+      $rootScope.fooPresent = true;
+      $rootScope.$digest();
+      expect(element).toHaveClass('foo');
+
+      $rootScope.fooPresent = false;
+      $rootScope.$digest();
+      expect(element).toHaveClass('foo');
+    })
+  );
+
+  it('should track changes of mutating object inside an array',
+    inject(function($rootScope, $compile) {
+      $rootScope.classVar = [{orange: true}];
+      element = $compile('<div ng-class="classVar"></div>')($rootScope);
+      $rootScope.$digest();
+      expect(element).toHaveClass('orange');
+
+      $rootScope.classVar[0].orange = false;
+      $rootScope.$digest();
+
+      expect(element).not.toHaveClass('orange');
+    })
+  );
+
+  describe('large objects', function() {
+
+    var verylargeobject, getProp;
+    beforeEach(function() {
+      getProp = jasmine.createSpy('getProp');
+      verylargeobject = {};
+      Object.defineProperty(verylargeobject, 'prop', {
+        get: getProp,
+        enumerable: true
+      });
+    });
+
+    it('should not be copied if static', inject(function($rootScope, $compile) {
+      element = $compile('<div ng-class="{foo: verylargeobject}"></div>')($rootScope);
+      $rootScope.verylargeobject = verylargeobject;
+      $rootScope.$digest();
+
+      expect(getProp).not.toHaveBeenCalled();
+    }));
+
+    it('should not be copied if dynamic', inject(function($rootScope, $compile) {
+      $rootScope.fooClass = {foo: verylargeobject};
+      element = $compile('<div ng-class="fooClass"></div>')($rootScope);
+      $rootScope.$digest();
+
+      expect(getProp).not.toHaveBeenCalled();
+    }));
+
+    it('should not be copied if inside an array', inject(function($rootScope, $compile) {
+      element = $compile('<div ng-class="[{foo: verylargeobject}]"></div>')($rootScope);
+      $rootScope.verylargeobject = verylargeobject;
+      $rootScope.$digest();
+
+      expect(getProp).not.toHaveBeenCalled();
+    }));
+
+    it('should not be copied when one-time binding', inject(function($rootScope, $compile) {
+      element = $compile('<div ng-class="::{foo: verylargeobject}"></div>')($rootScope);
+      $rootScope.verylargeobject = verylargeobject;
+      $rootScope.$digest();
+
+      expect(getProp).not.toHaveBeenCalled();
+    }));
+
+  });
+
 });
 
 describe('ngClass animations', function() {