fix(vue): using router.go now shows correct view

packages/vue-router/__tests__/locationHistory.spec.ts

@@ -15,15 +15,15 @@ describe('Location History', () => {
     locationHistory.add({ pathname: '/home' });
     locationHistory.add({ pathname: '/login', routerAction: 'replace' });
 
-    const current = locationHistory.current();
+    const current = locationHistory.last();
     expect(current.pathname).toEqual('/login');
   });
 
   it('should correctly pop an item from location history', () => {
     locationHistory.add({ pathname: '/home' });
     locationHistory.add({ pathname: '/login', routerAction: 'pop' });
 
-    const current = locationHistory.current();
+    const current = locationHistory.last();
     expect(current.pathname).toEqual('/login');
     expect(locationHistory.canGoBack(1)).toEqual(false);
   });
@@ -33,7 +33,7 @@ describe('Location History', () => {
     locationHistory.add({ pathname: '/login' });
     locationHistory.add({ pathname: '/logout', routerDirection: 'root' });
 
-    const current = locationHistory.current();
+    const current = locationHistory.last();
     expect(current.pathname).toEqual('/logout');
     expect(locationHistory.canGoBack(1)).toEqual(false);
   });
@@ -42,12 +42,12 @@ describe('Location History', () => {
     locationHistory.add({ id: '1', pathname: '/tabs/tab1', tab: 'tab1' });
     locationHistory.add({ id: '2', pathname: '/tabs/tab2' });
 
-    const current = { ...locationHistory.current() };
+    const current = { ...locationHistory.last() };
     current.tab = 'tab2';
 
     locationHistory.update(current);
 
-    const getCurrentAgain = locationHistory.current();
+    const getCurrentAgain = locationHistory.last();
     expect(getCurrentAgain.tab).toEqual('tab2');
   });
 
@@ -73,7 +73,7 @@ describe('Location History', () => {
     locationHistory.add({ pathname: '/home' });
     locationHistory.add({ pathname: '/login' });
 
-    const current = locationHistory.current();
+    const current = locationHistory.last();
     expect(current.pathname).toEqual('/login');
 
     const previous = locationHistory.previous();

packages/vue-router/__tests__/viewStacks.spec.ts

@@ -102,10 +102,39 @@ describe('View Stacks', () => {
 
     const viewItemsAgain = viewStacks.getViewStack(2);
     expect(viewItemsAgain).toEqual(undefined);
-  })
+  });
+
+  it('should unmount orphaned views', () => {
+    const itemA = createRegisteredViewItem(viewStacks, 1, '/home/1', true);
+    const itemB = createRegisteredViewItem(viewStacks, 1, '/home/2', true);
+    const itemC = createRegisteredViewItem(viewStacks, 1, '/home/3', true);
+    const itemD = createRegisteredViewItem(viewStacks, 1, '/home/4', true);
+
+    viewStacks.unmountLeavingViews(1, itemA, itemD);
+
+    expect(itemB.mount).toEqual(false);
+    expect(itemB.ionPageElement).toEqual(undefined);
+    expect(itemB.ionRoute).toEqual(false);
+
+    expect(itemC.mount).toEqual(false);
+    expect(itemC.ionPageElement).toEqual(undefined);
+    expect(itemC.ionRoute).toEqual(false);
+  });
+
+  it('should remount intermediary views', () => {
+    const itemA = createRegisteredViewItem(viewStacks);
+    const itemB = createRegisteredViewItem(viewStacks);
+    const itemC = createRegisteredViewItem(viewStacks);
+    const itemD = createRegisteredViewItem(viewStacks);
+
+    viewStacks.mountIntermediaryViews(1, itemD, itemA);
+
+    expect(itemB.mount).toEqual(true);
+    expect(itemC.mount).toEqual(true);
+  });
 })
 
