fix(router-store): NavigationCancel and NavigationError creates a cyc…

…le when used with routerReducer Closes #68
ngrx · Jul 14, 2017 · a085730 · a085730
1 parent 3827947
commit a085730
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 35 deletions.
diff --git a/modules/router-store/spec/integration.spec.ts b/modules/router-store/spec/integration.spec.ts
@@ -7,11 +7,9 @@ import {
   ROUTER_CANCEL,
   ROUTER_ERROR,
   ROUTER_NAVIGATION,
+  RouterAction,
   routerReducer,
   StoreRouterConnectingModule,
-  RouterNavigationAction,
-  RouterCancelAction,
-  RouterAction,
 } from '../src/index';
 import 'rxjs/add/operator/filter';
 import 'rxjs/add/operator/first';
@@ -106,15 +104,18 @@ describe('integration spec', () => {
       } else if (action.type === ROUTER_CANCEL) {
         return {
           url: action.payload.routerState.url.toString(),
-          storeState: action.payload.storeState,
+          storeState: action.payload.storeState.reducer,
           lastAction: ROUTER_CANCEL,
         };
       } else {
         return state;
       }
     };
 
-    createTestModule({ reducers: { reducer }, canActivate: () => false });
+    createTestModule({
+      reducers: { reducer, routerReducer },
+      canActivate: () => false,
+    });
 
     const router: Router = TestBed.get(Router);
     const store = TestBed.get(Store);
@@ -128,6 +129,7 @@ describe('integration spec', () => {
       })
       .then(r => {
         expect(r).toEqual(false);
+
         expect(log).toEqual([
           { type: 'router', event: 'NavigationStart', url: '/next' },
           { type: 'router', event: 'RoutesRecognized', url: '/next' },
@@ -140,9 +142,7 @@ describe('integration spec', () => {
             state: {
               url: '/next',
               lastAction: ROUTER_CANCEL,
-              storeState: {
-                reducer: { url: '/next', lastAction: ROUTER_NAVIGATION },
-              },
+              storeState: { url: '/next', lastAction: ROUTER_NAVIGATION },
             },
           },
           { type: 'router', event: 'NavigationCancel', url: '/next' },
@@ -162,7 +162,7 @@ describe('integration spec', () => {
       } else if (action.type === ROUTER_ERROR) {
         return {
           url: action.payload.routerState.url.toString(),
-          storeState: action.payload.storeState,
+          storeState: action.payload.storeState.reducer,
           lastAction: ROUTER_ERROR,
         };
       } else {
@@ -171,7 +171,7 @@ describe('integration spec', () => {
     };
 
     createTestModule({
-      reducers: { reducer },
+      reducers: { reducer, routerReducer },
       canActivate: () => {
         throw new Error('BOOM!');
       },
@@ -202,9 +202,7 @@ describe('integration spec', () => {
             state: {
               url: '/next',
               lastAction: ROUTER_ERROR,
-              storeState: {
-                reducer: { url: '/next', lastAction: ROUTER_NAVIGATION },
-              },
+              storeState: { url: '/next', lastAction: ROUTER_NAVIGATION },
             },
           },
           { type: 'router', event: 'NavigationError', url: '/next' },

diff --git a/modules/router-store/src/router_store_module.ts b/modules/router-store/src/router_store_module.ts
@@ -166,7 +166,8 @@ export class StoreRouterConnectingModule {
       routerState: RouterStateSnapshot
     ) => {
       this.routerState = routerState;
-      if (this.shouldDispatch()) this.dispatchEvent();
+      if (this.shouldDispatchRouterNavigation())
+        this.dispatchRouterNavigation();
       return of(true);
     };
   }
@@ -178,21 +179,7 @@ export class StoreRouterConnectingModule {
     });
   }
 
-  private dispatchEvent(): void {
-    this.dispatchTriggeredByRouter = true;
-    try {
-      const payload = {
-        routerState: this.routerState,
-        event: this.lastRoutesRecognized,
-      };
-      this.store.dispatch({ type: ROUTER_NAVIGATION, payload });
-    } finally {
-      this.dispatchTriggeredByRouter = false;
-      this.navigationTriggeredByDispatch = false;
-    }
-  }
-
-  private shouldDispatch(): boolean {
+  private shouldDispatchRouterNavigation(): boolean {
     if (!this.storeState['routerReducer']) return true;
     return !this.navigationTriggeredByDispatch;
   }
@@ -219,21 +206,36 @@ export class StoreRouterConnectingModule {
     });
   }
 
+  private dispatchRouterNavigation(): void {
+    this.dispatchRouterAction(ROUTER_NAVIGATION, {
+      routerState: this.routerState,
+      event: this.lastRoutesRecognized,
+    });
+  }
+
   private dispatchRouterCancel(event: NavigationCancel): void {
-    const payload = {
+    this.dispatchRouterAction(ROUTER_CANCEL, {
       routerState: this.routerState,
       storeState: this.storeState,
       event,
-    };
-    this.store.dispatch({ type: ROUTER_CANCEL, payload });
+    });
   }
 
   private dispatchRouterError(event: NavigationError): void {
-    const payload = {
+    this.dispatchRouterAction(ROUTER_ERROR, {
       routerState: this.routerState,
       storeState: this.storeState,
       event,
-    };
-    this.store.dispatch({ type: ROUTER_ERROR, payload });
+    });
+  }
+
+  private dispatchRouterAction(type: string, payload: any): void {
+    this.dispatchTriggeredByRouter = true;
+    try {
+      this.store.dispatch({ type, payload });
+    } finally {
+      this.dispatchTriggeredByRouter = false;
+      this.navigationTriggeredByDispatch = false;
+    }
   }
 }