From: Eduardo San Martin Morote Date: Fri, 3 May 2019 14:18:14 +0000 (+0200) Subject: feat: allow redirect in next X-Git-Tag: v4.0.0-alpha.0~400 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=795e889cc961ddc9c45ccc57b8145fd2113fc9df;p=thirdparty%2Fvuejs%2Frouter.git feat: allow redirect in next --- diff --git a/__tests__/guards/global-beforeEach.spec.js b/__tests__/guards/global-beforeEach.spec.js index af51e4d0..82aae142 100644 --- a/__tests__/guards/global-beforeEach.spec.js +++ b/__tests__/guards/global-beforeEach.spec.js @@ -23,6 +23,8 @@ const Foo = { template: `
Foo
` } const routes = [ { path: '/', component: Home }, { path: '/foo', component: Foo }, + { path: '/other', component: Foo }, + { path: '/n/:i', name: 'n', component: Home }, ] describe('router.beforeEach', () => { @@ -51,6 +53,86 @@ describe('router.beforeEach', () => { expect(spy).not.toHaveBeenCalled() }) + it('can redirect to a different location', async () => { + const spy = jest.fn() + const router = createRouter({ routes }) + await router.push('/foo') + spy.mockImplementation((to, from, next) => { + // only allow going to /other + if (to.fullPath !== '/other') next('/other') + else next() + }) + router.beforeEach(spy) + expect(spy).not.toHaveBeenCalled() + await router[navigationMethod]('/') + expect(spy).toHaveBeenCalledTimes(2) + // called before redirect + expect(spy).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ path: '/' }), + expect.objectContaining({ path: '/foo' }), + expect.any(Function) + ) + expect(spy).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ path: '/other' }), + expect.objectContaining({ path: '/foo' }), + expect.any(Function) + ) + expect(router.currentRoute.fullPath).toBe('/other') + }) + + async function assertRedirect(redirectFn) { + const spy = jest.fn() + const router = createRouter({ routes }) + await router.push('/') + spy.mockImplementation((to, from, next) => { + // only allow going to /other + const i = Number(to.params.i) + if (i >= 3) next() + else next(redirectFn(i + 1)) + }) + router.beforeEach(spy) + expect(spy).not.toHaveBeenCalled() + await router[navigationMethod]('/n/0') + expect(spy).toHaveBeenCalledTimes(4) + expect(router.currentRoute.fullPath).toBe('/n/3') + } + + it('can redirect multiple times with string redirect', async () => { + await assertRedirect(i => '/n/' + i) + }) + + it('can redirect multiple times with path object', async () => { + await assertRedirect(i => ({ path: '/n/' + i })) + }) + + it('can redirect multiple times with named route', async () => { + await assertRedirect(i => ({ name: 'n', params: { i } })) + }) + + it('is called when changing params', async () => { + const spy = jest.fn() + const router = createRouter({ routes: [...routes] }) + await router.push('/n/2') + spy.mockImplementation(noGuard) + router.beforeEach(spy) + spy.mockImplementationOnce(noGuard) + await router[navigationMethod]('/n/1') + expect(spy).toHaveBeenCalledTimes(1) + }) + + it('is not called with same params', async () => { + const spy = jest.fn() + const router = createRouter({ routes: [...routes] }) + await router.push('/n/2') + spy.mockImplementation(noGuard) + router.beforeEach(spy) + spy.mockImplementationOnce(noGuard) + await router[navigationMethod]('/n/2') + expect(spy).not.toHaveBeenCalled() + }) + it('waits before navigating', async () => { const [promise, resolve] = fakePromise() const router = createRouter({ routes }) diff --git a/src/errors.ts b/src/errors.ts index ef6d4b91..923794b7 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -1,6 +1,34 @@ +import { RouteLocationNormalized, RouteLocation } from './types' + export class NoRouteMatchError extends Error { constructor(currentLocation: any, location: any) { super('No match for ' + JSON.stringify({ ...currentLocation, ...location })) Object.setPrototypeOf(this, new.target.prototype) } } + +/** + * Error used when rejecting a navigation because of a redirection. Contains + * information about where we where trying to go and where we are going instead + */ +export class RedirectError extends Error { + from: RouteLocationNormalized + to: RouteLocation + constructor(from: RouteLocationNormalized, to: RouteLocation) { + super( + `Redirected from "${from.fullPath}" to "${stringifyRoute( + to + )}" via a navigation guard` + ) + Object.setPrototypeOf(this, new.target.prototype) + + this.from = from + this.to = to + } +} + +function stringifyRoute(to: RouteLocation): string { + if (typeof to === 'string') return to + if ('path' in to) return to.path + return 'TODO' +} diff --git a/src/router.ts b/src/router.ts index db28e56c..ae3e1c0d 100644 --- a/src/router.ts +++ b/src/router.ts @@ -10,9 +10,11 @@ import { NavigationGuard, TODO, PostNavigationGuard, + Lazy, } from './types/index' -import { guardToPromiseFn, last, extractComponentsGuards } from './utils' +import { guardToPromiseFn, extractComponentsGuards } from './utils' +import { RedirectError } from './errors' export interface RouterOptions { history: BaseHistory @@ -50,7 +52,7 @@ export class Router { * guards first * @param to where to go */ - async push(to: RouteLocation) { + async push(to: RouteLocation): Promise { let url: HistoryLocationNormalized let location: MatcherLocationNormalized if (typeof to === 'string' || 'path' in to) { @@ -69,9 +71,21 @@ export class Router { }) } + // TODO: should we throw an error as the navigation was aborted + if (this.currentRoute.fullPath === url.fullPath) return this.currentRoute + const toLocation: RouteLocationNormalized = { ...url, ...location } // trigger all guards, throw if navigation is rejected - await this.navigate(toLocation, this.currentRoute) + try { + await this.navigate(toLocation, this.currentRoute) + } catch (error) { + if (error instanceof RedirectError) { + // TODO: setup redirect stack + return this.push(error.to) + } else { + throw error + } + } // change URL if (to.replace === true) this.history.replace(url) @@ -82,6 +96,8 @@ export class Router { // navigation is confirmed, call afterGuards for (const guard of this.afterGuards) guard(toLocation, from) + + return this.currentRoute } /** @@ -94,13 +110,24 @@ export class Router { return this.push({ ...location, replace: true }) } + /** + * Runs a guard queue and handles redirects, rejections + * @param guards Array of guards converted to functions that return a promise + * @returns {boolean} true if the navigation should be cancelled false otherwise + */ + private async runGuardQueue(guards: Lazy[]): Promise { + for (const guard of guards) { + await guard() + } + } + private async navigate( to: RouteLocationNormalized, from: RouteLocationNormalized ): Promise { // TODO: Will probably need to be some kind of queue in the future that allows to remove // elements and other stuff - let guards: Array<() => Promise> + let guards: Lazy[] // TODO: is it okay to resolve all matched component or should we do it in order // TODO: use only components that we are leaving (children) @@ -112,26 +139,19 @@ export class Router { ) // run the queue of per route beforeEnter guards - for (const guard of guards) { - await guard() - } + await this.runGuardQueue(guards) // check global guards beforeEach // avoid if we are not changing route // TODO: trigger on child navigation - // TODO: should we completely avoid a navigation towards the same route? - if (last(to.matched) !== last(from.matched)) { - guards = [] - for (const guard of this.beforeGuards) { - guards.push(guardToPromiseFn(guard, to, from)) - } - - // console.log('Guarding against', guards.length, 'guards') - for (const guard of guards) { - await guard() - } + guards = [] + for (const guard of this.beforeGuards) { + guards.push(guardToPromiseFn(guard, to, from)) } + // console.log('Guarding against', guards.length, 'guards') + await this.runGuardQueue(guards) + // check in components beforeRouteUpdate guards = await extractComponentsGuards( to.matched.filter(record => from.matched.indexOf(record) > -1), @@ -141,9 +161,7 @@ export class Router { ) // run the queue of per route beforeEnter guards - for (const guard of guards) { - await guard() - } + await this.runGuardQueue(guards) // check the route beforeEnter // TODO: check children. Should we also check reused routes guards @@ -155,9 +173,7 @@ export class Router { } // run the queue of per route beforeEnter guards - for (const guard of guards) { - await guard() - } + await this.runGuardQueue(guards) // check in-component beforeRouteEnter // TODO: is it okay to resolve all matched component or should we do it in order @@ -169,9 +185,7 @@ export class Router { ) // run the queue of per route beforeEnter guards - for (const guard of guards) { - await guard() - } + await this.runGuardQueue(guards) } /** diff --git a/src/types/index.ts b/src/types/index.ts index eab328be..bebd9c20 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -1,6 +1,6 @@ import { HistoryQuery } from '../history/base' -type Lazy = () => Promise +export type Lazy = () => Promise export type TODO = any diff --git a/src/utils/guardToPromiseFn.ts b/src/utils/guardToPromiseFn.ts index 1d5f9377..6304eb24 100644 --- a/src/utils/guardToPromiseFn.ts +++ b/src/utils/guardToPromiseFn.ts @@ -6,6 +6,7 @@ import { } from '../types' import { isRouteLocation } from './index' +import { RedirectError } from '../errors' export function guardToPromiseFn( guard: NavigationGuard, @@ -22,6 +23,7 @@ export function guardToPromiseFn( if (valid === false) reject(new Error('Aborted')) else if (isRouteLocation(valid)) { // TODO: redirect + reject(new RedirectError(to, valid)) } else resolve() }