fix(vue): routing history is correctly replaced when overwriting browser history (#24670)

resolves #23873

packages/vue-router/src/locationHistory.ts

@@ -85,11 +85,25 @@ export const createLocationHistory = () => {
     locationHistory.push(routeInfo);
   }
 
-  const clearHistory = () => {
+  /**
-    locationHistory.length = 0;
+   * Wipes the location history arrays.
+   * You can optionally provide a routeInfo
+   * object which will wipe that entry
+   * and every entry that appears after it.
+   */
+  const clearHistory = (routeInfo?: RouteInfo) => {
     Object.keys(tabsHistory).forEach(key => {
       tabsHistory[key] = [];
     });
+
+    if (routeInfo) {
+      const existingRouteIndex = locationHistory.findIndex(r => r.position === routeInfo.position);
+      if (existingRouteIndex === -1) return;
+
+      locationHistory.splice(existingRouteIndex);
+    } else {
+      locationHistory.length = 0;
+    }
   }
   const getTabsHistory = (tab: string): RouteInfo[] => {
     let history;
@@ -105,17 +119,6 @@ export const createLocationHistory = () => {
 
   const size = () => locationHistory.length;
 
-  const updateByHistoryPosition = (routeInfo: RouteInfo, updateEntries: boolean) => {
-    const existingRouteIndex = locationHistory.findIndex(r => r.position === routeInfo.position);
-    if (existingRouteIndex === -1) return;
-
-    locationHistory[existingRouteIndex].pathname = routeInfo.pathname;
-
-    if (updateEntries) {
-      locationHistory[existingRouteIndex].pushedByRoute = routeInfo.pushedByRoute;
-    }
-  }
-
   /**
    * Finds and returns the location history item
    * given the state of browser's history API.
@@ -205,7 +208,6 @@ export const createLocationHistory = () => {
 
   return {
     current,
-    updateByHistoryPosition,
     size,
     last,
     previous,
@@ -214,6 +216,7 @@ export const createLocationHistory = () => {
     update,
     getFirstRouteInfoForTab,
     getCurrentRouteInfoForTab,
-    findLastLocation
+    findLastLocation,
+    clearHistory
   }
 }

packages/vue-router/src/router.ts

@@ -60,7 +60,7 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) =>
    * new pages or updating history via the forward
    * and back browser buttons.
    */
-  const initialHistoryPosition = opts.history.state.position as number;
+  let initialHistoryPosition = opts.history.state.position as number;
   let currentHistoryPosition = opts.history.state.position as number;
 
   let currentRouteInfo: RouteInfo;
@@ -159,8 +159,60 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) =>
         leavingLocationInfo = locationHistory.previous();
       } else if (incomingRouteParams.routerAction === 'pop') {
         leavingLocationInfo = locationHistory.current(initialHistoryPosition, currentHistoryPosition + 1);
+
+        /**
+         * If the Ionic Router action was "pop"
+         * and the browser history action was "replace", then
+         * it is the case that the user clicked an IonBackButton
+         * that is trying to go back to the route specified
+         * by the defaultHref property.
+         *
+         * The problem is that this route currently does
+         * not exist in the browser history, and we cannot
+         * prepend an item in the browser's history stack.
+         * To work around this, we replace the state of
+         * the current item instead.
+         * Given this scenario:
+         * /page2 --> /page3 --> (back) /page2 --> (defaultHref) /page1
+         * We would replace the state of /page2 with the state of /page1.
+         *
+         * When doing this, we are essentially re-writing past
+         * history which makes the future history no longer relevant.
+         * As a result, we clear out the location history so that users
+         * can begin pushing new routes to the stack.
+         *
+         * This pattern is aligned with how the browser handles
+         * pushing new routes after going back as well as how
+         * other stack based operations such as undo/redo work.
+         * For example, if you do tasks A, B, C, undo B and C, and
+         * then do task D, you cannot "redo" B and C because you
+         * rewrote the stack's past history.
+         *
+         * With browser history, it is a similar concept.
+         * Going /page1 --> /page2 --> /page3 and then doing
+         * router.go(-2) will bring you back to /page1.
+         * If you then push /page4, you have rewritten
+         * the past history and you can no longer go
+         * forward to /page2 or /page3.
+         */
+        if (action === 'replace') {
+          locationHistory.clearHistory();
+        }
       } else {
-        leavingLocationInfo = locationHistory.current(initialHistoryPosition, currentHistoryPosition - 1);
+        /**
+         * If the routerDirection was specified as "root", then
+         * we are replacing the initial state of location history
+         * with this incoming route. As a result, the leaving
+         * history info is stored at the same location as
+         * where the incoming history location will be stored.
+         *
+         * Otherwise, we can assume this is just another route
+         * that will be pushed onto the end of location history,
+         * so we can grab the previous item in history relative
+         * to where the history state currently is.
+         */
+        const position = (incomingRouteParams.routerDirection === 'root') ? currentHistoryPosition : currentHistoryPosition - 1;
+        leavingLocationInfo = locationHistory.current(initialHistoryPosition, position);
       }
     } else {
       leavingLocationInfo = currentRouteInfo;
@@ -285,26 +337,40 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) =>
        */
       if (historySize > historyDiff && routeInfo.tab === undefined) {
         /**
-         * When going from /a --> /a/1 --> /b, then going
+         * When navigating back through the history,
-         * back to /a, then going /a --> /a/2 --> /b, clicking
+         * if users then push a new route the future
-         * the ion-back-button should return us to /a/2, not /a/1.
+         * history stack is no longer relevant. As
-         * However, since the route entry for /b already exists,
+         * a result, we need to clear out all entries
-         * we need to update other information such as the "pushedByRoute"
+         * that appear after the current routeInfo
-         * so we know which route pushed this new route.
+         * so that we can then append the new history.
          *
-         * However, when using router.go with a stride of >1 or <-1,
+         * This does not apply when using router.go
-         * we should not update this additional information because
+         * as that is traversing through the history,
-         * we are traversing through the history, not pushing new states.
+         * not altering it.
-         * Going from /a --> /b --> /c, then doing router.go(-2), then doing
+         *
-         * router.go(2) to go from /a --> /c should not update the route
+         * Previously we had only updated the existing route
-         * listing to say that /c was pushed by /a.
+         * and then left the future history alone. That
+         * worked for some use cases but was not sufficient
+         * in other scenarios.
          */
-        const hasDeltaStride = delta !== undefined && Math.abs(delta) !== 1;
+
-        locationHistory.updateByHistoryPosition(routeInfo, !hasDeltaStride);
+        if (routeInfo.routerAction === 'push' && delta === undefined) {
+          locationHistory.clearHistory(routeInfo);
+          locationHistory.add(routeInfo);
+        }
       } else {
         locationHistory.add(routeInfo);
       }
 
+      /**
+       * If we recently reset the location history
+       * then we also need to update the initial
+       * history position.
+       */
+      if (locationHistory.size() === 1) {
+        initialHistoryPosition = routeInfo.position;
+      }
+
       currentRouteInfo = routeInfo;
     }
     incomingRouteParams = undefined;

