diff --git a/packages/vue/src/components/IonRouterOutlet.ts b/packages/vue/src/components/IonRouterOutlet.ts index 8397ab9a86..dea05d9525 100644 --- a/packages/vue/src/components/IonRouterOutlet.ts +++ b/packages/vue/src/components/IonRouterOutlet.ts @@ -8,7 +8,8 @@ import { watch, shallowRef, InjectionKey, - onUnmounted + onUnmounted, + Ref } from 'vue'; import { AnimationBuilder, LIFECYCLE_DID_ENTER, LIFECYCLE_DID_LEAVE, LIFECYCLE_WILL_ENTER, LIFECYCLE_WILL_LEAVE } from '@ionic/core/components'; import { IonRouterOutlet as IonRouterOutletCmp } from '@ionic/core/components/ion-router-outlet.js'; @@ -29,6 +30,8 @@ export const IonRouterOutlet = /*@__PURE__*/ defineComponent({ const route = useRoute(); const depth = inject(viewDepthKey, 0); const matchedRouteRef: any = computed(() => route.matched[depth]); + let previousMatchedRouteRef: Ref | undefined; + let previousMatchedPath: string | undefined; provide(viewDepthKey, depth + 1) provide(matchedRouteKey, matchedRouteRef); @@ -48,31 +51,55 @@ export const IonRouterOutlet = /*@__PURE__*/ defineComponent({ let parentOutletPath: string; /** - * We need to watch the route object - * to listen for navigation changes. - * Previously we had watched matchedRouteRef, - * but if you had a /page/:id route, going from - * page/1 to page/2 would not cause this callback - * to fire since the matchedRouteRef was the same. + * Note: Do not listen for matchedRouteRef by itself here + * as the callback will not fire for parameterized routes (i.e. /page/:id). + * So going from /page/1 to /page/2 would not fire this callback if we + * only listened for changes to matchedRouteRef. */ - watch(() => [route, matchedRouteRef.value], ([currentRoute, currentMatchedRouteRef], [_, previousMatchedRouteRef]) => { + watch(() => [route, matchedRouteRef.value], ([currentRoute, currentMatchedRouteRef]) => { /** - * If the matched route ref has changed, - * then we need to set up a new view item. - * If the matched route ref has not changed, - * it is possible that this is a parameterized URL - * change such as /page/1 to /page/2. In that case, - * we can assume that the `route` object has changed, - * but we should only set up a new view item in this outlet - * if that last matched view item matches our current matched - * view item otherwise if we had this in a nested outlet the - * parent outlet would re-render as well as the child page. + * This callback checks whether or not a router outlet + * needs to respond to a change in the matched route. + * It handles a few cases: + * 1. The matched route is undefined. This means that + * the matched route is not applicable to this outlet. + * For example, a /settings route is not applicable + * to a /tabs/... route. + * + * Note: When going back to a tabs outlet from a non-tabs outlet, + * the tabs outlet should NOT attempt a page transition from the + * previous tab to the active tab. To do this we compare the current + * route with the previous route. Unfortunately, we cannot rely on the + * previous value provided by Vue in the watch callback. This is because + * when coming back to the tabs context, the previous matched route will + * be undefined (because nothing in the tabs context matches /settings) + * but the current matched route will be defined and so a transition + * will always occur. + * + * 2. The matched route is defined and is different than + * the previously matched route. This is the most + * common case such as when you go from /page1 to /page2. + * + * 3. The matched route is the same but the parameters are different. + * This is a special case for parameterized routes (i.e. /page/:id). + * When going from /page/1 to /page/2, the matched route object will + * be the same, but we still need to perform a page transition. To do this + * we check if the parameters are different (i.e. 1 vs 2). To avoid enumerating + * all of the keys in the params object, we check the url path to + * see if they are different after ensuring we are in a parameterized route. */ - if ( - currentMatchedRouteRef !== previousMatchedRouteRef || - currentRoute.matched[currentRoute.matched.length - 1] === currentMatchedRouteRef - ) { + if (currentMatchedRouteRef === undefined) { return; } + + const matchedDifferentRoutes = currentMatchedRouteRef !== previousMatchedRouteRef; + const matchedDifferentParameterRoutes = ( + currentRoute.matched[currentRoute.matched.length - 1] === currentMatchedRouteRef && + currentRoute.path !== previousMatchedPath + ); + + if (matchedDifferentRoutes || matchedDifferentParameterRoutes) { setupViewItem(matchedRouteRef); + previousMatchedRouteRef = currentMatchedRouteRef; + previousMatchedPath = currentRoute.path; } }); diff --git a/packages/vue/test-app/src/main.ts b/packages/vue/test-app/src/main.ts index ce30a3f479..c86c39adbb 100644 --- a/packages/vue/test-app/src/main.ts +++ b/packages/vue/test-app/src/main.ts @@ -23,10 +23,24 @@ import '@ionic/vue/css/display.css'; /* Theme variables */ import './theme/variables.css'; +/** + * Vue 3 has its own error handling. + * Throwing errors in promises go through + * this handler, but Cypress does not + * pick up on them so tests that are meant + * to fail will pass. By listening for unhandledrejection + * we can throw an error outside of Vue that will + * cause the test to fail as it should. + * See https://github.com/cypress-io/cypress/issues/5385#issuecomment-547642523 + */ +window.addEventListener('unhandledrejection', (err) => { + throw new Error(err.reason); +}); + const app = createApp(App) .use(IonicVue) .use(router); - + router.isReady().then(() => { app.mount('#app'); -}); \ No newline at end of file +}); diff --git a/packages/vue/test-app/tests/e2e/specs/tabs.js b/packages/vue/test-app/tests/e2e/specs/tabs.js index 3ce2d90d2b..4e66c45ec0 100644 --- a/packages/vue/test-app/tests/e2e/specs/tabs.js +++ b/packages/vue/test-app/tests/e2e/specs/tabs.js @@ -339,6 +339,32 @@ describe('Tabs', () => { cy.ionPageVisible('tab2'); cy.ionPageHidden('tab1'); }); + + // Verifies fix for https://github.com/ionic-team/ionic-framework/issues/24654 + it('should not error when going back to a tabs view from a non tabs view', () => { + cy.visit('http://localhost:8080/tabs'); + + cy.routerPush('/tabs/tab1/childone'); + cy.ionPageVisible('tab1childone'); + cy.ionPageHidden('tab1'); + + cy.routerGo(-1); + cy.ionPageDoesNotExist('tab1childone'); + cy.ionPageVisible('tab1'); + + cy.routerPush('/tabs/tab1/childtwo'); + cy.ionPageVisible('tab1childtwo'); + cy.ionPageHidden('tab1'); + + cy.routerPush('/inputs'); + cy.ionPageVisible('tab1childtwo'); + cy.ionPageHidden('tabs'); + + cy.routerGo(-1); + cy.ionPageDoesNotExist('inputs'); + cy.ionPageVisible('tab1childtwo'); + cy.ionPageVisible('tabs'); + }) }) describe('Tabs - Swipe to Go Back', () => {