refactor(NavController): adds better error handling

fixes #10090

src/components/app/app.ts

@@ -220,8 +220,6 @@ export class App {
   present(enteringView: ViewController, opts: NavOptions, appPortal?: AppPortal): Promise<any> {
     const portal = this._appRoot._getPortal(appPortal);
 
-    enteringView._setNav(portal);
-
     opts.keyboardClose = false;
     opts.direction = DIRECTION_FORWARD;
 

src/components/menu/menu-controller.ts

@@ -313,7 +313,7 @@ export class MenuController {
    * @private
    */
   _setActiveMenu(menu: Menu) {
-    assert(menu.enabled);
+    assert(menu.enabled, 'menu must be enabled');
     assert(this._menus.indexOf(menu) >= 0, 'menu is not registered');
 
     // if this menu should be enabled

src/components/popover/test/basic/app.module.ts

@@ -1,5 +1,5 @@
 import { Component, ViewChild, ElementRef, ViewEncapsulation, NgModule } from '@angular/core';
-import { IonicApp, IonicModule, PopoverController, NavParams, ViewController } from '../../../../../ionic-angular';
+import { IonicApp, IonicModule, PopoverController, NavParams, ToastController, ViewController } from '../../../../../ionic-angular';
 
 
 @Component({
@@ -188,7 +188,10 @@ export class E2EPage {
   @ViewChild('popoverContent', {read: ElementRef}) content: ElementRef;
   @ViewChild('popoverText', {read: ElementRef}) text: ElementRef;
 
-  constructor(private popoverCtrl: PopoverController) {}
+  constructor(
+    private popoverCtrl: PopoverController,
+    private toastCtrl: ToastController,
+  ) { }
 
   presentListPopover(ev: UIEvent) {
     let popover = this.popoverCtrl.create(PopoverListPage);
@@ -221,6 +224,13 @@ export class E2EPage {
     this.popoverCtrl.create(PopoverListPage).present();
   }
 
+  presentToast() {
+    this.toastCtrl.create({
+      message: 'Toast example',
+      duration: 1000
+    }).present();
+  }
+
 }
 
 

src/components/popover/test/basic/main.html

@@ -62,6 +62,11 @@
     <div>Aenean rhoncus urna at interdum blandit. Donec ac massa nec libero vehicula tincidunt. Sed sit amet hendrerit risus. Aliquam vitae vestibulum ipsum, non feugiat orci. Vivamus eu rutrum elit. Nulla dapibus tortor non dignissim pretium. Nulla in luctus turpis. Etiam non mattis tortor, at aliquet ex. Nunc ut ante varius, auctor dui vel, volutpat elit. Nunc laoreet augue sit amet ultrices porta. Vestibulum ante ipsum primis in faucibus orci luctus et ultrices posuere cubilia Curae; Vestibulum pellentesque lobortis est, ut tincidunt ligula mollis sit amet. In porta risus arcu, quis pellentesque dolor mattis non. Vestibulum ante ipsum primis in faucibus orci luctus et ultrices posuere cubilia Curae;</div>
   </div>
 
+
+  <button ion-button block color="secondary" (click)="presentToast()">
+    Open Toast
+  </button>
+
 </ion-content>
 
 

src/navigation/nav-controller-base.ts

@@ -202,8 +202,8 @@ export class NavControllerBase extends Ion implements NavController {
       });
     }
 
+    // ti.resolve() is called when the navigation transition is finished successfully
     ti.resolve = (hasCompleted: boolean, isAsync: boolean, enteringName: string, leavingName: string, direction: string) => {
-      // transition has successfully resolved
       this._trnsId = null;
       this._init = true;
       resolve && resolve(hasCompleted, isAsync, enteringName, leavingName, direction);
@@ -214,23 +214,22 @@ export class NavControllerBase extends Ion implements NavController {
       this._nextTrns();
     };
 
-    ti.reject = (rejectReason: any, trns: Transition) => {
+    // ti.reject() is called when the navigation transition fails. ie. it is rejected at some point.
-      // rut row raggy, something rejected this transition
+    ti.reject = (rejectReason: any, transition: Transition) => {
       this._trnsId = null;
       this._queue.length = 0;
 
-      while (trns) {
+      // walk through the transition views so they are destroyed
-        if (trns.enteringView && (trns.enteringView._state !== ViewState.LOADED)) {
+      while (transition) {
-          // destroy the entering views and all of their hopes and dreams
+        var enteringView = transition.enteringView;
-          this._destroyView(trns.enteringView);
+        if (enteringView && (enteringView._state === ViewState.ATTACHED)) {
+          this._destroyView(enteringView);
         }
-        if (!trns.parent) {
+        if (transition.isRoot()) {
+          this._trnsCtrl.destroy(transition.trnsId);
           break;
         }
-      }
+        transition = transition.parent;
-
-      if (trns) {
-        this._trnsCtrl.destroy(trns.trnsId);
       }
 
       reject && reject(false, false, rejectReason);
@@ -278,7 +277,19 @@ export class NavControllerBase extends Ion implements NavController {
       return false;
     }
 
-    // Get entering and leaving views
+    // ensure any of the inserted view are used
+    const insertViews = ti.insertViews;
+    if (insertViews) {
+      for (var i = 0; i < insertViews.length; i++) {
+        var nav = insertViews[i]._nav;
+        if (nav && nav !== this || insertViews[i]._state === ViewState.DESTROYED) {
+          ti.reject('leavingView and enteringView are null. stack is already empty');
+          return false;
+        }
+      }
+    }
+
+    // get entering and leaving views
     const leavingView = this.getActive();
     const enteringView = this._getEnteringView(ti, leavingView);
 
@@ -291,7 +302,7 @@ export class NavControllerBase extends Ion implements NavController {
     this.setTransitioning(true);
 
     // Initialize enteringView
-    if (enteringView && isBlank(enteringView._state)) {
+    if (enteringView && enteringView._state === ViewState.NEW) {
       // render the entering view, and all child navs and views
       // ******** DOM WRITE ****************
       this._viewInit(enteringView);
@@ -303,7 +314,6 @@ export class NavControllerBase extends Ion implements NavController {
       // views have been initialized, now let's test
       // to see if the transition is even allowed or not
       return this._viewTest(enteringView, leavingView, ti);
-
     } else {
       return this._postViewInit(enteringView, leavingView, ti);
     }
@@ -419,7 +429,6 @@ export class NavControllerBase extends Ion implements NavController {
       // add the views to the
       for (i = 0; i < insertViews.length; i++) {
         view = insertViews[i];
-        assert(view, 'view must be non null');
         this._insertViewAt(view, ti.insertStart + i);
       }
 
@@ -477,6 +486,9 @@ export class NavControllerBase extends Ion implements NavController {
    * DOM WRITE
    */
   _viewInit(enteringView: ViewController) {
+    assert(enteringView, 'enteringView must be non null');
+    assert(enteringView._state === ViewState.NEW, 'enteringView state must be NEW');
+
     // entering view has not been initialized yet
     const componentProviders = ReflectiveInjector.resolve([
       { provide: NavController, useValue: this },
@@ -501,7 +513,7 @@ export class NavControllerBase extends Ion implements NavController {
     // render the component ref instance to the DOM
     // ******** DOM WRITE ****************
     viewport.insert(componentRef.hostView, viewport.length);
-    view._state = ViewState.PRE_RENDERED;
+    view._state = ViewState.ATTACHED;
 
     if (view._cssClass) {
       // the ElementRef of the actual ion-page created
@@ -604,14 +616,12 @@ export class NavControllerBase extends Ion implements NavController {
       }
     });
 
-    if (enteringView && enteringView._state === ViewState.INITIALIZED) {
+    if (enteringView && (enteringView._state === ViewState.INITIALIZED)) {
       // render the entering component in the DOM
       // this would also render new child navs/views
       // which may have their very own async canEnter/Leave tests
       // ******** DOM WRITE ****************
       this._viewAttachToDOM(enteringView, enteringView._cmp, this._viewport);
-    } else {
-      console.debug('enteringView state is not INITIALIZED', enteringView);
     }
 
     if (!transition.hasChildren) {
@@ -762,9 +772,10 @@ export class NavControllerBase extends Ion implements NavController {
     if (existingIndex > -1) {
       // this view is already in the stack!!
       // move it to its new location
+      assert(view._nav === this, 'view is not part of the nav');
       this._views.splice(index, 0, this._views.splice(existingIndex, 1)[0]);
-
     } else {
+      assert(!view._nav, 'nav is used');
       // this is a new view to add to the stack
       // create the new entering view
       view._setNav(this);
@@ -781,6 +792,8 @@ export class NavControllerBase extends Ion implements NavController {
   }
 
   _removeView(view: ViewController) {
+    assert(view._state === ViewState.ATTACHED || view._state === ViewState.DESTROYED, 'view state should be loaded or destroyed');
+
     const views = this._views;
     const index = views.indexOf(view);
     assert(index > -1, 'view must be part of the stack');
@@ -1056,7 +1069,7 @@ export class NavControllerBase extends Ion implements NavController {
   dismissPageChangeViews() {
     for (let view of this._views) {
       if (view.data && view.data.dismissOnPageChange) {
-        view.dismiss();
+        view.dismiss().catch(null);
       }
     }
   }

src/navigation/nav-util.ts

@@ -193,9 +193,10 @@ export interface TransitionInstruction {
 }
 
 export enum ViewState {
-  INITIALIZED,
+  NEW, // New created ViewController
-  PRE_RENDERED,
+  INITIALIZED, // Initialized by the NavController
-  LOADED,
+  ATTACHED, // Loaded to the DOM
+  DESTROYED // Destroyed by the NavController
 }
 
 export const INIT_ZINDEX = 100;

src/navigation/test/view-controller.spec.ts

@@ -1,5 +1,5 @@
 import { mockNavController, mockView, mockViews } from '../../util/mock-providers';
-
+import { ViewState } from '../nav-util';
 
 describe('ViewController', () => {
 
@@ -16,6 +16,7 @@ describe('ViewController', () => {
       });
 
       // act
+      viewController._state = ViewState.ATTACHED;
       viewController._willEnter();
     }, 10000);
   });
@@ -33,6 +34,7 @@ describe('ViewController', () => {
       });
 
       // act
+      viewController._state = ViewState.ATTACHED;
       viewController._didEnter();
     }, 10000);
   });
@@ -50,6 +52,7 @@ describe('ViewController', () => {
       });
 
       // act
+      viewController._state = ViewState.ATTACHED;
       viewController._willLeave(false);
     }, 10000);
   });

src/navigation/view-controller.ts

@@ -1,7 +1,7 @@
 import { ComponentRef, ElementRef, EventEmitter, Output, Renderer } from '@angular/core';
 
 import { Footer, Header } from '../components/toolbar/toolbar';
-import { isPresent } from '../util/util';
+import { isPresent, assert } from '../util/util';
 import { Navbar } from '../components/navbar/navbar';
 import { NavController } from './nav-controller';
 import { NavOptions, ViewState } from './nav-util';
@@ -46,7 +46,7 @@ export class ViewController {
   _cmp: ComponentRef<any>;
   _nav: NavController;
   _zIndex: number;
-  _state: ViewState;
+  _state: ViewState = ViewState.NEW;
   _cssClass: string;
 
   /**
@@ -227,7 +227,7 @@ export class ViewController {
    * @private
    */
   get name(): string {
-    return this.component ? this.component.name : '';
+    return (this.component ? this.component.name : '');
   }
 
   /**
@@ -261,14 +261,12 @@ export class ViewController {
     // _hidden value of '' means the hidden attribute will be added
     // _hidden value of null means the hidden attribute will be removed
     // doing checks to make sure we only update the DOM when actually needed
-    if (this._cmp) {
+    // if it should render, then the hidden attribute should not be on the element
-      // if it should render, then the hidden attribute should not be on the element
+    if (this._cmp && shouldShow === this._isHidden) {
-      if (shouldShow === this._isHidden) {
+      this._isHidden = !shouldShow;
-        this._isHidden = !shouldShow;
+      let value = (shouldShow ? null : '');
-        let value = (shouldShow ? null : '');
+      // ******** DOM WRITE ****************
-        // ******** DOM WRITE ****************
+      renderer.setElementAttribute(this.pageRef().nativeElement, 'hidden', value);
-        renderer.setElementAttribute(this.pageRef().nativeElement, 'hidden', value);
-      }
     }
   }
 
@@ -411,6 +409,7 @@ export class ViewController {
   }
 
   _preLoad() {
+    assert(this._state === ViewState.INITIALIZED, 'view state must be INITIALIZED');
     this._lifecycle('PreLoad');
   }
 
@@ -420,6 +419,7 @@ export class ViewController {
    * This event is fired before the component and his children have been initialized.
    */
   _willLoad() {
+    assert(this._state === ViewState.INITIALIZED, 'view state must be INITIALIZED');
     this._lifecycle('WillLoad');
   }
 
@@ -432,6 +432,7 @@ export class ViewController {
    * recommended method to use when a view becomes active.
    */
   _didLoad() {
+    assert(this._state === ViewState.ATTACHED, 'view state must be ATTACHED');
     this._lifecycle('DidLoad');
   }
 
@@ -440,6 +441,8 @@ export class ViewController {
    * The view is about to enter and become the active view.
    */
   _willEnter() {
+    assert(this._state === ViewState.ATTACHED, 'view state must be ATTACHED');
+
     if (this._detached && this._cmp) {
       // ensure this has been re-attached to the change detector
       this._cmp.changeDetectorRef.reattach();
@@ -456,6 +459,8 @@ export class ViewController {
    * will fire, whether it was the first load or loaded from the cache.
    */
   _didEnter() {
+    assert(this._state === ViewState.ATTACHED, 'view state must be ATTACHED');
+
     this._nb && this._nb.didEnter();
     this.didEnter.emit(null);
     this._lifecycle('DidEnter');
@@ -510,6 +515,8 @@ export class ViewController {
    * DOM WRITE
    */
   _destroy(renderer: Renderer) {
+    assert(this._state !== ViewState.DESTROYED, 'view state must be ATTACHED');
+
     if (this._cmp) {
       if (renderer) {
         // ensure the element is cleaned up for when the view pool reuses this element
@@ -524,6 +531,7 @@ export class ViewController {
     }
 
     this._nav = this._cmp = this.instance = this._cntDir = this._cntRef = this._leavingOpts = this._hdrDir = this._ftrDir = this._nb = this._onDidDismiss = this._onWillDismiss = null;
+    this._state = ViewState.DESTROYED;
   }
 
   /**
@@ -534,7 +542,7 @@ export class ViewController {
     const methodName = 'ionViewCan' + lifecycle;
     if (instance && instance[methodName]) {
       try {
-        let result = instance[methodName]();
+        var result = instance[methodName]();
         if (result === false) {
           return false;
         } else if (result instanceof Promise) {

src/util/mock-providers.ts

@@ -348,7 +348,9 @@ export function mockView(component?: any, data?: any) {
 
 export function mockViews(nav: NavControllerBase, views: ViewController[]) {
   nav._views = views;
-  views.forEach(v => v._setNav(nav));
+  views.forEach(v => {
+    v._setNav(nav);
+  });
 }
 
 export function mockComponentRef(): ComponentRef<any> {

src/util/util.ts

@@ -170,7 +170,7 @@ function _runInDev(fn: Function) {
 
 
 /** @private */
-function _assert(actual: any, reason?: string) {
+function _assert(actual: any, reason: string) {
   if (!actual && ASSERT_ENABLED === true) {
     let message = 'IONIC ASSERT: ' + reason;
     console.error(message);