fix(navigation): check existence of done transition callback (#12640)

Author: soumak77

src/components/tabs/tab.ts

@@ -8,7 +8,7 @@ import { GestureController } from '../../gestures/gesture-controller';
 import { isTrueProperty } from '../../util/util';
 import { Tab as ITab } from '../../navigation/nav-interfaces';
 import { NavControllerBase } from '../../navigation/nav-controller-base';
-import { NavOptions } from '../../navigation/nav-util';
+import { NavOptions, TransitionDoneFn } from '../../navigation/nav-util';
 import { Platform } from '../../platform/platform';
 import { TabButton } from './tab-button';
 import { Tabs } from './tabs';
@@ -304,7 +304,7 @@ export class Tab extends NavControllerBase implements ITab {
   /**
    * @hidden
    */
-  load(opts: NavOptions, done?: () => void) {
+  load(opts: NavOptions, done?: TransitionDoneFn) {
     if (this._lazyRootFromUrl || (!this._loaded && this.root)) {
       this.setElementClass('show-tab', true);
       // okay, first thing we need to do if check if the view already exists
@@ -317,7 +317,10 @@ export class Tab extends NavControllerBase implements ITab {
           if (i === numViews) {
             // this is the last view in the stack and it's the same
             // as the segment so there's no change needed
-            return done();
+            if (done) {
+              done(false, false);
+            }
+            return;
           } else {
             // it's not the exact view as the end
             // let's have this nav go back to this exact view
@@ -343,7 +346,10 @@ export class Tab extends NavControllerBase implements ITab {
       this._dom.read(() => {
         this.resize();
       });
-      return done();
+      if (done) {
+        done(false, false);
+      }
+      return;
     }
   }
 

src/navigation/deep-linker.ts

@@ -2,7 +2,7 @@ import { ComponentFactory, ComponentFactoryResolver } from '@angular/core';
 import { Location } from '@angular/common';
 
 import { App } from '../components/app/app';
-import { DIRECTION_BACK, NavLink, NavSegment, convertToViews, isNav, isTab, isTabs,  } from './nav-util';
+import { DIRECTION_BACK, NavLink, NavSegment, TransitionDoneFn, convertToViews, isNav, isTab, isTabs } from './nav-util';
 import { ModuleLoader } from '../util/module-loader';
 import { isArray, isPresent } from '../util/util';
 import { Tab, Tabs } from './nav-interfaces';
@@ -380,9 +380,9 @@ export class DeepLinker {
    *
    * @internal
    */
-  _loadViewForSegment(navContainer: NavigationContainer, segment: NavSegment, done: Function) {
+  _loadViewForSegment(navContainer: NavigationContainer, segment: NavSegment, done: TransitionDoneFn) {
     if (!segment) {
-      return done();
+      return done(false, false);
     }
 
     if (isTabs(navContainer) || (isTab(navContainer) && navContainer.parent)) {
@@ -395,7 +395,7 @@ export class DeepLinker {
         updateUrl: false,
         animate: false
       }, true);
-      return done();
+      return done(false, false);
     }
 
     const navController = <NavController> <any> navContainer;
@@ -410,7 +410,7 @@ export class DeepLinker {
         if (i === numViews) {
           // this is the last view in the stack and it's the same
           // as the segment so there's no change needed
-          return done();
+          return done(false, false);
         } else {
           // it's not the exact view as the end
           // let's have this nav go back to this exact view

src/navigation/nav-controller-base.ts

@@ -24,6 +24,7 @@ import {
   STATE_DESTROYED,
   STATE_INITIALIZED,
   STATE_NEW,
+  TransitionDoneFn,
   TransitionInstruction,
   convertToViews,
 } from './nav-util';
@@ -106,39 +107,39 @@ export class NavControllerBase extends Ion implements NavController {
     this._destroyed = false;
   }
 
-  push(page: any, params?: any, opts?: NavOptions, done?: () => void): Promise<any> {
+  push(page: any, params?: any, opts?: NavOptions, done?: TransitionDoneFn): Promise<any> {
     return this._queueTrns({
       insertStart: -1,
       insertViews: [{ page: page, params: params }],
       opts: opts,
     }, done);
   }
 
-  insert(insertIndex: number, page: any, params?: any, opts?: NavOptions, done?: () => void): Promise<any> {
+  insert(insertIndex: number, page: any, params?: any, opts?: NavOptions, done?: TransitionDoneFn): Promise<any> {
     return this._queueTrns({
       insertStart: insertIndex,
       insertViews: [{ page: page, params: params }],
       opts: opts,
     }, done);
   }
 
-  insertPages(insertIndex: number, insertPages: any[], opts?: NavOptions, done?: () => void): Promise<any> {
+  insertPages(insertIndex: number, insertPages: any[], opts?: NavOptions, done?: TransitionDoneFn): Promise<any> {
     return this._queueTrns({
       insertStart: insertIndex,
       insertViews: insertPages,
       opts: opts,
     }, done);
   }
 
-  pop(opts?: NavOptions, done?: () => void): Promise<any> {
+  pop(opts?: NavOptions, done?: TransitionDoneFn): Promise<any> {
     return this._queueTrns({
       removeStart: -1,
       removeCount: 1,
       opts: opts,
     }, done);
   }
 
-  popTo(indexOrViewCtrl: any, opts?: NavOptions, done?: () => void): Promise<any> {
+  popTo(indexOrViewCtrl: any, opts?: NavOptions, done?: TransitionDoneFn): Promise<any> {
     let config: TransitionInstruction = {
       removeStart: -1,
       removeCount: -1,
@@ -153,7 +154,7 @@ export class NavControllerBase extends Ion implements NavController {
     return this._queueTrns(config, done);
   }
 
-  popToRoot(opts?: NavOptions, done?: () => void): Promise<any> {
+  popToRoot(opts?: NavOptions, done?: TransitionDoneFn): Promise<any> {
     return this._queueTrns({
       removeStart: 1,
       removeCount: -1,
@@ -169,15 +170,15 @@ export class NavControllerBase extends Ion implements NavController {
     return Promise.all(promises);
   }
 
-  remove(startIndex: number, removeCount: number = 1, opts?: NavOptions, done?: () => void): Promise<any> {
+  remove(startIndex: number, removeCount: number = 1, opts?: NavOptions, done?: TransitionDoneFn): Promise<any> {
     return this._queueTrns({
       removeStart: startIndex,
       removeCount: removeCount,
       opts: opts,
     }, done);
   }
 
-  removeView(viewController: ViewController, opts?: NavOptions, done?: () => void): Promise<any> {
+  removeView(viewController: ViewController, opts?: NavOptions, done?: TransitionDoneFn): Promise<any> {
     return this._queueTrns({
       removeView: viewController,
       removeStart: 0,
@@ -186,12 +187,12 @@ export class NavControllerBase extends Ion implements NavController {
     }, done);
   }
 
-  setRoot(pageOrViewCtrl: any, params?: any, opts?: NavOptions, done?: () => void): Promise<any> {
+  setRoot(pageOrViewCtrl: any, params?: any, opts?: NavOptions, done?: TransitionDoneFn): Promise<any> {
     return this.setPages([{ page: pageOrViewCtrl, params: params }], opts, done);
   }
 
 
-  setPages(viewControllers: any[], opts?: NavOptions, done?: () => void): Promise<any> {
+  setPages(viewControllers: any[], opts?: NavOptions, done?: TransitionDoneFn): Promise<any> {
     if (isBlank(opts)) {
       opts = {};
     }
@@ -218,7 +219,7 @@ export class NavControllerBase extends Ion implements NavController {
   // 7. _transitionStart(): called once the transition actually starts, it initializes the Animation underneath.
   // 8. _transitionFinish(): called once the transition finishes
   // 9. _cleanup(): syncs the navigation internal state with the DOM. For example it removes the pages from the DOM or hides/show them.
-  _queueTrns(ti: TransitionInstruction, done: () => void): Promise<boolean> {
+  _queueTrns(ti: TransitionInstruction, done: TransitionDoneFn): Promise<boolean> {
     const promise = new Promise<boolean>((resolve, reject) => {
       ti.resolve = resolve;
       ti.reject = reject;

src/navigation/nav-controller.ts

@@ -1,7 +1,7 @@
 import { EventEmitter } from '@angular/core';
 
 import { Config } from '../config/config';
-import { NavOptions } from './nav-util';
+import { NavOptions, TransitionDoneFn } from './nav-util';
 import { Page } from './nav-util';
 import { ViewController } from './view-controller';
 import { NavigationContainer } from './navigation-container';
@@ -420,7 +420,7 @@ export abstract class NavController implements NavigationContainer {
    * @param {object} [opts={}] Nav options to go with this transition.
    * @returns {Promise} Returns a promise which is resolved when the transition has completed.
    */
-  abstract push(page: Page | string, params?: any, opts?: NavOptions, done?: Function): Promise<any>;
+  abstract push(page: Page | string, params?: any, opts?: NavOptions, done?: TransitionDoneFn): Promise<any>;
 
   /**
    * Inserts a component into the nav stack at the specified index. This is useful if
@@ -433,7 +433,7 @@ export abstract class NavController implements NavigationContainer {
    * @param {object} [opts={}] Nav options to go with this transition.
    * @returns {Promise} Returns a promise which is resolved when the transition has completed.
    */
-  abstract insert(insertIndex: number, page: Page | string, params?: any, opts?: NavOptions, done?: Function): Promise<any>;
+  abstract insert(insertIndex: number, page: Page | string, params?: any, opts?: NavOptions, done?: TransitionDoneFn): Promise<any>;
 
   /**
    * Inserts an array of components into the nav stack at the specified index.
@@ -445,7 +445,7 @@ export abstract class NavController implements NavigationContainer {
    * @param {object} [opts={}] Nav options to go with this transition.
    * @returns {Promise} Returns a promise which is resolved when the transition has completed.
    */
-  abstract insertPages(insertIndex: number, insertPages: Array<{page: Page | string, params?: any}>, opts?: NavOptions, done?: Function): Promise<any>;
+  abstract insertPages(insertIndex: number, insertPages: Array<{page: Page | string, params?: any}>, opts?: NavOptions, done?: TransitionDoneFn): Promise<any>;
 
   /**
    * Call to navigate back from a current component. Similar to `push()`, you
@@ -454,15 +454,15 @@ export abstract class NavController implements NavigationContainer {
    * @param {object} [opts={}] Nav options to go with this transition.
    * @returns {Promise} Returns a promise which is resolved when the transition has completed.
    */
-  abstract pop(opts?: NavOptions, done?: Function): Promise<any>;
+  abstract pop(opts?: NavOptions, done?: TransitionDoneFn): Promise<any>;
 
   /**
    * Navigate back to the root of the stack, no matter how far back that is.
    *
    * @param {object} [opts={}] Nav options to go with this transition.
    * @returns {Promise} Returns a promise which is resolved when the transition has completed.
    */
-  abstract popToRoot(opts?: NavOptions, done?: Function): Promise<any>;
+  abstract popToRoot(opts?: NavOptions, done?: TransitionDoneFn): Promise<any>;
 
   /**
    * @hidden
@@ -479,7 +479,7 @@ export abstract class NavController implements NavigationContainer {
    * @param {object} [opts={}] Nav options to go with this transition.
    * @returns {Promise} Returns a promise which is resolved when the transition has completed.
    */
-  abstract popTo(page: Page | string | ViewController, params?: any, opts?: NavOptions, done?: Function): Promise<any>;
+  abstract popTo(page: Page | string | ViewController, params?: any, opts?: NavOptions, done?: TransitionDoneFn): Promise<any>;
 
   /**
    * @hidden
@@ -497,7 +497,7 @@ export abstract class NavController implements NavigationContainer {
    * @param {object} [opts={}] Any options you want to use pass to transtion.
    * @returns {Promise} Returns a promise which is resolved when the transition has completed.
    */
-  abstract remove(startIndex: number, removeCount?: number, opts?: NavOptions, done?: Function): Promise<any>;
+  abstract remove(startIndex: number, removeCount?: number, opts?: NavOptions, done?: TransitionDoneFn): Promise<any>;
 
   /**
    * Removes the specified view controller from the nav stack.
@@ -506,7 +506,7 @@ export abstract class NavController implements NavigationContainer {
    * @param {object} [opts={}] Any options you want to use pass to transtion.
    * @returns {Promise} Returns a promise which is resolved when the transition has completed.
    */
-  abstract removeView(viewController: ViewController, opts?: NavOptions, done?: Function): Promise<any>;
+  abstract removeView(viewController: ViewController, opts?: NavOptions, done?: TransitionDoneFn): Promise<any>;
 
   /**
    * Set the root for the current navigation stack.
@@ -516,7 +516,7 @@ export abstract class NavController implements NavigationContainer {
    * @param {Function} done Callback function on done.
    * @returns {Promise} Returns a promise which is resolved when the transition has completed.
    */
-  abstract setRoot(pageOrViewCtrl: Page | string | ViewController, params?: any, opts?: NavOptions, done?: Function): Promise<any>;
+  abstract setRoot(pageOrViewCtrl: Page | string | ViewController, params?: any, opts?: NavOptions, done?: TransitionDoneFn): Promise<any>;
   abstract goToRoot(options: NavOptions): Promise<any>;
 
   /**
@@ -529,7 +529,7 @@ export abstract class NavController implements NavigationContainer {
    * @param {Object} [opts={}] Nav options to go with this transition.
    * @returns {Promise} Returns a promise which is resolved when the transition has completed.
    */
-  abstract setPages(pages: ({page: Page | string, params?: any} | ViewController)[], opts?: NavOptions, done?: Function): Promise<any>;
+  abstract setPages(pages: ({page: Page | string, params?: any} | ViewController)[], opts?: NavOptions, done?: TransitionDoneFn): Promise<any>;
 
   /**
    * @param {number} index The index of the page to get.

src/navigation/nav-util.ts

@@ -221,6 +221,10 @@ export interface TransitionRejectFn {
   (rejectReason: any, transition?: Transition): void;
 }
 
+export interface TransitionDoneFn {
+  (hasCompleted: boolean, requiresTransition: boolean, enteringName?: string, leavingName?: string, direction?: string): void;
+}
+
 export interface TransitionInstruction {
   opts: NavOptions;
   insertStart?: number;
@@ -230,7 +234,7 @@ export interface TransitionInstruction {
   removeCount?: number;
   resolve?: (hasCompleted: boolean) => void;
   reject?: (rejectReason: string) => void;
-  done?: Function;
+  done?: TransitionDoneFn;
   leavingRequiresTransition?: boolean;
   enteringRequiresTransition?: boolean;
   requiresTransition?: boolean;

src/util/mock-providers.ts

@@ -11,7 +11,7 @@ import { GestureController } from '../gestures/gesture-controller';
 import { Haptic } from '../tap-click/haptic';
 import { IonicApp } from '../components/app/app-root';
 import { Menu } from '../components/menu/menu';
-import { NavOptions } from '../navigation/nav-util';
+import { NavOptions, TransitionDoneFn } from '../navigation/nav-util';
 import { NavControllerBase } from '../navigation/nav-controller-base';
 import { OverlayPortal } from '../components/app/overlay-portal';
 import { PageTransition } from '../transitions/page-transition';
@@ -501,8 +501,9 @@ export function mockTab(parentTabs: Tabs, overrideLoad: boolean = true): Tab {
   );
 
   if (overrideLoad) {
-    tab.load = (_opts: any, cb: Function) => {
-      cb();
+    tab.load = (_opts: any, cb: TransitionDoneFn) => {
+      cb(false, false);
+      return Promise.resolve();
     };
   }