Some tweaks around $state.transition plus more tests (#41)

Author: ksperling

src/state.js

@@ -136,10 +136,7 @@ function $StateProvider(   $urlRouterProvider,   $urlMatcherFactory) {
       params: {},
       current: root.self,
       $current: root,
-
       transition: null,
-      $transition: $q.when(root.self),
-
 
       transitionTo: transitionTo,
 
@@ -165,12 +162,13 @@ function $StateProvider(   $urlRouterProvider,   $urlMatcherFactory) {
         locals = toLocals[keep] = state.locals;
       }
 
-      // If we're going to the same state and all locals are kept, we've got nothing to do. But
-      // update 'transition' anyway, as we still want to cancel any other pending transitions.
+      // If we're going to the same state and all locals are kept, we've got nothing to do.
+      // But clear 'transition', as we still want to cancel any other pending transitions.
       // TODO: We may not want to bump 'transition' if we're called from a location change that we've initiated ourselves,
       // because we might accidentally abort a legitimate transition initiated from code?
       if (to === from && locals === from.locals) {
-        return $state.$transition = $q.when($state.current);
+        $state.transition = null;
+        return $q.when($state.current);
       }
 
       // TODO: should we be passing from and to $stateParams as well?
@@ -193,10 +191,10 @@ function $StateProvider(   $urlRouterProvider,   $urlMatcherFactory) {
       // and return a promise for the new state. We also keep track of what the
       // current promise is, so that we can detect overlapping transitions and
       // keep only the outcome of the last transition.
-      var transition = $state.transition = $state.$transition = resolved.then(function () {
+      var transition = $state.transition = resolved.then(function () {
         var l, entering, exiting;
 
-        if ($state.transition !== transition) return; // superseded by a new transition
+        if ($state.transition !== transition) return $q.reject(new Error('transition superseded'));
 
         // Exit 'from' states not kept
         for (l=fromPath.length-1; l>=keep; l--) {

test/stateSpec.js

@@ -2,56 +2,119 @@ describe('state', function () {
 
   beforeEach(module('ui.state'));
 
-  describe('(transitions and related promises)', function () {
-    var A = { data: {} };
-    
-    beforeEach(module(function ($stateProvider) {
-      $stateProvider.state('A', A)
-    }));
+  var A = { data: {} }, B = {}, C = {};
+  
+  beforeEach(module(function ($stateProvider) {
+    $stateProvider
+      .state('A', A)
+      .state('B', B)
+      .state('C', C);
+  }));
+
+  var log = '';
+  function eventLogger(event, to, from) {
+    log += event.name + '(' + to.name + ',' + from.name + ');';
+  }
+
+  beforeEach(inject(function ($rootScope) {
+    $rootScope.$on('$stateChangeStart', eventLogger);
+    $rootScope.$on('$stateChangeSuccess', eventLogger);
+  }));
+
+  function $get(what) {
+    return jasmine.getEnv().currentSpec.$injector.get(what);
+  }
 
-    it('.current is always defined', inject(function ($state) {
+  function initStateTo(state) {
+    var $state = $get('$state'), $q = $get('$q');
+    $state.transitionTo(state, {});
+    $q.flush();
+    expect($state.current).toBe(state);
+    log = '';
+  }
+
+
+  describe('.current', function () {
+    it('is always defined', inject(function ($state) {
       expect($state.current).toBeDefined();
     }));
 
-    it('.$current is always defined', inject(function ($state) {
+    it('updates asynchronously as the transitionTo() promise is resolved', inject(function ($state, $q) {
+      var trans = $state.transitionTo(A, {});
+      expect($state.current).not.toBe(A);
+      $q.flush();
+      expect($state.current).toBe(A);
+    }));
+  });
+
+
+  describe('$current', function () {
+    it('is always defined', inject(function ($state) {
       expect($state.$current).toBeDefined();
     }));
 
-    it('.$current wraps the raw state object', inject(function ($state, $q) {
+    it('wraps the raw state object', inject(function ($state, $q) {
       $state.transitionTo(A, {});
       $q.flush();
       expect($state.$current.data).toBe(A.data); // 'data' is reserved for app use
     }));
+  });
+
+  
+  describe('.transition', function () {
+    it('is null when no transition is taking place', inject(function ($state, $q) {
+      expect($state.transition).toBeNull();
+      $state.transitionTo(A, {});
+      $q.flush();
+      expect($state.transition).toBeNull();
+    }));
+
+    it('is the current transition', inject(function ($state, $q) {
+      var trans = $state.transitionTo(A, {});
+      expect($state.transition).toBe(trans);
+    }));
+  });
 
-    it('.transitionTo() returns a promise for the target state', inject(function ($state, $q) {
+  
+  describe('.transitionTo()', function () {
+    it('returns a promise for the target state', inject(function ($state, $q) {
       var trans = $state.transitionTo(A, {});
       $q.flush();
       expect(resolvedValue(trans)).toBe(A);
     }));
 
-    it('.current updates asynchronously as the transitionTo() promise is resolved', inject(function ($state, $q) {
-      var trans = $state.transitionTo(A, {});
-      expect($state.current).not.toBe(A);
+    it('is a no-op when passing the current state and identical parameters', inject(function ($state, $q) {
+      initStateTo(A);
+      var trans = $state.transitionTo(A, {}); // no-op
+      expect(trans).toBeDefined(); // but we still get a valid promise
       $q.flush();
+      expect(resolvedValue(trans)).toBe(A);
       expect($state.current).toBe(A);
+      expect(log).toBe('');
     }));
 
-    it('.$transition is always the current or last transition', inject(function ($state, $q) {
-      expect($state.$transition).toBeDefined();
+    it('aborts pending transitions (last call wins)', inject(function ($state, $q) {
+      initStateTo(A);
+      var superseded = $state.transitionTo(B, {});
+      $state.transitionTo(C, {});
       $q.flush();
-      expect(resolvedValue($state.$transition)).toBe($state.current);
-      var trans = $state.transitionTo(A, {});
-      expect($state.$transition).toBeDefined();
-      expect($state.$transition).toBe(trans);
-      $q.flush();
-      expect($state.$transition).toBe(trans);
+      expect($state.current).toBe(C);
+      expect(resolvedError(superseded)).toBeDefined();
+      expect(log).toBe(
+        '$stateChangeStart(B,A);' +
+        '$stateChangeStart(C,A);' +
+        '$stateChangeSuccess(C,A);');
     }));
 
-    it('.transition is null when no transition is taking place', inject(function ($state, $q) {
-      expect($state.transition).toBeNull();
+    it('aborts pending transitions even when going back to the curren state', inject(function ($state, $q) {
+      initStateTo(A);
+      var superseded = $state.transitionTo(B, {});
       $state.transitionTo(A, {});
       $q.flush();
-      expect($state.transition).toBeNull();
+      expect($state.current).toBe(A);
+      expect(resolvedError(superseded)).toBeDefined();
+      expect(log).toBe(
+        '$stateChangeStart(B,A);');
     }));
   });
 });