Fix ngClass animation bugs with enter, move and leave

src/ng/rootScope.js

@@ -118,7 +118,8 @@ function $RootScopeProvider(){
                      this.$$childHead = this.$$childTail = null;
       this['this'] = this.$root =  this;
       this.$$destroyed = false;
-      this.$$asyncQueue = [];
+      this.$$internalAsyncQueue = [];
+      this.$$externalAsyncQueue = [];
       this.$$listeners = {};
       this.$$isolateBindings = {};
     }
@@ -166,7 +167,8 @@ function $RootScopeProvider(){
           child = new Scope();
           child.$root = this.$root;
           // ensure that there is just one async queue per $rootScope and it's children
-          child.$$asyncQueue = this.$$asyncQueue;
+          child.$$internalAsyncQueue = this.$$internalAsyncQueue;
+          child.$$externalAsyncQueue = this.$$externalAsyncQueue;
         } else {
           Child = function() {}; // should be anonymous; This is so that when the minifier munges
             // the name it does not become random set of chars. These will then show up as class
@@ -493,7 +495,8 @@ function $RootScopeProvider(){
       $digest: function() {
         var watch, value, last,
             watchers,
-            asyncQueue = this.$$asyncQueue,
+            internalAsyncQueue = this.$$internalAsyncQueue,
+            externalAsyncQueue = this.$$externalAsyncQueue,
             length,
             dirty, ttl = TTL,
             next, current, target = this,
@@ -506,9 +509,9 @@ function $RootScopeProvider(){
           dirty = false;
           current = target;
 
-          while(asyncQueue.length) {
+          while(internalAsyncQueue.length) {
             try {
-              current.$eval(asyncQueue.shift());
+              current.$eval(internalAsyncQueue.shift());
             } catch (e) {
               $exceptionHandler(e);
             }
@@ -563,9 +566,17 @@ function $RootScopeProvider(){
                 '{0} $digest() iterations reached. Aborting!\nWatchers fired in the last 5 iterations: {1}',
                 TTL, toJson(watchLog));
           }
-        } while (dirty || asyncQueue.length);
+        } while (dirty || internalAsyncQueue.length);
 
         clearPhase();
+
+        while(externalAsyncQueue.length) {
+          try {
+            target.$eval(externalAsyncQueue.shift());
+          } catch (e) {
+            $exceptionHandler(e);
+          }
+        }
       },
 
 
@@ -678,9 +689,10 @@ function $RootScopeProvider(){
        *    - `string`: execute using the rules as defined in  {@link guide/expression expression}.
        *    - `function(scope)`: execute the function with the current `scope` parameter.
        *
+       * @param {boolean} runDigest Whether or not to skip the next digest cycle after the expr is evaluated
        */
-      $evalAsync: function(expr) {
-        this.$$asyncQueue.push(expr);
+      $evalAsync: function(expr, runDigest) {
+        (runDigest === false ? this.$$externalAsyncQueue : this.$$internalAsyncQueue).push(expr);
       },
 
       /**

src/ngAnimate/animate.js

@@ -204,8 +204,8 @@ angular.module('ngAnimate', ['ng'])
 
     var NG_ANIMATE_STATE = '$$ngAnimateState';
     var rootAnimateState = {running:true};
-    $provide.decorator('$animate', ['$delegate', '$injector', '$sniffer', '$rootElement', '$timeout',
-                            function($delegate,   $injector,   $sniffer,   $rootElement,   $timeout) {
+    $provide.decorator('$animate', ['$delegate', '$injector', '$sniffer', '$rootElement', '$timeout', '$rootScope',
+                            function($delegate,   $injector,   $sniffer,   $rootElement,   $timeout,   $rootScope) {
 
       $rootElement.data(NG_ANIMATE_STATE, rootAnimateState);
 
@@ -283,7 +283,9 @@ angular.module('ngAnimate', ['ng'])
         */
         enter : function(element, parent, after, done) {
           $delegate.enter(element, parent, after);
-          performAnimation('enter', 'ng-enter', element, parent, after, done);
+          $rootScope.$evalAsync(function() {
+            performAnimation('enter', 'ng-enter', element, parent, after, done);
+          }, false);
         },
 
         /**
@@ -314,9 +316,11 @@ angular.module('ngAnimate', ['ng'])
          * @param {function()=} done callback function that will be called once the animation is complete
         */
         leave : function(element, done) {
-          performAnimation('leave', 'ng-leave', element, null, null, function() {
-            $delegate.leave(element, done);
-          });
+          $rootScope.$evalAsync(function() {
+            performAnimation('leave', 'ng-leave', element, null, null, function() {
+              $delegate.leave(element, done);
+            });
+          }, false);
         },
 
         /**
@@ -351,7 +355,9 @@ angular.module('ngAnimate', ['ng'])
         */
         move : function(element, parent, after, done) {
           $delegate.move(element, parent, after);
-          performAnimation('move', 'ng-move', element, null, null, done);
+          $rootScope.$evalAsync(function() {
+            performAnimation('move', 'ng-move', element, null, null, done);
+          }, false);
         },
 
         /**

test/ng/directive/ngClassSpec.js

@@ -308,42 +308,112 @@ describe('ngClass', function() {
 describe('ngClass animations', function() {
   var body, element, $rootElement;
 
-  beforeEach(module('mock.animate'));
-
-  it("should avoid calling addClass accidentally when removeClass is going on",
+  it("should avoid calling addClass accidentally when removeClass is going on", function() {
+    module('mock.animate');
     inject(function($compile, $rootScope, $animate, $timeout) {
+      var element = angular.element('<div ng-class="val"></div>');
+      var body = jqLite(document.body);
+      body.append(element);
+      $compile(element)($rootScope);
 
-    var element = angular.element('<div ng-class="val"></div>');
-    var body = jqLite(document.body);
-    body.append(element);
-    $compile(element)($rootScope);
+      expect($animate.queue.length).toBe(0);
 
-    expect($animate.queue.length).toBe(0);
+      $rootScope.val = 'one';
+      $rootScope.$digest();
+      $animate.flushNext('addClass');
+      $animate.flushNext('addClass');
+      $timeout.flush();
+      expect($animate.queue.length).toBe(0);
 
-    $rootScope.val = 'one';
-    $rootScope.$digest();
-    $animate.flushNext('addClass');
-    $animate.flushNext('addClass');
-    $timeout.flush();
-    expect($animate.queue.length).toBe(0);
+      $rootScope.val = '';
+      $rootScope.$digest();
+      $animate.flushNext('removeClass'); //only removeClass is called
+      expect($animate.queue.length).toBe(0);
+      $timeout.flush();
 
-    $rootScope.val = '';
-    $rootScope.$digest();
-    $animate.flushNext('removeClass'); //only removeClass is called
-    expect($animate.queue.length).toBe(0);
-    $timeout.flush();
+      $rootScope.val = 'one';
+      $rootScope.$digest();
+      $animate.flushNext('addClass');
+      $timeout.flush();
+      expect($animate.queue.length).toBe(0);
 
-    $rootScope.val = 'one';
-    $rootScope.$digest();
-    $animate.flushNext('addClass');
-    $timeout.flush();
-    expect($animate.queue.length).toBe(0);
+      $rootScope.val = 'two';
+      $rootScope.$digest();
+      $animate.flushNext('removeClass');
+      $animate.flushNext('addClass');
+      $timeout.flush();
+      expect($animate.queue.length).toBe(0);
+    });
+  });
 
-    $rootScope.val = 'two';
-    $rootScope.$digest();
-    $animate.flushNext('removeClass');
-    $animate.flushNext('addClass');
-    $timeout.flush();
-    expect($animate.queue.length).toBe(0);
-  }));
+  it("should consider the ngClass expression evaluation before performing an animation", function() {
+
+    //mocks are not used since the enter delegation method is called before addClass and
+    //it makes it impossible to test to see that addClass is called first
+    module('ngAnimate');
+
+    var digestQueue = [];
+    module(function($animateProvider) {
+      $animateProvider.register('.crazy', function() {
+        return {
+          enter : function(element, done) {
+            element.data('state', 'crazy-enter');
+            done();
+          }
+        };
+      });
+
+      return function($rootScope) {
+        var before = $rootScope.$evalAsync;
+        $rootScope.$evalAsync = function() {
+          var args = arguments;
+          digestQueue.push(function() {
+            before.apply($rootScope, args);
+          });
+        };
+      };
+    });
+    inject(function($compile, $rootScope, $rootElement, $animate, $timeout, $document) {
+
+      //since we skip animations upon first digest, this needs to be set to true
+      $animate.enabled(true);
+
+      $rootScope.val = 'crazy';
+      var element = angular.element('<div ng-class="val"></div>');
+      jqLite($document[0].body).append($rootElement);
+
+      $compile(element)($rootScope);
+
+      var enterComplete = false;
+      $animate.enter(element, $rootElement, null, function() {
+        enterComplete = true;
+      });
+
+      //jquery doesn't compare both elements properly so let's use the nodes
+      expect(element.parent()[0]).toEqual($rootElement[0]);
+      expect(element.hasClass('crazy')).toBe(false);
+      expect(enterComplete).toBe(false);
+
+      expect(digestQueue.length).toBe(2);
+      $rootScope.$digest();
+
+      digestQueue.shift()(); //addClass
+      $timeout.flush();
+
+      expect(element.hasClass('crazy')).toBe(true);
+      expect(enterComplete).toBe(false);
+
+      digestQueue.shift()(); //enter
+      expect(digestQueue.length).toBe(0);
+
+      //we don't normally need this, but since the timing between digests
+      //is spaced-out then it is required so that the original digestion
+      //is kicked into gear
+      $rootScope.$digest();
+      $timeout.flush();
+
+      expect(element.data('state')).toBe('crazy-enter');
+      expect(enterComplete).toBe(true);
+    });
+  });
 });

test/ng/rootScopeSpec.js

@@ -672,6 +672,66 @@ describe('Scope', function() {
       expect(log).toEqual('parent.async;child.async;parent.$digest;child.$digest;');
     }));
 
+    it('should not run another digest for an $evalAsync call when it is external', inject(function($rootScope) {
+      var internalWatchCount = 0;
+      var externalWatchCount = 0;
+
+      $rootScope.internalCount = 0;
+      $rootScope.externalCount = 0;
+
+      $rootScope.$evalAsync(function(scope) {
+        $rootScope.internalCount++;
+      });
+
+      $rootScope.$evalAsync(function(scope) {
+        $rootScope.externalCount++;
+      }, false);
+
+      $rootScope.$watch('internalCount', function(value) {
+        internalWatchCount = value;
+      });
+      $rootScope.$watch('externalCount', function(value) {
+        externalWatchCount = value;
+      });
+
+      $rootScope.$digest();
+
+      expect(internalWatchCount).toEqual(1);
+      expect(externalWatchCount).toEqual(0);
+    }));
+
+    it('should run an external evalAsync on the rootscope when digest from below', inject(function($rootScope) {
+      var count = 0;
+      var scope = $rootScope.$new();
+      $rootScope.$evalAsync(function(scope) {
+        count++;
+      }, false);
+      scope.$digest();
+      expect(count).toEqual(1);
+    }));
+
+    it('should run an external evalAsync call on all child scopes when a parent scope is digested', inject(function($rootScope) {
+      var parent = $rootScope.$new(),
+          child = parent.$new(),
+          count = 0;
+
+      $rootScope.$evalAsync(function() {
+        count++;
+      }, false);
+
+      parent.$evalAsync(function() {
+        count++;
+      }, false);
+
+      child.$evalAsync(function() {
+        count++;
+      }, false);
+
+      expect(count).toBe(0);
+      $rootScope.$digest();
+      expect(count).toBe(3);
+    }));
+
     it('should cause a $digest rerun', inject(function($rootScope) {
       $rootScope.log = '';
       $rootScope.value = 0;
@@ -701,9 +761,9 @@ describe('Scope', function() {
       childScope.$evalAsync('childExpression');
       isolateScope.$evalAsync('isolateExpression');
 
-      expect(childScope.$$asyncQueue).toBe($rootScope.$$asyncQueue);
-      expect(isolateScope.$$asyncQueue).toBe($rootScope.$$asyncQueue);
-      expect($rootScope.$$asyncQueue).toEqual(['rootExpression', 'childExpression', 'isolateExpression']);
+      expect(childScope.$$internalAsyncQueue).toBe($rootScope.$$internalAsyncQueue);
+      expect(isolateScope.$$internalAsyncQueue).toBe($rootScope.$$internalAsyncQueue);
+      expect($rootScope.$$internalAsyncQueue).toEqual(['rootExpression', 'childExpression', 'isolateExpression']);
     }));
   });