fix(router): transition race condition

fixes #14873
fixes #15090

angular/src/directives/navigation/ion-router-outlet.ts

@@ -162,11 +162,11 @@ export class IonRouterOutlet implements OnDestroy, OnInit {
 }
 
 function emitEvent(el: HTMLElement) {
-  const event = new CustomEvent('ionRouterOutletActivated', {
+  const ev = new CustomEvent('ionRouterOutletActivated', {
     bubbles: true,
     cancelable: true,
   });
-  el.dispatchEvent(event);
+  el.dispatchEvent(ev);
 }
 
 class OutletInjector implements Injector {

angular/src/directives/navigation/router-controller.ts

@@ -39,7 +39,6 @@ export class StackController {
     const leavingView = this.getActive();
     this.insertView(enteringView, direction);
     await this.transition(enteringView, leavingView, direction, animated, this.canGoBack(1));
-
     this.cleanup();
   }
 

angular/src/ionic-module.ts

@@ -1,4 +1,5 @@
 import { APP_INITIALIZER, ModuleWithProviders, NgModule } from '@angular/core';
+import { IonicConfig } from '@ionic/core';
 import { CommonModule } from '@angular/common';
 
 import { appInitialize } from './app-initialize';
@@ -133,7 +134,7 @@ const PROVIDERS = [
   imports: [CommonModule]
 })
 export class IonicModule {
-  static forRoot(config?: { [key: string]: any }): ModuleWithProviders {
+  static forRoot(config?: IonicConfig): ModuleWithProviders {
     return {
       ngModule: IonicModule,
       providers: [

angular/src/providers/config.ts

@@ -1,35 +1,35 @@
-import { Config as CoreConfig } from '@ionic/core';
+import { Config as CoreConfig, IonicConfig } from '@ionic/core';
 import { InjectionToken } from '@angular/core';
 import { IonicWindow } from '../types/interfaces';
 
 
 export class Config {
 
-  get(key: string, fallback?: any): any {
+  get(key: keyof IonicConfig, fallback?: any): any {
     const c = getConfig();
     if (c) {
       return c.get(key, fallback);
     }
     return null;
   }
 
-  getBoolean(key: string, fallback?: boolean): boolean {
+  getBoolean(key: keyof IonicConfig, fallback?: boolean): boolean {
     const c = getConfig();
     if (c) {
       return c.getBoolean(key, fallback);
     }
     return false;
   }
 
-  getNumber(key: string, fallback?: number): number {
+  getNumber(key: keyof IonicConfig, fallback?: number): number {
     const c = getConfig();
     if (c) {
       return c.getNumber(key, fallback);
     }
     return 0;
   }
 
-  set(key: string, value?: any) {
+  set(key: keyof IonicConfig, value?: any) {
     const c = getConfig();
     if (c) {
       c.set(key, value);

core/src/components/router-outlet/route-outlet.tsx

@@ -11,9 +11,9 @@ import { attachComponent, detachComponent } from '../../utils/framework-delegate
 })
 export class RouterOutlet implements NavOutlet {
 
-  private isTransitioning = false;
   private activeEl: HTMLElement | undefined;
   private activeComponent: any;
+  private waitPromise?: Promise<void>;
 
   mode!: Mode;
 
@@ -36,7 +36,6 @@ export class RouterOutlet implements NavOutlet {
     if (this.animated === undefined) {
       this.animated = this.config.getBoolean('animate', true);
     }
-
     this.ionNavWillLoad.emit();
   }
 
@@ -49,20 +48,19 @@ export class RouterOutlet implements NavOutlet {
    */
   @Method()
   async setRoot(component: ComponentRef, params?: ComponentProps, opts?: RouterOutletOptions): Promise<boolean> {
-    if (this.isTransitioning || this.activeComponent === component) {
+    if (this.activeComponent === component) {
       return false;
     }
-    this.activeComponent = component;
 
     // attach entering view to DOM
-    const enteringEl = await attachComponent(this.delegate, this.el, component, ['ion-page', 'ion-page-invisible'], params);
     const leavingEl = this.activeEl;
+    const enteringEl = await attachComponent(this.delegate, this.el, component, ['ion-page', 'ion-page-invisible'], params);
+
+    this.activeComponent = component;
+    this.activeEl = enteringEl;
 
     // commit animation
     await this.commit(enteringEl, leavingEl, opts);
-
-    // remove leaving view
-    this.activeEl = enteringEl;
     detachComponent(this.delegate, leavingEl);
 
     return true;
@@ -71,11 +69,56 @@ export class RouterOutlet implements NavOutlet {
   /** @hidden */
   @Method()
   async commit(enteringEl: HTMLElement, leavingEl: HTMLElement | undefined, opts?: RouterOutletOptions): Promise<boolean> {
+    const unlock = await this.lock();
+    let changed = false;
+    try {
+      changed = await this.transition(enteringEl, leavingEl, opts);
+    } catch (e) {
+      console.error(e);
+    }
+    unlock();
+    return changed;
+  }
+
+  /** @hidden */
+  @Method()
+  async setRouteId(id: string, params: any, direction: number): Promise<RouteWrite> {
+    const changed = await this.setRoot(id, params, {
+      duration: direction === 0 ? 0 : undefined,
+      direction: direction === -1 ? 'back' : 'forward',
+    });
+    return {
+      changed,
+      element: this.activeEl
+    };
+  }
+
+  /** Returns the ID for the current route */
+  @Method()
+  getRouteId(): RouteID | undefined {
+    const active = this.activeEl;
+    return active ? {
+      id: active.tagName,
+      element: active,
+    } : undefined;
+  }
+
+  private async lock() {
+    const p = this.waitPromise;
+    let resolve!: () => void;
+    this.waitPromise = new Promise(r => resolve = r);
+
+    if (p) {
+      await p;
+    }
+    return resolve;
+  }
+
+  async transition(enteringEl: HTMLElement, leavingEl: HTMLElement | undefined, opts?: RouterOutletOptions): Promise<boolean> {
     // isTransitioning acts as a lock to prevent reentering
-    if (this.isTransitioning || leavingEl === enteringEl) {
+    if (leavingEl === enteringEl) {
       return false;
     }
-    this.isTransitioning = true;
 
     // emit nav will change event
     this.ionNavWillChange.emit();
@@ -95,34 +138,11 @@ export class RouterOutlet implements NavOutlet {
 
       ...opts
     });
-    this.isTransitioning = false;
 
     // emit nav changed event
     this.ionNavDidChange.emit();
-    return true;
-  }
 
-  /** @hidden */
-  @Method()
-  async setRouteId(id: string, params: any, direction: number): Promise<RouteWrite> {
-    const changed = await this.setRoot(id, params, {
-      duration: direction === 0 ? 0 : undefined,
-      direction: direction === -1 ? 'back' : 'forward',
-    });
-    return {
-      changed,
-      element: this.activeEl
-    };
-  }
-
-  /** Returns the ID for the current route */
-  @Method()
-  getRouteId(): RouteID | undefined {
-    const active = this.activeEl;
-    return active ? {
-      id: active.tagName,
-      element: active,
-    } : undefined;
+    return true;
   }
 
   render() {

core/src/components/router-outlet/test/basic/index.html

@@ -75,7 +75,28 @@ <h1>Page Three</h1>
       <ion-route url="/two" component="page-two"> </ion-route>
       <ion-route url="/page-3" component="page-three"> </ion-route>
     </ion-router>
-    <ion-router-outlet></ion-router-outlet>
+
+    <ion-split-pane>
+      <ion-menu>
+        <ion-header>
+          <ion-toolbar>
+            <ion-title>Menu</ion-title>
+          </ion-toolbar>
+        </ion-header>
+        <ion-content>
+          <ion-item href="#/">
+            <ion-label>Page 1</ion-label>
+          </ion-item>
+          <ion-item href="#/two">
+            <ion-label>Page 2</ion-label>
+          </ion-item>
+          <ion-item href="#/page-3">
+            <ion-label>Page 3</ion-label>
+          </ion-item>
+        </ion-content>
+      </ion-menu>
+      <ion-router-outlet main></ion-router-outlet>
+    </ion-split-pane>
   </ion-app>
 </body>
 

core/src/components/router/router.tsx

@@ -19,6 +19,7 @@ export class Router {
   private busy = false;
   private state = 0;
   private lastState = 0;
+  private waitPromise?: Promise<void>;
 
   @Element() el!: HTMLElement;
 
@@ -82,10 +83,10 @@ export class Router {
   /** Navigate to the specified URL */
   @Method()
   push(url: string, direction: RouterDirection = 'forward') {
-    const path = parsePath(url);
-    const intent = DIRECTION_TO_INTENT[direction];
     console.debug('[ion-router] URL pushed -> updating nav', url, direction);
 
+    const path = parsePath(url);
+    const intent = DIRECTION_TO_INTENT[direction];
     this.setPath(path, intent);
     return this.writeNavStateRoot(path, intent);
   }
@@ -103,6 +104,7 @@ export class Router {
   @Method()
   async navChanged(intent: number): Promise<boolean> {
     if (this.busy) {
+      console.warn('[ion-router] router is busy, navChanged was cancelled');
       return false;
     }
     const { ids, outlet } = readNavState(this.win.document.body);
@@ -122,7 +124,7 @@ export class Router {
     console.debug('[ion-router] nav changed -> update URL', ids, path);
     this.setPath(path, intent);
 
-    await this.writeNavState(outlet, chain, RouterIntent.None, path, null, ids.length);
+    await this.safeWriteNavState(outlet, chain, RouterIntent.None, path, null, ids.length);
     return true;
   }
 
@@ -157,9 +159,6 @@ export class Router {
   }
 
   private async writeNavStateRoot(path: string[] | null, intent: RouterIntent): Promise<boolean> {
-    if (this.busy) {
-      return false;
-    }
     if (!path) {
       console.error('[ion-router] URL is not part of the routing set');
       return false;
@@ -184,7 +183,34 @@ export class Router {
     }
 
     // write DOM give
-    return this.writeNavState(this.win.document.body, chain, intent, path, redirectFrom);
+    return this.safeWriteNavState(this.win.document.body, chain, intent, path, redirectFrom);
+  }
+
+  private async safeWriteNavState(
+    node: HTMLElement | undefined, chain: RouteChain, intent: RouterIntent,
+    path: string[], redirectFrom: string[] | null,
+    index = 0
+  ): Promise<boolean> {
+    const unlock = await this.lock();
+    let changed = false;
+    try {
+      changed = await this.writeNavState(node, chain, intent, path, redirectFrom, index);
+    } catch (e) {
+      console.error(e);
+    }
+    unlock();
+    return changed;
+  }
+
+  private async lock() {
+    const p = this.waitPromise;
+    let resolve!: () => void;
+    this.waitPromise = new Promise(r => resolve = r);
+
+    if (p) {
+      await p;
+    }
+    return resolve;
   }
 
   private async writeNavState(
@@ -193,6 +219,7 @@ export class Router {
     index = 0
   ): Promise<boolean> {
     if (this.busy) {
+      console.warn('[ion-router] router is busy, transition was cancelled');
       return false;
     }
     this.busy = true;

core/src/global/config.ts

@@ -1,5 +1,12 @@
+import { Mode } from '../interface';
 
 export interface IonicConfig {
+  /**
+   * The mode determines which platform styles to use.
+   * Possible values are: `"ios"` or `"md"`.
+   */
+  mode?: Mode;
+
   isDevice?: boolean;
   statusbarPadding?: boolean;
   inputShims?: boolean;
@@ -12,7 +19,6 @@ export interface IonicConfig {
   pickerSpinner?: string;
   refreshingIcon?: string;
   refreshingSpinner?: string;
-  mode?: string;
   menuType?: string;
   scrollPadding?: string;
   inputBlurring?: string;