From 24659a527abe0c70df7e8ae6da3dcb4017bf500c Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Thu, 14 Oct 2021 11:28:36 -0400 Subject: [PATCH] fix(vue): mount correct views when navigating (#24056) resolves #23914 --- .../vue-router/__tests__/viewStacks.spec.ts | 4 +- packages/vue-router/src/router.ts | 1 + packages/vue-router/src/types.ts | 1 + packages/vue-router/src/viewStacks.ts | 52 ++++----- .../vue/src/components/IonRouterOutlet.ts | 6 +- .../vue/test-app/tests/unit/routing.spec.ts | 100 ++++++++++++++++++ 6 files changed, 125 insertions(+), 39 deletions(-) diff --git a/packages/vue-router/__tests__/viewStacks.spec.ts b/packages/vue-router/__tests__/viewStacks.spec.ts index a31a466565..2897f8c9a1 100644 --- a/packages/vue-router/__tests__/viewStacks.spec.ts +++ b/packages/vue-router/__tests__/viewStacks.spec.ts @@ -110,7 +110,7 @@ describe('View Stacks', () => { const itemC = createRegisteredViewItem(viewStacks, 1, '/home/3', true); const itemD = createRegisteredViewItem(viewStacks, 1, '/home/4', true); - viewStacks.unmountLeavingViews(1, itemA, itemD); + viewStacks.unmountLeavingViews(1, itemA, -3); expect(itemB.mount).toEqual(false); expect(itemB.ionPageElement).toEqual(undefined); @@ -127,7 +127,7 @@ describe('View Stacks', () => { const itemC = createRegisteredViewItem(viewStacks); const itemD = createRegisteredViewItem(viewStacks); - viewStacks.mountIntermediaryViews(1, itemD, itemA); + viewStacks.mountIntermediaryViews(1, itemA, 3); expect(itemB.mount).toEqual(true); expect(itemC.mount).toEqual(true); diff --git a/packages/vue-router/src/router.ts b/packages/vue-router/src/router.ts index 5d81b91756..9ca00d9e0f 100644 --- a/packages/vue-router/src/router.ts +++ b/packages/vue-router/src/router.ts @@ -262,6 +262,7 @@ export const createIonRouter = (opts: IonicVueRouterOptions, router: Router) => } routeInfo.position = currentHistoryPosition; + routeInfo.delta = delta; const historySize = locationHistory.size(); const historyDiff = currentHistoryPosition - initialHistoryPosition; diff --git a/packages/vue-router/src/types.ts b/packages/vue-router/src/types.ts index 9c4774ca1f..32eced4939 100644 --- a/packages/vue-router/src/types.ts +++ b/packages/vue-router/src/types.ts @@ -27,6 +27,7 @@ export interface RouteInfo { pushedByRoute?: string; tab?: string; position?: number; + delta?: number; } export interface RouteParams { diff --git a/packages/vue-router/src/viewStacks.ts b/packages/vue-router/src/viewStacks.ts index 7fd5c331da..74043600e3 100644 --- a/packages/vue-router/src/viewStacks.ts +++ b/packages/vue-router/src/viewStacks.ts @@ -164,34 +164,6 @@ export const createViewStacks = (router: Router) => { return []; } - /** - * Given a view stack and entering/leaving views, - * determine the position of each item in the stack. - * This is useful for removing/adding views in between - * the view items when navigating using router.go. - * Use this method instead of doing an `Array.findIndex` - * for both view items. - */ - const findViewIndex = (viewStack: ViewItem[], enteringViewItem: ViewItem, leavingViewItem: ViewItem) => { - let enteringIndex = -1; - let leavingIndex = -1; - - for (let i = 0; i <= viewStack.length - 1; i++) { - const viewItem = viewStack[i]; - if (viewItem === enteringViewItem) { - enteringIndex = i; - } else if (viewItem === leavingViewItem) { - leavingIndex = i; - } - - if (enteringIndex > -1 && leavingIndex > -1) { - break; - } - } - - return { enteringIndex, leavingIndex }; - } - /** * When navigating backwards, we need to clean up and * leaving pages so that they are re-created if @@ -199,13 +171,13 @@ export const createViewStacks = (router: Router) => { * important when using router.go and stepping back * multiple pages at a time. */ - const unmountLeavingViews = (outletId: number, enteringViewItem: ViewItem, leavingViewItem: ViewItem) => { + const unmountLeavingViews = (outletId: number, viewItem: ViewItem, delta: number = 1) => { const viewStack = viewStacks[outletId]; if (!viewStack) return; - const { enteringIndex: startIndex, leavingIndex: endIndex } = findViewIndex(viewStack, enteringViewItem, leavingViewItem); + const startIndex = viewStack.findIndex(v => v === viewItem); - for (let i = startIndex + 1; i < endIndex; i++) { + for (let i = startIndex + 1; i < startIndex - delta; i++) { const viewItem = viewStack[i]; viewItem.mount = false; viewItem.ionPageElement = undefined; @@ -219,14 +191,26 @@ export const createViewStacks = (router: Router) => { * developers to step forward over multiple views. * The intermediary views need to be remounted so that * swipe to go back works properly. + * We need to account for the delta value here too because + * we do not want to remount an unrelated view. + * Example: + * /home --> /page2 --> router.back() --> /page3 + * Going to /page3 would remount /page2 since we do + * not prune /page2 from the stack. However, /page2 + * needs to remain in the stack. + * Example: + * /home --> /page2 --> /page3 --> router.go(-2) --> router.go(2) + * We would end up on /page3, but users need to be able to swipe + * to go back to /page2 and /home, so we need both pages mounted + * in the DOM. */ - const mountIntermediaryViews = (outletId: number, enteringViewItem: ViewItem, leavingViewItem: ViewItem) => { + const mountIntermediaryViews = (outletId: number, viewItem: ViewItem, delta: number = 1) => { const viewStack = viewStacks[outletId]; if (!viewStack) return; - const { enteringIndex: endIndex, leavingIndex: startIndex } = findViewIndex(viewStack, enteringViewItem, leavingViewItem); + const startIndex = viewStack.findIndex(v => v === viewItem); - for (let i = startIndex + 1; i < endIndex; i++) { + for (let i = startIndex + 1; i < startIndex + delta; i++) { viewStack[i].mount = true; } } diff --git a/packages/vue/src/components/IonRouterOutlet.ts b/packages/vue/src/components/IonRouterOutlet.ts index ffbad4d255..e5053f7aa3 100644 --- a/packages/vue/src/components/IonRouterOutlet.ts +++ b/packages/vue/src/components/IonRouterOutlet.ts @@ -214,7 +214,7 @@ export const IonRouterOutlet = defineComponent({ const handlePageTransition = async () => { const routeInfo = ionRouter.getCurrentRouteInfo(); - const { routerDirection, routerAction, routerAnimation, prevRouteLastPathname } = routeInfo; + const { routerDirection, routerAction, routerAnimation, prevRouteLastPathname, delta } = routeInfo; const enteringViewItem = viewStacks.findViewItemByRouteInfo(routeInfo, id, usingDeprecatedRouteSetup); let leavingViewItem = viewStacks.findLeavingViewItemByRouteInfo(routeInfo, id, true, usingDeprecatedRouteSetup); @@ -286,10 +286,10 @@ See https://ionicframework.com/docs/vue/navigation#ionpage for more information. leavingViewItem.mount = false; leavingViewItem.ionPageElement = undefined; leavingViewItem.ionRoute = false; - viewStacks.unmountLeavingViews(id, enteringViewItem, leavingViewItem); + viewStacks.unmountLeavingViews(id, enteringViewItem, delta); } } else { - viewStacks.mountIntermediaryViews(id, enteringViewItem, leavingViewItem); + viewStacks.mountIntermediaryViews(id, leavingViewItem, delta); } fireLifecycle(leavingViewItem.vueComponent, leavingViewItem.vueComponentRef, LIFECYCLE_DID_LEAVE); diff --git a/packages/vue/test-app/tests/unit/routing.spec.ts b/packages/vue/test-app/tests/unit/routing.spec.ts index ae510fc22e..62433e1067 100644 --- a/packages/vue/test-app/tests/unit/routing.spec.ts +++ b/packages/vue/test-app/tests/unit/routing.spec.ts @@ -441,4 +441,104 @@ describe('Routing', () => { expect(beforeRouteEnterSpy).toHaveBeenCalledTimes(2); }); + + it('should not mount intermediary components when delta is 1', async () => { + const Page = { + components: { IonPage }, + template: `` + } + const Page2 = { + components: { IonPage }, + template: `` + } + const Page3 = { + components: { IonPage }, + template: `` + } + + const router = createRouter({ + history: createWebHistory(process.env.BASE_URL), + routes: [ + { path: '/page', component: Page }, + { path: '/page2', component: Page2 }, + { path: '/page3', component: Page3 }, + { path: '/', redirect: '/page' } + ] + }); + + router.push('/'); + await router.isReady(); + const wrapper = mount(App, { + global: { + plugins: [router, IonicVue] + } + }); + + expect(wrapper.findComponent(Page).exists()).toBe(true); + + router.push('/page2'); + await waitForRouter(); + + expect(wrapper.findComponent(Page2).exists()).toBe(true); + + router.back(); + await waitForRouter(); + + expect(wrapper.findComponent(Page2).exists()).toBe(false); + + router.push('/page3'); + await waitForRouter(); + + expect(wrapper.findComponent(Page2).exists()).toBe(false); + expect(wrapper.findComponent(Page3).exists()).toBe(true); + }); + + it('should unmount intermediary components when using router.go', async () => { + const Page = { + components: { IonPage }, + template: `` + } + const Page2 = { + components: { IonPage }, + template: `` + } + const Page3 = { + components: { IonPage }, + template: `` + } + + const router = createRouter({ + history: createWebHistory(process.env.BASE_URL), + routes: [ + { path: '/page', component: Page }, + { path: '/page2', component: Page2 }, + { path: '/page3', component: Page3 }, + { path: '/', redirect: '/page' } + ] + }); + + router.push('/'); + await router.isReady(); + const wrapper = mount(App, { + global: { + plugins: [router, IonicVue] + } + }); + + router.push('/page2'); + await waitForRouter(); + + router.push('/page3'); + await waitForRouter(); + + expect(wrapper.findComponent(Page2).exists()).toBe(true); + expect(wrapper.findComponent(Page3).exists()).toBe(true); + + router.go(-2); + await waitForRouter(); + + expect(wrapper.findComponent(Page).exists()).toBe(true); + expect(wrapper.findComponent(Page2).exists()).toBe(false); + expect(wrapper.findComponent(Page3).exists()).toBe(false); + }); });