packages/vue/test-app/src/App.vue

@@ -5,7 +5,7 @@
 </template>
 
 <script lang="ts">
-import { IonApp, IonRouterOutlet } from '@ionic/vue';
+import { IonApp, IonRouterOutlet, useIonRouter } from '@ionic/vue';
 import { defineComponent } from 'vue';
 
 export default defineComponent({
@@ -13,6 +13,9 @@ export default defineComponent({
   components: {
     IonApp,
     IonRouterOutlet
+  },
+  setup() {
+    (window as any).debugIonRouter = useIonRouter();
   }
 });
 </script>

packages/vue/test-app/tests/e2e/specs/routing.js

@@ -400,6 +400,119 @@ describe('Routing', () => {
     cy.ionPageVisible('home');
     cy.ionPageHidden('inputs');
   })
+
+  // Verifies fix for https://github.com/ionic-team/ionic-framework/issues/23873
+  it('should correctly show pages after going back to defaultHref page', () => {
+    cy.visit('http://localhost:8080/default-href');
+
+    cy.routerPush('/routing');
+    cy.ionPageVisible('routing');
+    cy.ionPageHidden('defaulthref');
+
+    cy.ionBackClick('routing');
+    cy.ionPageDoesNotExist('routing');
+    cy.ionPageVisible('defaulthref');
+
+    cy.ionBackClick('defaulthref');
+    cy.ionPageDoesNotExist('defaulthref');
+    cy.ionPageVisible('home');
+
+    cy.routerPush('/default-href');
+    cy.ionPageVisible('defaulthref');
+    cy.ionPageHidden('home');
+
+    cy.routerPush('/routing');
+    cy.ionPageVisible('routing');
+    cy.ionPageHidden('defaulthref');
+
+    cy.ionBackClick('routing');
+    cy.ionPageDoesNotExist('routing');
+    cy.ionPageVisible('defaulthref');
+
+    cy.ionBackClick('defaulthref');
+    cy.ionPageDoesNotExist('defaulthref');
+    cy.ionPageVisible('home');
+  })
+
+  it('should correctly update location history after rewriting past state', () => {
+    cy.visit('http://localhost:8080');
+
+    cy.routerPush('/routing');
+    cy.ionPageVisible('routing');
+    cy.ionPageHidden('home');
+
+    cy.routerPush('/inputs');
+    cy.ionPageVisible('inputs');
+    cy.ionPageHidden('routing');
+
+    cy.ionBackClick('inputs');
+    cy.ionPageVisible('routing');
+    cy.ionPageDoesNotExist('inputs');
+
+    cy.ionBackClick('routing');
+    cy.ionPageVisible('home');
+    cy.ionPageDoesNotExist('routing');
+
+    cy.routerPush('default-href');
+    cy.ionPageVisible('defaulthref');
+    cy.ionPageHidden('home');
+
+    cy.routerPush('/routing');
+    cy.ionPageVisible('routing');
+    cy.ionPageHidden('defaulthref');
+
+    cy.routerPush('/inputs');
+    cy.ionPageVisible('inputs');
+    cy.ionPageHidden('routing');
+
+    cy.ionBackClick('inputs');
+    cy.ionPageVisible('routing');
+    cy.ionPageDoesNotExist('inputs');
+
+    cy.ionBackClick('routing');
+    cy.ionPageVisible('defaulthref');
+    cy.ionPageDoesNotExist('routing');
+  })
+
+  it('should correctly update location history after setting root state', () => {
+    cy.visit('http://localhost:8080');
+
+    cy.routerPush('/routing');
+    cy.ionPageVisible('routing');
+    cy.ionPageHidden('home');
+
+    cy.routerPush('/routing/1');
+    cy.ionPageVisible('routingparameter-1');
+    cy.ionPageHidden('routing');
+
+    cy.ionBackClick('routingparameter-1');
+    cy.ionPageVisible('routing');
+    cy.ionPageDoesNotExist('routingparameter-1');
+
+    cy.ionBackClick('routing');
+    cy.ionPageVisible('home');
+    cy.ionPageDoesNotExist('routing');
+
+    cy.ionRouterNavigate('/inputs', 'root')
+    cy.ionPageVisible('inputs');
+    cy.ionPageDoesNotExist('home');
+
+    cy.routerPush('/routing');
+    cy.ionPageVisible('routing');
+    cy.ionPageHidden('inputs');
+
+    cy.routerPush('/routing/1');
+    cy.ionPageVisible('routingparameter-1');
+    cy.ionPageHidden('routing');
+
+    cy.ionBackClick('routingparameter-1');
+    cy.ionPageVisible('routing');
+    cy.ionPageDoesNotExist('routingparameter-1');
+
+    cy.ionBackClick('routing');
+    cy.ionPageVisible('inputs');
+    cy.ionPageDoesNotExist('routing');
+  })
 });
 
 describe('Routing - Swipe to Go Back', () => {

packages/vue/test-app/tests/e2e/support/commands.js

@@ -82,6 +82,12 @@ Cypress.Commands.add('routerGo', (n) => {
   });
 });
 
