fix(router): run navigations serialy

Closes #11754

modules/@angular/router/src/router.ts

@@ -8,10 +8,12 @@
 
 import {Location} from '@angular/common';
 import {Compiler, ComponentFactoryResolver, Injector, NgModuleFactoryLoader, ReflectiveInjector, Type} from '@angular/core';
+import {BehaviorSubject} from 'rxjs/BehaviorSubject';
 import {Observable} from 'rxjs/Observable';
 import {Subject} from 'rxjs/Subject';
 import {Subscription} from 'rxjs/Subscription';
 import {from} from 'rxjs/observable/from';
+import {fromPromise} from 'rxjs/observable/fromPromise';
 import {of } from 'rxjs/observable/of';
 import {concatMap} from 'rxjs/operator/concatMap';
 import {every} from 'rxjs/operator/every';
@@ -275,6 +277,16 @@ function defaultErrorHandler(error: any): any {
   throw error;
 }
 
+type NavigationParams = {
+  id: number,
+  rawUrl: UrlTree,
+  prevRawUrl: UrlTree,
+  extras: NavigationExtras,
+  resolve: any,
+  reject: any,
+  promise: Promise<boolean>
+};
+
 /**
  * @whatItDoes Provides the navigation and url manipulation capabilities.
  *
@@ -287,11 +299,13 @@ function defaultErrorHandler(error: any): any {
 export class Router {
   private currentUrlTree: UrlTree;
   private rawUrlTree: UrlTree;
-  private lastNavigation: UrlTree;
+
+  private navigations: BehaviorSubject<NavigationParams> =
+      new BehaviorSubject<NavigationParams>(null);
+  private routerEvents: Subject<Event> = new Subject<Event>();
 
   private currentRouterState: RouterState;
   private locationSubscription: Subscription;
-  private routerEvents: Subject<Event>;
   private navigationId: number = 0;
   private configLoader: RouterConfigLoader;
 
@@ -321,11 +335,12 @@ export class Router {
       private outletMap: RouterOutletMap, private location: Location, private injector: Injector,
       loader: NgModuleFactoryLoader, compiler: Compiler, public config: Routes) {
     this.resetConfig(config);
-    this.routerEvents = new Subject<Event>();
     this.currentUrlTree = createEmptyUrlTree();
     this.rawUrlTree = this.currentUrlTree;
     this.configLoader = new RouterConfigLoader(loader, compiler);
     this.currentRouterState = createEmptyState(this.currentUrlTree, this.rootComponentType);
+
+    this.processNavigations();
   }
 
   /**
@@ -355,18 +370,8 @@ export class Router {
     // which does not work properly with zone.js in IE and Safari
     this.locationSubscription = <any>this.location.subscribe(Zone.current.wrap((change: any) => {
       const rawUrlTree = this.urlSerializer.parse(change['url']);
-      const tree = this.urlHandlingStrategy.extract(rawUrlTree);
-
-
       setTimeout(() => {
-        // we fire multiple events for a single URL change
+        this.scheduleNavigation(rawUrlTree, {skipLocationChange: change['pop'], replaceUrl: true});
-        // we should navigate only once
-        if (!this.lastNavigation || this.lastNavigation.toString() !== tree.toString()) {
-          this.scheduleNavigation(
-              rawUrlTree, tree, {skipLocationChange: change['pop'], replaceUrl: true});
-        } else {
-          this.rawUrlTree = rawUrlTree;
-        }
       }, 0);
     }));
   }
@@ -488,10 +493,11 @@ export class Router {
   navigateByUrl(url: string|UrlTree, extras: NavigationExtras = {skipLocationChange: false}):
       Promise<boolean> {
     if (url instanceof UrlTree) {
-      return this.scheduleNavigation(this.rawUrlTree, url, extras);
+      return this.scheduleNavigation(this.urlHandlingStrategy.merge(url, this.rawUrlTree), extras);
     } else {
       const urlTree = this.urlSerializer.parse(url);
-      return this.scheduleNavigation(this.rawUrlTree, urlTree, extras);
+      return this.scheduleNavigation(
+          this.urlHandlingStrategy.merge(urlTree, this.rawUrlTree), extras);
     }
   }
 
@@ -518,7 +524,7 @@ export class Router {
    */
   navigate(commands: any[], extras: NavigationExtras = {skipLocationChange: false}):
       Promise<boolean> {
-    return this.scheduleNavigation(this.rawUrlTree, this.createUrlTree(commands, extras), extras);
+    return this.navigateByUrl(this.createUrlTree(commands, extras), extras);
   }
 
   /**
@@ -543,33 +549,76 @@ export class Router {
     }
   }
 
-  private scheduleNavigation(rawUrl: UrlTree, url: UrlTree, extras: NavigationExtras):
+  private processNavigations(): void {
-      Promise<boolean> {
+    concatMap
-    if (this.urlHandlingStrategy.shouldProcessUrl(url)) {
+        .call(
-      const id = ++this.navigationId;
+            this.navigations,
-      this.routerEvents.next(new NavigationStart(id, this.serializeUrl(url)));
+            (nav: NavigationParams) => {
+              if (nav) {
+                this.executeScheduledNavigation(nav);
+                // a failed navigation should not stop the router from processing
+                // further navigations => the catch
+                return nav.promise.catch(() => {});
+              } else {
+                return <any>of (null);
+              }
+            })
+        .subscribe(() => {});
+  }
 
-      return Promise.resolve().then(
+  private scheduleNavigation(rawUrl: UrlTree, extras: NavigationExtras): Promise<boolean> {
-          (_) => this.runNavigate(
+    const prevRawUrl = this.navigations.value ? this.navigations.value.rawUrl : null;
-              rawUrl, url, extras.skipLocationChange, extras.replaceUrl, id, null));
+    if (prevRawUrl && prevRawUrl.toString() === rawUrl.toString()) {
+      return this.navigations.value.promise;
+    }
+
+    let resolve: any = null;
+    let reject: any = null;
+
+    const promise = new Promise((res, rej) => {
+      resolve = res;
+      reject = rej;
+    });
+
+    const id = ++this.navigationId;
+    this.navigations.next({id, rawUrl, prevRawUrl, extras, resolve, reject, promise});
+
+    return promise;
+  }
+
+  private executeScheduledNavigation({id, rawUrl, prevRawUrl, extras, resolve,
+                                      reject}: NavigationParams): void {
+    const url = this.urlHandlingStrategy.extract(rawUrl);
+    const prevUrl = prevRawUrl ? this.urlHandlingStrategy.extract(prevRawUrl) : null;
+    const urlTransition = !prevUrl || url.toString() !== prevUrl.toString();
+
+    if (urlTransition && this.urlHandlingStrategy.shouldProcessUrl(rawUrl)) {
+      this.routerEvents.next(new NavigationStart(id, this.serializeUrl(url)));
+      Promise.resolve()
+          .then(
+              (_) => this.runNavigate(
+                  url, rawUrl, extras.skipLocationChange, extras.replaceUrl, id, null))
+          .then(resolve, reject);
 
       // we cannot process the current URL, but we could process the previous one =>
       // we need to do some cleanup
-    } else if (this.urlHandlingStrategy.shouldProcessUrl(this.rawUrlTree)) {
+    } else if (
-      const id = ++this.navigationId;
+        urlTransition && prevRawUrl && this.urlHandlingStrategy.shouldProcessUrl(prevRawUrl)) {
       this.routerEvents.next(new NavigationStart(id, this.serializeUrl(url)));
+      Promise.resolve()
+          .then(
+              (_) => this.runNavigate(
+                  url, rawUrl, false, false, id, createEmptyState(url, this.rootComponentType)))
+          .then(resolve, reject);
 
-      return Promise.resolve().then(
-          (_) => this.runNavigate(
-              rawUrl, url, false, false, id, createEmptyState(url, this.rootComponentType)));
     } else {
       this.rawUrlTree = rawUrl;
-      return Promise.resolve(null);
+      resolve(null);
     }
   }
 
   private runNavigate(
-      rawUrl: UrlTree, url: UrlTree, shouldPreventPushState: boolean, shouldReplaceUrl: boolean,
+      url: UrlTree, rawUrl: UrlTree, shouldPreventPushState: boolean, shouldReplaceUrl: boolean,
       id: number, precreatedState: RouterState): Promise<boolean> {
     if (id !== this.navigationId) {
       this.location.go(this.urlSerializer.serialize(this.currentUrlTree));
@@ -624,9 +673,15 @@ export class Router {
         preActivation.traverse(this.outletMap);
       });
 
-      const preactivation2$ = mergeMap.call(preactivation$, () => preActivation.checkGuards());
+      const preactivation2$ = mergeMap.call(preactivation$, () => {
+        if (this.navigationId !== id) return of (false);
+
+        return preActivation.checkGuards();
+      });
 
       const resolveData$ = mergeMap.call(preactivation2$, (shouldActivate: boolean) => {
+        if (this.navigationId !== id) return of (false);
+
         if (shouldActivate) {
           return map.call(preActivation.resolveData(), () => shouldActivate);
         } else {
@@ -641,7 +696,6 @@ export class Router {
               return;
             }
 
-            this.lastNavigation = appliedUrl;
             this.currentUrlTree = appliedUrl;
             this.rawUrlTree = this.urlHandlingStrategy.merge(this.currentUrlTree, rawUrl);
 

modules/@angular/router/src/router_preloader.ts

@@ -83,7 +83,8 @@ export class RouterPreloader {
 
   setUpPreloading(): void {
     const navigations = filter.call(this.router.events, (e: any) => e instanceof NavigationEnd);
-    this.subscription = concatMap.call(navigations, () => this.preload()).subscribe((v: any) => {});
+    this.subscription =
+        concatMap.call(navigations, () => this.preload()).subscribe((v: any) => {});
   }
 
   preload(): Observable<any> { return this.processRoutes(this.injector, this.router.config); }

modules/@angular/router/test/integration.spec.ts

@@ -20,7 +20,6 @@ import {forEach} from '../src/utils/collection';
 import {RouterTestingModule, SpyNgModuleFactoryLoader} from '../testing';
 
 
-
 describe('Integration', () => {
   beforeEach(() => {
     TestBed.configureTestingModule({
@@ -42,6 +41,75 @@ describe('Integration', () => {
        expect(location.path()).toEqual('/simple');
      })));
 
+  describe('should execute navigations serialy', () => {
+    let log: any[] = [];
+
+    beforeEach(() => {
+      log = [];
+
+      TestBed.configureTestingModule({
+        providers: [
+          {
+            provide: 'trueRightAway',
+            useValue: () => {
+              log.push('trueRightAway');
+              return true;
+            }
+          },
+          {
+            provide: 'trueIn2Seconds',
+            useValue: () => {
+              log.push('trueIn2Seconds-start');
+              let res: any = null;
+              const p = new Promise(r => res = r);
+              setTimeout(() => {
+                log.push('trueIn2Seconds-end');
+                res(true);
+              }, 2000);
+              return p;
+            }
+          }
+        ]
+      });
+    });
+
+    it('should execute navigations serialy',
+       fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
+         const fixture = createRoot(router, RootCmp);
+
+         router.resetConfig([
+           {path: 'a', component: SimpleCmp, canActivate: ['trueRightAway', 'trueIn2Seconds']},
+           {path: 'b', component: SimpleCmp, canActivate: ['trueRightAway', 'trueIn2Seconds']}
+         ]);
+
+         router.navigateByUrl('/a');
+         tick(100);
+         fixture.detectChanges();
+
+         router.navigateByUrl('/b');
+         tick(100);  // 200
+         fixture.detectChanges();
+
+         expect(log).toEqual(['trueRightAway', 'trueIn2Seconds-start']);
+
+         tick(2000);  // 2200
+         fixture.detectChanges();
+
+         expect(log).toEqual([
+           'trueRightAway', 'trueIn2Seconds-start', 'trueIn2Seconds-end', 'trueRightAway',
+           'trueIn2Seconds-start'
+         ]);
+
+         tick(2000);  // 4200
+         fixture.detectChanges();
+
+         expect(log).toEqual([
+           'trueRightAway', 'trueIn2Seconds-start', 'trueIn2Seconds-end', 'trueRightAway',
+           'trueIn2Seconds-start', 'trueIn2Seconds-end'
+         ]);
+       })));
+  });
+
   it('should work when an outlet is in an ngIf',
      fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
        const fixture = createRoot(router, RootCmp);
@@ -415,9 +483,9 @@ describe('Integration', () => {
          [NavigationStart, '/user/init'], [RoutesRecognized, '/user/init'],
          [NavigationEnd, '/user/init'],
 
-         [NavigationStart, '/user/victor'], [NavigationStart, '/user/fedor'],
+         [NavigationStart, '/user/victor'], [NavigationCancel, '/user/victor'],
 
-         [NavigationCancel, '/user/victor'], [RoutesRecognized, '/user/fedor'],
+         [NavigationStart, '/user/fedor'], [RoutesRecognized, '/user/fedor'],
          [NavigationEnd, '/user/fedor']
        ]);
      })));
@@ -1458,8 +1526,8 @@ describe('Integration', () => {
              expect(location.path()).toEqual('/blank');
 
              expectEvents(recordedEvents, [
-               [NavigationStart, '/lazyFalse/loaded'], [NavigationStart, '/blank'],
+               [NavigationStart, '/lazyFalse/loaded'], [NavigationCancel, '/lazyFalse/loaded'],
-               [RoutesRecognized, '/blank'], [NavigationCancel, '/lazyFalse/loaded'],
+               [NavigationStart, '/blank'], [RoutesRecognized, '/blank'],
                [NavigationEnd, '/blank']
              ]);
            })));
@@ -1961,6 +2029,7 @@ describe('Integration', () => {
 
                const config: any = router.config;
                const firstConfig = config[1]._loadedConfig;
+
                expect(firstConfig).toBeDefined();
                expect(firstConfig.routes[0].path).toEqual('LoadedModule1');
 
@@ -1979,8 +2048,11 @@ describe('Integration', () => {
 
         extract(url: UrlTree): UrlTree {
           const oldRoot = url.root;
-          const root = new UrlSegmentGroup(
+          const children: any = {};
-              oldRoot.segments, {[PRIMARY_OUTLET]: oldRoot.children[PRIMARY_OUTLET]});
+          if (oldRoot.children[PRIMARY_OUTLET]) {
+            children[PRIMARY_OUTLET] = oldRoot.children[PRIMARY_OUTLET];
+          }
+          const root = new UrlSegmentGroup(oldRoot.segments, children);
           return new UrlTree(root, url.queryParams, url.fragment);
         }