-const createRegisteredViewItem = (viewStacks, outletId = '1', route = `/home/${counter++}`) => {
+const createRegisteredViewItem = (viewStacks, outletId = '1', route = `/home/${counter++}`, mount = false) => {
   const item = viewStacks.createViewItem(
     outletId,
     () => {},
@@ -115,10 +144,15 @@ const createRegisteredViewItem = (viewStacks, outletId = '1', route = `/home/${c
 
   viewStacks.add(item);
 
-  const ionPage = document.createElement('div');
-  ionPage.classList.add('ion-page');
+  if (mount) {
+    const ionPage = document.createElement('div');
+    ionPage.classList.add('ion-page');
+
+    viewStacks.registerIonPage(item, ionPage);
 
-  viewStacks.registerIonPage(item, ionPage);
+    item.mount = true;
+    item.ionRoute = true;
+  }
 
   return item;
 }

packages/vue-router/src/locationHistory.ts

@@ -102,8 +102,33 @@ export const createLocationHistory = () => {
 
     return history;
   }
-  const previous = () => locationHistory[locationHistory.length - 2] || current();
-  const current = () => locationHistory[locationHistory.length - 1];
+
+  const size = () => locationHistory.length;
+
+  const updateByHistoryPosition = (routeInfo: RouteInfo) => {
+    const existingRouteIndex = locationHistory.findIndex(r => r.position === routeInfo.position);
+    if (existingRouteIndex === -1) return;
+
+    locationHistory[existingRouteIndex].pathname = routeInfo.pathname;
+  }
+
+  /**
+   * Finds and returns the location history item
+   * given the state of browser's history API.
+   * This is useful when jumping around in browser
+   * history using router.go.
+   */
+  const current = (initialHistory: number, currentHistory: number) => {
+    /**
+     * initialHistory does not always start at 0 if users navigated
+     * to app from another website, so doing this math lets us
+     * find the correct index in our locationHistory array.
+     */
+    const index = currentHistory - initialHistory;
+    return locationHistory[index] || last();
+  }
+  const previous = () => locationHistory[locationHistory.length - 2] || last();
+  const last = () => locationHistory[locationHistory.length - 1];
   const canGoBack = (deep: number = 1) => locationHistory.length > deep;
 
   const getFirstRouteInfoForTab = (tab: string): RouteInfo | undefined => {
@@ -122,23 +147,41 @@ export const createLocationHistory = () => {
     return undefined;
   }
 
-  const findLastLocation = (routeInfo: RouteInfo): RouteInfo | undefined => {
+  /**
+   * Finds and returns the previous view based upon
+   * what originally pushed it (pushedByRoute).
+   * When `delta` < -1 then we should just index into
+   * to array because the previous view that we want is not
+   * necessarily the view that pushed our current view.
+   * Additionally, when jumping around in history, we
+   * do not modify the locationHistory stack so we would
+   * not update pushedByRoute anyways.
+   */
+  const findLastLocation = (routeInfo: RouteInfo, delta: number = -1): RouteInfo | undefined => {
     const routeInfos = getTabsHistory(routeInfo.tab);
     if (routeInfos) {
-      for (let i = routeInfos.length - 2; i >= 0; i--) {
-        const ri = routeInfos[i];
-        if (ri) {
-          if (ri.pathname === routeInfo.pushedByRoute) {
-            return ri;
+      if (delta < -1) {
+        return routeInfos[routeInfos.length - 1 + delta];
+      } else {
+        for (let i = routeInfos.length - 2; i >= 0; i--) {
+          const ri = routeInfos[i];
+          if (ri) {
+            if (ri.pathname === routeInfo.pushedByRoute) {
+              return ri;
+            }
           }
         }
       }
     }
-    for (let i = locationHistory.length - 2; i >= 0; i--) {
-      const ri = locationHistory[i];
-      if (ri) {
-        if (ri.pathname === routeInfo.pushedByRoute) {
-          return ri;
+    if (delta < -1) {
+      return locationHistory[locationHistory.length - 1 + delta];
+    } else {
+      for (let i = locationHistory.length - 2; i >= 0; i--) {
+        const ri = locationHistory[i];
+        if (ri) {
+          if (ri.pathname === routeInfo.pushedByRoute) {
+            return ri;
+          }
         }
       }
     }
@@ -147,6 +190,9 @@ export const createLocationHistory = () => {
 
   return {
     current,
+    updateByHistoryPosition,
+    size,
+    last,
     previous,
     add,
     canGoBack,

packages/vue-router/src/router.ts

@@ -18,7 +18,7 @@ import {
 import { AnimationBuilder } from '@ionic/vue';
 
 export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) => {
-  let currentNavigationInfo: NavigationInformation = { direction: undefined, action: undefined };
+  let currentNavigationInfo: NavigationInformation = { direction: undefined, action: undefined, delta: undefined };
 
   /**
    * Ionic Vue should only react to navigation
@@ -32,7 +32,7 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) =>
   router.afterEach((to: RouteLocationNormalized, _: RouteLocationNormalized, failure?: NavigationFailure) => {
     if (failure) return;
 
-    const { direction, action } = currentNavigationInfo;
+    const { direction, action, delta } = currentNavigationInfo;
 
     /**
      * When calling router.replace, we are not informed
@@ -42,13 +42,26 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) =>
      * We need to use opts.history rather than window.history
      * because window.history will be undefined when using SSR.
      */
+
+    currentHistoryPosition = opts.history.state.position as number;
+
     const replaceAction = opts.history.state.replaced ? 'replace' : undefined;
-    handleHistoryChange(to, action || replaceAction, direction);
+    handleHistoryChange(to, action || replaceAction, direction, delta);
 
-    currentNavigationInfo = { direction: undefined, action: undefined };
+    currentNavigationInfo = { direction: undefined, action: undefined, delta: undefined };
   });
 
   const locationHistory = createLocationHistory();
+
+  /**
+   * Keeping track of the history position
+   * allows us to determine if a user is pushing
+   * new pages or updating history via the forward
+   * and back browser buttons.
+   */
+  const initialHistoryPosition = opts.history.state.position as number;
+  let currentHistoryPosition = opts.history.state.position as number;
+
   let currentRouteInfo: RouteInfo;
   let incomingRouteParams: RouteParams;
   let currentTab: string | undefined;
@@ -78,14 +91,21 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) =>
      * router.beforeEach
      */
     currentNavigationInfo = {
-      action: info.type,
+      delta: info.delta,
+
+      /**
+       * Both the browser forward and backward actions
+       * are considered "pop" actions, but when going forward
+       * we want to make sure the forward animation is used.
+       */
+      action: (info.type === 'pop' && info.delta >= 1) ? 'push' : info.type,
       direction: info.direction === '' ? 'forward' : info.direction
     };
   });
 
   const handleNavigateBack = (defaultHref?: string, routerAnimation?: AnimationBuilder) => {
     // todo grab default back button href from config
-    const routeInfo = locationHistory.current();
+    const routeInfo = locationHistory.current(initialHistoryPosition, currentHistoryPosition);
     if (routeInfo && routeInfo.pushedByRoute) {
       const prevInfo = locationHistory.findLastLocation(routeInfo);
       if (prevInfo) {
@@ -131,16 +151,23 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) =>
   }
 
   // TODO RouteLocationNormalized
-  const handleHistoryChange = (location: any, action?: RouteAction, direction?: RouteDirection) => {
+  const handleHistoryChange = (
+    location: any,
+    action?: RouteAction,
+    direction?: RouteDirection,
+    delta?: number
+  ) => {
     let leavingLocationInfo: RouteInfo;
     if (incomingRouteParams) {
       if (incomingRouteParams.routerAction === 'replace') {
         leavingLocationInfo = locationHistory.previous();
+      } else if (incomingRouteParams.routerAction === 'pop') {
+        leavingLocationInfo = locationHistory.current(initialHistoryPosition, currentHistoryPosition + 1);
       } else {
-        leavingLocationInfo = locationHistory.current();
+        leavingLocationInfo = locationHistory.current(initialHistoryPosition, currentHistoryPosition - 1);
       }
     } else {
-      leavingLocationInfo = locationHistory.current();
+      leavingLocationInfo = currentRouteInfo;
     }
 
     if (!leavingLocationInfo) {
@@ -160,9 +187,10 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) =>
             tab: currentTab
           }
         } else if (action === 'pop') {
-          const routeInfo = locationHistory.current();
+          const routeInfo = locationHistory.current(initialHistoryPosition, currentHistoryPosition - delta);
+
           if (routeInfo && routeInfo.pushedByRoute) {
-            const prevRouteInfo = locationHistory.findLastLocation(routeInfo);
+            const prevRouteInfo = locationHistory.findLastLocation(routeInfo, delta);
             incomingRouteParams = {
               ...prevRouteInfo,
               routerAction: 'pop',
@@ -191,7 +219,6 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) =>
           ...incomingRouteParams,
           lastPathname: leavingLocationInfo.pathname
         }
-        locationHistory.add(routeInfo);
 
       } else {
         const isPushed = incomingRouteParams.routerAction === 'push' && incomingRouteParams.routerDirection === 'forward';
@@ -215,7 +242,7 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) =>
           const lastRoute = locationHistory.getCurrentRouteInfoForTab(routeInfo.tab);
           routeInfo.pushedByRoute = lastRoute?.pushedByRoute;
         } else if (routeInfo.routerAction === 'replace') {
-          const currentRouteInfo = locationHistory.current();
+          const currentRouteInfo = locationHistory.last();
 
           /**
            * If going from /home to /child, then replacing from
@@ -232,8 +259,27 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) =>
           routeInfo.prevRouteLastPathname = currentRouteInfo?.lastPathname;
         }
 
+      }
+
+      routeInfo.position = currentHistoryPosition;
+      const historySize = locationHistory.size();
+      const historyDiff = currentHistoryPosition - initialHistoryPosition;
+
+      /**
+       * If the size of location history is greater
+       * than the difference between the current history
+       * position and the initial history position
+       * then we are guaranteed to already have a history
+       * item for this route. In other words, a user
+       * is navigating within the history without pushing
+       * new items within the stack.
+       */
+      if (historySize > historyDiff && routeInfo.tab === undefined) {
+        locationHistory.updateByHistoryPosition(routeInfo);
+      } else {
         locationHistory.add(routeInfo);
       }
+
       currentRouteInfo = routeInfo;
     }
     incomingRouteParams = undefined;
@@ -301,7 +347,7 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) =>
   const handleSetCurrentTab = (tab: string) => {
     currentTab = tab;
 
-    const ri = { ...locationHistory.current() };
+    const ri = { ...locationHistory.last() };
     if (ri.tab !== tab) {
       ri.tab = tab;
       locationHistory.update(ri);
@@ -313,7 +359,12 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) =>
     historyChangeListeners.push(cb);
   }
 
+  const getLeavingRouteInfo = () => {
+    return locationHistory.current(initialHistoryPosition, currentHistoryPosition);
+  }
+
   return {
+    getLeavingRouteInfo,
     handleNavigateBack,
     handleSetCurrentTab,
     getCurrentRouteInfo,