+Cypress.Commands.add('ionRouterNavigate', (...args) => {
+  cy.window().then(win => {
+    win.debugIonRouter.navigate(...args);
+  });
+});
+
 Cypress.Commands.add('ionBackButtonHidden', (pageId) => {
   cy.get(`div.ion-page[data-pageid=${pageId}]`)
     .should('be.visible', true)

packages/vue/test-app/tests/unit/routing.spec.ts

@@ -312,6 +312,124 @@ describe('Routing', () => {
     expect(cmpAgain[0].props()).toEqual({ title: 'xyz' });
   });
 
+  /**
+   * TODO(FW-653) Either Ionic Vue or Vue Test Utils
+   * is leaking routing information between tests. Moving these tests
+   * to the end cause them to fail. Need to figure out what the root cause is.
+   */
+  // Verifies fix for https://github.com/ionic-team/ionic-framework/issues/24109
+  it('canGoBack() should return the correct value', async () => {
+    const Page = {
+      components: { IonPage },
+      template: `<ion-page></ion-page>`
+    }
+    const Page2 = {
+      components: { IonPage },
+      template: `<ion-page></ion-page>`
+    }
+    const AppWithInject = {
+      components: { IonApp, IonRouterOutlet },
+      template: '<ion-app><ion-router-outlet /></ion-app>',
+      setup() {
+        const ionRouter = useIonRouter();
+        return { ionRouter }
+      }
+    }
+
+    const router = createRouter({
+      history: createWebHistory(process.env.BASE_URL),
+      routes: [
+        { path: '/', component: Page }
+        { path: '/page2', component: Page2 }
+      ]
+    });
+
+    router.push('/');
+    await router.isReady();
+    const wrapper = mount(AppWithInject, {
+      global: {
+        plugins: [router, IonicVue]
+      }
+    });
+
+    const ionRouter = wrapper.vm.ionRouter;
+    expect(ionRouter.canGoBack()).toEqual(false);
+
+    router.push('/page2');
+    await waitForRouter();
+
+    expect(ionRouter.canGoBack()).toEqual(true);
+
+    router.back();
+    await waitForRouter();
+
+    expect(ionRouter.canGoBack()).toEqual(false);
+  });
+
+  // Verifies fix for https://github.com/ionic-team/ionic-framework/issues/24109
+  it('canGoBack() should return the correct value when using router.go', async () => {
+    const Page = {
+      components: { IonPage },
+      template: `<ion-page></ion-page>`
+    }
+    const Page2 = {
+      components: { IonPage },
+      template: `<ion-page></ion-page>`
+    }
+    const Page3 = {
+      components: { IonPage },
+      template: `<ion-page></ion-page>`
+    }
+    const AppWithInject = {
+      components: { IonApp, IonRouterOutlet },
+      template: '<ion-app><ion-router-outlet /></ion-app>',
+      setup() {
+        const ionRouter = useIonRouter();
+        return { ionRouter }
+      }
+    }
+
+    const router = createRouter({
+      history: createWebHistory(process.env.BASE_URL),
+      routes: [
+        { path: '/', component: Page }
+        { path: '/page2', component: Page2 },
+        { path: '/page3', component: Page3 },
+      ]
+    });
+
+    router.push('/');
+    await router.isReady();
+    const wrapper = mount(AppWithInject, {
+      global: {
+        plugins: [router, IonicVue]
+      }
+    });
+
+    const ionRouter = wrapper.vm.ionRouter;
+    expect(ionRouter.canGoBack()).toEqual(false);
+
+    router.push('/page2');
+    await waitForRouter();
+
+    expect(ionRouter.canGoBack()).toEqual(true);
+
+    router.push('/page3');
+    await waitForRouter();
+
+    expect(ionRouter.canGoBack()).toEqual(true);
+
+    router.go(-2);
+    await waitForRouter();
+
+    expect(ionRouter.canGoBack()).toEqual(false);
+
+    router.go(2);
+    await waitForRouter();
+
+    expect(ionRouter.canGoBack()).toEqual(true);
+  });
+
   // Verifies fix for https://github.com/ionic-team/ionic-framework/issues/23043
   it('should call the props function again when params change', async () => {
     const Page1 = {
@@ -549,117 +667,4 @@ describe('Routing', () => {
     expect(wrapper.findComponent(Page2).exists()).toBe(false);
     expect(wrapper.findComponent(Page3).exists()).toBe(false);
   });
-
-  // Verifies fix for https://github.com/ionic-team/ionic-framework/issues/24109
-  it('canGoBack() should return the correct value', async () => {
-    const Page = {
-      components: { IonPage },
-      template: `<ion-page></ion-page>`
-    }
-    const Page2 = {
-      components: { IonPage },
-      template: `<ion-page></ion-page>`
-    }
-    const AppWithInject = {
-      components: { IonApp, IonRouterOutlet },
-      template: '<ion-app><ion-router-outlet /></ion-app>',
-      setup() {
-        const ionRouter = useIonRouter();
-        return { ionRouter }
-      }
-    }
-
-    const router = createRouter({
-      history: createWebHistory(process.env.BASE_URL),
-      routes: [
-        { path: '/', component: Page }
-        { path: '/page2', component: Page2 }
-      ]
-    });
-
-    router.push('/');
-    await router.isReady();
-    const wrapper = mount(AppWithInject, {
-      global: {
-        plugins: [router, IonicVue]
-      }
-    });
-
-    const ionRouter = wrapper.vm.ionRouter;
-    expect(ionRouter.canGoBack()).toEqual(false);
-
-    router.push('/page2');
-    await waitForRouter();
-
-    expect(ionRouter.canGoBack()).toEqual(true);
-
-    router.back();
-    await waitForRouter();
-
-    expect(ionRouter.canGoBack()).toEqual(false);
-  });
-
-  // Verifies fix for https://github.com/ionic-team/ionic-framework/issues/24109
-  it('canGoBack() should return the correct value when using router.go', async () => {
-    const Page = {
-      components: { IonPage },
-      template: `<ion-page></ion-page>`
-    }
-    const Page2 = {
-      components: { IonPage },
-      template: `<ion-page></ion-page>`
-    }
-    const Page3 = {
-      components: { IonPage },
-      template: `<ion-page></ion-page>`
-    }
-    const AppWithInject = {
-      components: { IonApp, IonRouterOutlet },
-      template: '<ion-app><ion-router-outlet /></ion-app>',
-      setup() {
-        const ionRouter = useIonRouter();
-        return { ionRouter }
-      }
-    }
-
-    const router = createRouter({
-      history: createWebHistory(process.env.BASE_URL),
-      routes: [
-        { path: '/', component: Page }
-        { path: '/page2', component: Page2 },
-        { path: '/page3', component: Page3 },
-      ]
-    });
-
-    router.push('/');
-    await router.isReady();
-    const wrapper = mount(AppWithInject, {
-      global: {
-        plugins: [router, IonicVue]
-      }
-    });
-
-    const ionRouter = wrapper.vm.ionRouter;
-    expect(ionRouter.canGoBack()).toEqual(false);
-
-    router.push('/page2');
-    await waitForRouter();
-
-    expect(ionRouter.canGoBack()).toEqual(true);
-
-    router.push('/page3');
-    await waitForRouter();
-
-    expect(ionRouter.canGoBack()).toEqual(true);
-
-    router.go(-2);
-    await waitForRouter();
-
-    expect(ionRouter.canGoBack()).toEqual(false);
-
-    router.go(2);
-    await waitForRouter();
-
-    expect(ionRouter.canGoBack()).toEqual(true);
-  });
 });