fix(vue): canGoBack method now works correctly (#24188)

resolves #24109
ionic-team · Nov 9, 2021 · 7c43589 · 7c43589
1 parent 642255e
commit 7c43589
Show file tree

Hide file tree

Showing 5 changed files with 131 additions and 6 deletions.
diff --git a/packages/vue-router/__tests__/locationHistory.spec.ts b/packages/vue-router/__tests__/locationHistory.spec.ts
@@ -84,7 +84,7 @@ describe('Location History', () => {
     locationHistory.add({ pathname: '/home' });
     locationHistory.add({ pathname: '/login' });
 
-    expect(locationHistory.canGoBack(1)).toEqual(true);
-    expect(locationHistory.canGoBack(2)).toEqual(false);
+    expect(locationHistory.canGoBack(1, 0, 1)).toEqual(true);
+    expect(locationHistory.canGoBack(2, 0, 1)).toEqual(false);
   });
 });
diff --git a/packages/vue-router/src/locationHistory.ts b/packages/vue-router/src/locationHistory.ts
@@ -133,7 +133,18 @@ export const createLocationHistory = () => {
   }
   const previous = () => locationHistory[locationHistory.length - 2] || last();
   const last = () => locationHistory[locationHistory.length - 1];
-  const canGoBack = (deep: number = 1) => locationHistory.length > deep;
+
+  /**
+   * With the introduction of router.go support, we no longer remove
+   * items from locationHistory as they may be needed again in the future.
+   * As a result, we need to look at the current position in location history
+   * to see if users can navigate back n pages. Previously we were checking
+   * the length of locationHistory, but that only worked since we were pruning
+   * the array.
+   */
+  const canGoBack = (deep: number = 1, initialHistory: number, currentHistory: number) => {
+    return currentHistory - deep >= initialHistory;
+  }
 
   const getFirstRouteInfoForTab = (tab: string): RouteInfo | undefined => {
     const tabHistory = getTabsHistory(tab);

diff --git a/packages/vue-router/src/router.ts b/packages/vue-router/src/router.ts
@@ -305,7 +305,7 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) =>
 
   const getCurrentRouteInfo = () => currentRouteInfo;
 
-  const canGoBack = (deep: number = 1) => locationHistory.canGoBack(deep);
+  const canGoBack = (deep: number = 1) => locationHistory.canGoBack(deep, initialHistoryPosition, currentHistoryPosition);
 
   const navigate = (navigationOptions: ExternalNavigationOptions) => {
     const { routerAnimation, routerDirection, routerLink } = navigationOptions;

diff --git a/packages/vue/test-app/src/App.vue b/packages/vue/test-app/src/App.vue
@@ -15,4 +15,4 @@ export default defineComponent({
     IonRouterOutlet
   }
 });
-</script>
+</script>
diff --git a/packages/vue/test-app/tests/unit/routing.spec.ts b/packages/vue/test-app/tests/unit/routing.spec.ts
@@ -8,7 +8,8 @@ import {
   IonTabs,
   IonTabBar,
   IonTabButton,
-  IonLabel
+  IonLabel,
+  useIonRouter
 } from '@ionic/vue';
 import { onBeforeRouteLeave } from 'vue-router';
 import { waitForRouter } from './utils';
@@ -541,4 +542,117 @@ 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);
+  });
 });