diff --git a/packages/vue-router/src/router.ts b/packages/vue-router/src/router.ts index 7045dad5947..f4308e1a7cf 100644 --- a/packages/vue-router/src/router.ts +++ b/packages/vue-router/src/router.ts @@ -117,10 +117,6 @@ export const createIonRouter = ( * instead, so there is no failure to inspect there and the staged state * would survive. The error is re-thrown to the app the same way it was * before, so navigation outcomes are unchanged. - * - * A guard that returns a location is still not covered, because that - * redirects rather than fails and afterEach is never called for the original - * navigation. */ addErrorHandler((error: unknown, to: RouteLocationNormalized) => { discardStagedStateFor(to); @@ -183,20 +179,71 @@ export const createIonRouter = ( incomingRouteParamsUnclaimed = false; }; + /** + * Whether `to` is a guard redirect of the navigation that claimed `owner`. + * vue-router points `redirectedFrom` at the first location in a redirect + * chain, so an owner that a `redirect:` record led to is matched on its own + * `redirectedFrom`. + */ + const isGuardRedirectOf = ( + owner: RouteLocationNormalized | undefined, + to: RouteLocationNormalized + ) => + owner !== undefined && + to.redirectedFrom !== undefined && + (owner.redirectedFrom ?? owner) === to.redirectedFrom; + + /** + * Whether `to` was redirected by a guard rather than a `redirect:` record. + * This can't tell a record redirect apart from one a leave guard redirected + * again afterwards. + */ + const isRedirectedByGuard = (to: RouteLocationNormalized) => { + const origin = to.redirectedFrom; + + return ( + origin !== undefined && + origin.matched[origin.matched.length - 1]?.redirect === undefined + ); + }; + /** * The navigation that starts first after params are staged is the one they * were staged for, so it takes ownership of them here. Registered before any * guard the app adds so that it still runs when one of those aborts. + * + * A guard that returns a location starts a new navigation and the original + * never reaches afterEach or onError, so we discard what it claimed here + * instead. Leave guards run before this hook, so a navigation they redirect + * hasn't claimed anything yet and its state is discarded unclaimed. */ router.beforeEach((to: RouteLocationNormalized) => { + if (isGuardRedirectOf(currentNavigationInfoOwner, to)) { + clearNavigationInfo(); + } + + if (isGuardRedirectOf(incomingRouteParamsOwner, to)) { + clearStagedParams(); + } + + const redirectedByGuard = isRedirectedByGuard(to); + if (incomingRouteParamsUnclaimed) { - incomingRouteParamsOwner = to; - incomingRouteParamsUnclaimed = false; + if (redirectedByGuard) { + clearStagedParams(); + } else { + incomingRouteParamsOwner = to; + incomingRouteParamsUnclaimed = false; + } } if (currentNavigationInfoUnclaimed) { - currentNavigationInfoOwner = to; - currentNavigationInfoUnclaimed = false; + if (redirectedByGuard) { + clearNavigationInfo(); + } else { + currentNavigationInfoOwner = to; + currentNavigationInfoUnclaimed = false; + } } }); @@ -219,18 +266,22 @@ export const createIonRouter = ( * Both are matched on the navigation that owns them rather than on where it * was heading, because two navigations can head for the same path and a path * cannot tell them apart. State still unclaimed belongs to this navigation, - * since nothing has started since it was staged. + * since nothing has started since it was staged. A guard redirect that fails + * before reaching beforeEach, like one to the current page, is matched + * through the navigation it redirected. */ const discardStagedStateFor = (to: RouteLocationNormalized) => { const deltaIsForThisNavigation = currentNavigationInfoOwner === undefined ? currentNavigationInfoUnclaimed - : currentNavigationInfoOwner === to; + : currentNavigationInfoOwner === to || + isGuardRedirectOf(currentNavigationInfoOwner, to); const paramsAreForThisNavigation = incomingRouteParamsOwner === undefined ? incomingRouteParamsUnclaimed - : incomingRouteParamsOwner === to; + : incomingRouteParamsOwner === to || + isGuardRedirectOf(incomingRouteParamsOwner, to); if (deltaIsForThisNavigation) { clearNavigationInfo(); diff --git a/packages/vue/test/base/tests/unit/routing.spec.ts b/packages/vue/test/base/tests/unit/routing.spec.ts index dcb7d39c6de..ffa5985f4ea 100644 --- a/packages/vue/test/base/tests/unit/routing.spec.ts +++ b/packages/vue/test/base/tests/unit/routing.spec.ts @@ -75,6 +75,16 @@ const waitUntil = async (predicate: () => boolean, label: string) => { throw new Error(`timed out waiting for ${label}`); }; +/* + * A redirect ends a navigation somewhere other than where it started, so wait + * for the path it should end on before letting the router settle. + */ +const navigateTo = async (router: any, path: string, navigate: () => void) => { + navigate(); + await waitUntil(() => router.currentRoute.value.path === path, `the navigation to ${path}`); + await waitForRouter(); +}; + describe('Routing', () => { it('should pass no props', async () => { const Page1 = { @@ -1609,6 +1619,304 @@ describe('Routing', () => { ]); }); + const mountBackRedirect = async () => { + let navManager: any; + + const Home = { + ...createPage('home'), + setup() { + navManager = inject('navManager'); + } + }; + + const router = createRouter({ + history: createWebHistory(process.env.BASE_URL), + routes: [ + { path: '/', redirect: '/home' }, + { path: '/home', component: Home }, + { path: '/login', component: createPage('login') }, + { path: '/profile', component: createPage('profile') } + ] + }); + + router.beforeEach((to, from) => { + if (from.path === '/profile' && to.path === '/home') { + return '/login'; + } + + return true; + }); + + router.push('/'); + await router.isReady(); + const wrapper = mount(IonRouterOutlet, { + global: { + plugins: [router, IonicVue] + } + }); + + router.push('/profile'); + await waitForRouter(); + + return { router, navManager, wrapper }; + }; + + it('should show the redirect target when a guard redirects a browser back', async () => { + const { router, navManager, wrapper } = await mountBackRedirect(); + + await navigateTo(router, '/login', () => router.back()); + + expect(currentRoute(navManager)).toEqual({ + pathname: '/login', + routerAction: 'push', + routerDirection: 'forward' + }); + expect(viewStack(wrapper)).toEqual([ + { id: 'home', hidden: true }, + { id: 'profile', hidden: true }, + { id: 'login', hidden: false } + ]); + }); + + it('should show the redirect target when a guard redirects a back button navigation', async () => { + const { router, navManager, wrapper } = await mountBackRedirect(); + + /* + * The back button stages the Home route it expects to go back to, which + * Login would inherit if it were carried over. + */ + await navigateTo(router, '/login', () => navManager.handleNavigateBack()); + + expect(currentRoute(navManager)).toEqual({ + pathname: '/login', + routerAction: 'push', + routerDirection: 'forward' + }); + expect(viewStack(wrapper)).toEqual([ + { id: 'home', hidden: true }, + { id: 'profile', hidden: true }, + { id: 'login', hidden: false } + ]); + }); + + it('should go back normally after a guard redirects a browser back', async () => { + const { router, navManager, wrapper } = await mountBackRedirect(); + + await navigateTo(router, '/login', () => router.back()); + + // The redirect replaced Profile's history entry, so back lands on Home. + await navigateTo(router, '/home', () => router.back()); + + expect(currentRoute(navManager)).toEqual({ + pathname: '/home', + routerAction: 'pop', + routerDirection: 'back' + }); + expect(viewStack(wrapper)).toEqual([ + { id: 'home', hidden: false }, + { id: 'profile', hidden: true } + ]); + }); + + /* + * Leave guards run before any global guard, so Ionic never sees the + * navigation they redirect start. + */ + const mountLeaveRedirect = async () => { + let navManager: any; + + const Home = { + ...createPage('home'), + setup() { + navManager = inject('navManager'); + } + }; + const Profile = { + ...createPage('profile'), + setup() { + onBeforeRouteLeave((to) => (to.path === '/home' ? '/login' : true)); + } + }; + + const router = createRouter({ + history: createWebHistory(process.env.BASE_URL), + routes: [ + { path: '/', redirect: '/home' }, + { path: '/home', component: Home }, + { path: '/login', component: createPage('login') }, + { path: '/profile', component: Profile } + ] + }); + + router.push('/'); + await router.isReady(); + const wrapper = mount(IonRouterOutlet, { + global: { + plugins: [router, IonicVue] + } + }); + + router.push('/profile'); + await waitForRouter(); + + return { router, navManager, wrapper }; + }; + + it('should show the redirect target when a leave guard redirects a browser back', async () => { + const { router, navManager, wrapper } = await mountLeaveRedirect(); + + await navigateTo(router, '/login', () => router.back()); + + expect(currentRoute(navManager)).toEqual({ + pathname: '/login', + routerAction: 'push', + routerDirection: 'forward' + }); + expect(viewStack(wrapper)).toEqual([ + { id: 'home', hidden: true }, + { id: 'profile', hidden: true }, + { id: 'login', hidden: false } + ]); + }); + + it('should show the redirect target when a leave guard redirects a back button navigation', async () => { + const { router, navManager, wrapper } = await mountLeaveRedirect(); + + await navigateTo(router, '/login', () => navManager.handleNavigateBack()); + + expect(currentRoute(navManager)).toEqual({ + pathname: '/login', + routerAction: 'push', + routerDirection: 'forward' + }); + expect(viewStack(wrapper)).toEqual([ + { id: 'home', hidden: true }, + { id: 'profile', hidden: true }, + { id: 'login', hidden: false } + ]); + }); + + /* + * Only a navigation that no guard redirected should keep the root direction, + * including one that a `redirect:` record redirected. + */ + const mountRootRedirect = async () => { + let navManager: any; + + const Home = { + ...createPage('home'), + setup() { + navManager = inject('navManager'); + } + }; + + const router = createRouter({ + history: createWebHistory(process.env.BASE_URL), + routes: [ + { path: '/', redirect: '/home' }, + { path: '/home', component: Home }, + { path: '/login', component: createPage('login') }, + { path: '/dashboard', component: createPage('dashboard') }, + { path: '/tabs', redirect: '/tabs/tab' }, + { path: '/tabs/tab', component: createPage('tab') }, + { path: '/admin', redirect: '/admin/users' }, + { path: '/admin/users', component: createPage('users') } + ] + }); + + router.beforeEach((to) => { + if (to.path === '/dashboard' || to.path === '/admin/users') { + return '/login'; + } + + return true; + }); + + router.push('/'); + await router.isReady(); + const wrapper = mount(IonRouterOutlet, { + global: { + plugins: [router, IonicVue] + } + }); + + return { router, navManager, wrapper }; + }; + + it('should not apply the params of a guard redirected navigation to the redirect target', async () => { + const { router, navManager, wrapper } = await mountRootRedirect(); + + // If it were carried over, the root direction would drop Home from the back stack. + await navigateTo(router, '/login', () => navManager.handleNavigate('/dashboard', 'push', 'root')); + + expect(currentRoute(navManager)).toEqual({ + pathname: '/login', + routerAction: 'push', + routerDirection: 'forward' + }); + expect(navManager.canGoBack()).toBe(true); + expect(viewStack(wrapper)).toEqual([ + { id: 'home', hidden: true }, + { id: 'login', hidden: false } + ]); + }); + + // Guards against clearing the params of a navigation that only a route record redirected. + it('should keep the params of a navigation redirected by a route record', async () => { + const { router, navManager, wrapper } = await mountRootRedirect(); + + await navigateTo(router, '/tabs/tab', () => navManager.handleNavigate('/tabs', 'push', 'root')); + + expect(currentRoute(navManager)).toEqual({ + pathname: '/tabs/tab', + routerAction: 'push', + routerDirection: 'root' + }); + expect(navManager.canGoBack()).toBe(false); + expect(viewStack(wrapper)).toEqual([{ id: 'tab', hidden: false }]); + }); + + it('should not apply the params when a guard redirects after a route record', async () => { + const { router, navManager, wrapper } = await mountRootRedirect(); + + await navigateTo(router, '/login', () => navManager.handleNavigate('/admin', 'push', 'root')); + + expect(currentRoute(navManager)).toEqual({ + pathname: '/login', + routerAction: 'push', + routerDirection: 'forward' + }); + expect(navManager.canGoBack()).toBe(true); + expect(viewStack(wrapper)).toEqual([ + { id: 'home', hidden: true }, + { id: 'login', hidden: false } + ]); + }); + + it('should not apply the params when a guard redirects to the current page', async () => { + const { router, navManager, wrapper } = await mountRootRedirect(); + + await navigateTo(router, '/login', () => router.push('/login')); + + // The guard sends it back to Login, where the router already is, so it fails as a duplicate. + navManager.handleNavigate('/dashboard', 'push', 'root'); + await waitForRouter(); + + await navigateTo(router, '/tabs/tab', () => router.push('/tabs')); + + expect(currentRoute(navManager)).toEqual({ + pathname: '/tabs/tab', + routerAction: 'push', + routerDirection: 'forward' + }); + expect(navManager.canGoBack()).toBe(true); + expect(viewStack(wrapper)).toEqual([ + { id: 'home', hidden: true }, + { id: 'login', hidden: true }, + { id: 'tab', hidden: false } + ]); + }); + /* * Registering an error handler stops vue-router logging an uncaught * navigation error itself, so these cover the log still reaching an app that