From: Eduardo San Martin Morote Date: Fri, 17 Apr 2020 17:51:23 +0000 (+0200) Subject: feat: refactor navigation to comply with vuejs/rfcs#150 X-Git-Tag: v4.0.0-alpha.7~4 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=290c3be1f6cb476016f23b77d6fc49987dd84751;p=thirdparty%2Fvuejs%2Frouter.git feat: refactor navigation to comply with vuejs/rfcs#150 BREAKING CHANGE: This follows the RFC at https://github.com/vuejs/rfcs/pull/150 Summary: `router.afterEach` and `router.onError` are now the global equivalent of `router.push`/`router.replace` as well as navigation through the interface (`history.go()`). A navigation only rejects if there was an unexpected error. A navigation failure will still resolve the promise returned by `router.push` and be exposed as the resolved value. --- diff --git a/__tests__/errors.spec.ts b/__tests__/errors.spec.ts index 4ae4c263..305d7763 100644 --- a/__tests__/errors.spec.ts +++ b/__tests__/errors.spec.ts @@ -1,27 +1,17 @@ +import fakePromise from 'faked-promise' import { createRouter as newRouter, createMemoryHistory } from '../src' -import { ErrorTypes } from '../src/errors' -import { components } from './utils' -import { RouteRecordRaw } from '../src/types' +import { NavigationFailure, NavigationFailureType } from '../src/errors' +import { components, tick } from './utils' +import { RouteRecordRaw, NavigationGuard, RouteLocationRaw } from '../src/types' const routes: RouteRecordRaw[] = [ { path: '/', component: components.Home }, + { path: '/redirect', redirect: '/' }, { path: '/foo', component: components.Foo, name: 'Foo' }, - { path: '/to-foo', redirect: '/foo' }, - { path: '/to-foo-named', redirect: { name: 'Foo' } }, - { path: '/to-foo2', redirect: '/to-foo' }, - { path: '/p/:p', name: 'Param', component: components.Bar }, - { path: '/to-p/:p', redirect: to => `/p/${to.params.p}` }, - { - path: '/inc-query-hash', - redirect: to => ({ - name: 'Foo', - query: { n: to.query.n + '-2' }, - hash: to.hash + '-2', - }), - }, ] const onError = jest.fn() +const afterEach = jest.fn() function createRouter() { const history = createMemoryHistory() const router = newRouter({ @@ -30,27 +20,298 @@ function createRouter() { }) router.onError(onError) + router.afterEach(afterEach) return { router, history } } -describe('Errors', () => { +describe('Errors & Navigation failures', () => { beforeEach(() => { onError.mockReset() + afterEach.mockReset() }) - it('triggers onError when navigation is aborted', async () => { + it('next(false) triggers afterEach', async () => { + await testNavigation( + false, + expect.objectContaining({ + type: NavigationFailureType.aborted, + }) + ) + }) + + it('next("/location") triggers afterEach', async () => { + await testNavigation( + ((to, from, next) => { + if (to.path === '/location') next() + else next('/location') + }) as NavigationGuard, + undefined + ) + }) + + it('redirect triggers afterEach', async () => { + await testNavigation(undefined, undefined, '/redirect') + }) + + it('next() triggers afterEach', async () => { + await testNavigation(undefined, undefined) + }) + + it('next(true) triggers afterEach', async () => { + await testNavigation(true, undefined) + }) + + it('triggers afterEach if a new navigation happens', async () => { const { router } = createRouter() + const [promise, resolve] = fakePromise() router.beforeEach((to, from, next) => { - next(false) + // let it hang otherwise + if (to.path === '/') next() + else promise.then(() => next()) }) - try { - await router.push('/foo') - } catch (err) { - expect(err.type).toBe(ErrorTypes.NAVIGATION_ABORTED) - } - expect(onError).toHaveBeenCalledWith( - expect.objectContaining({ type: ErrorTypes.NAVIGATION_ABORTED }) + let from = router.currentRoute.value + + // should hang + let navigationPromise = router.push('/foo') + + await expect(router.push('/')).resolves.toEqual(undefined) + expect(afterEach).toHaveBeenCalledTimes(1) + expect(onError).toHaveBeenCalledTimes(0) + + resolve() + await navigationPromise + expect(afterEach).toHaveBeenCalledTimes(2) + expect(onError).toHaveBeenCalledTimes(0) + + expect(afterEach).toHaveBeenLastCalledWith( + expect.objectContaining({ path: '/foo' }), + from, + expect.objectContaining({ type: NavigationFailureType.cancelled }) ) }) + + it('next(new Error()) triggers onError', async () => { + let error = new Error() + await testError(error, error) + }) + + it('triggers onError with thrown errors', async () => { + let error = new Error() + await testError(() => { + throw error + }, error) + }) + + it('triggers onError with rejected promises', async () => { + let error = new Error() + await testError(async () => { + throw error + }, error) + }) + + describe('history navigation', () => { + it('triggers afterEach with history.back', async () => { + const { router, history } = createRouter() + + await router.push('/') + await router.push('/foo') + + afterEach.mockReset() + onError.mockReset() + + const [promise, resolve] = fakePromise() + router.beforeEach((to, from, next) => { + // let it hang otherwise + if (to.path === '/') next() + else promise.then(() => next()) + }) + + let from = router.currentRoute.value + + // should hang + let navigationPromise = router.push('/bar') + + // goes from /foo to / + history.go(-1) + + await tick() + + expect(afterEach).toHaveBeenCalledTimes(1) + expect(onError).toHaveBeenCalledTimes(0) + expect(afterEach).toHaveBeenLastCalledWith( + expect.objectContaining({ path: '/' }), + from, + undefined + ) + + resolve() + await expect(navigationPromise).resolves.toEqual( + expect.objectContaining({ type: NavigationFailureType.cancelled }) + ) + + expect(afterEach).toHaveBeenCalledTimes(2) + expect(onError).toHaveBeenCalledTimes(0) + + expect(afterEach).toHaveBeenLastCalledWith( + expect.objectContaining({ path: '/bar' }), + from, + expect.objectContaining({ type: NavigationFailureType.cancelled }) + ) + }) + + it('next(false) triggers afterEach with history.back', async () => { + await testHistoryNavigation( + false, + expect.objectContaining({ type: NavigationFailureType.aborted }) + ) + }) + + it('next("/location") triggers afterEach with history.back', async () => { + await testHistoryNavigation( + ((to, from, next) => { + if (to.path === '/location') next() + else next('/location') + }) as NavigationGuard, + undefined + ) + }) + + it('next() triggers afterEach with history.back', async () => { + await testHistoryNavigation(undefined, undefined) + }) + + it('next(true) triggers afterEach with history.back', async () => { + await testHistoryNavigation(true, undefined) + }) + + it('next(new Error()) triggers onError with history.back', async () => { + let error = new Error() + await testHistoryError(error, error) + }) + + it('triggers onError with thrown errors with history.back', async () => { + let error = new Error() + await testHistoryError(() => { + throw error + }, error) + }) + + it('triggers onError with rejected promises with history.back', async () => { + let error = new Error() + await testHistoryError(async () => { + throw error + }, error) + }) + }) }) + +async function testError( + nextArgument: any | NavigationGuard, + expectedError: Error | void = undefined, + to: RouteLocationRaw = '/foo' +) { + const { router } = createRouter() + router.beforeEach( + typeof nextArgument === 'function' + ? nextArgument + : (to, from, next) => { + next(nextArgument) + } + ) + + await expect(router.push(to)).rejects.toEqual(expectedError) + + expect(afterEach).toHaveBeenCalledTimes(0) + expect(onError).toHaveBeenCalledTimes(1) + + expect(onError).toHaveBeenCalledWith(expectedError) +} + +async function testNavigation( + nextArgument: any | NavigationGuard, + expectedFailure: NavigationFailure | void = undefined, + to: RouteLocationRaw = '/foo' +) { + const { router } = createRouter() + router.beforeEach( + typeof nextArgument === 'function' + ? nextArgument + : (to, from, next) => { + next(nextArgument) + } + ) + + await expect(router.push(to)).resolves.toEqual(expectedFailure) + + expect(afterEach).toHaveBeenCalledTimes(1) + expect(onError).toHaveBeenCalledTimes(0) + + expect(afterEach).toHaveBeenCalledWith( + expect.any(Object), + expect.any(Object), + expectedFailure + ) +} + +async function testHistoryNavigation( + nextArgument: any | NavigationGuard, + expectedFailure: NavigationFailure | void = undefined, + to: RouteLocationRaw = '/foo' +) { + const { router, history } = createRouter() + await router.push(to) + + router.beforeEach( + typeof nextArgument === 'function' + ? nextArgument + : (to, from, next) => { + next(nextArgument) + } + ) + + afterEach.mockReset() + onError.mockReset() + + history.go(-1) + + await tick() + + expect(afterEach).toHaveBeenCalledTimes(1) + expect(onError).toHaveBeenCalledTimes(0) + + expect(afterEach).toHaveBeenCalledWith( + expect.any(Object), + expect.any(Object), + expectedFailure + ) +} + +async function testHistoryError( + nextArgument: any | NavigationGuard, + expectedError: Error | void = undefined, + to: RouteLocationRaw = '/foo' +) { + const { router, history } = createRouter() + await router.push(to) + + router.beforeEach( + typeof nextArgument === 'function' + ? nextArgument + : (to, from, next) => { + next(nextArgument) + } + ) + + afterEach.mockReset() + onError.mockReset() + + history.go(-1) + + await tick() + + expect(afterEach).toHaveBeenCalledTimes(0) + expect(onError).toHaveBeenCalledTimes(1) + + expect(onError).toHaveBeenCalledWith(expectedError) +} diff --git a/__tests__/guards/global-after.spec.ts b/__tests__/guards/global-after.spec.ts index ae458c07..01a9d44f 100644 --- a/__tests__/guards/global-after.spec.ts +++ b/__tests__/guards/global-after.spec.ts @@ -31,7 +31,8 @@ describe('router.afterEach', () => { expect(spy).toHaveBeenCalledTimes(1) expect(spy).toHaveBeenCalledWith( expect.objectContaining({ fullPath: '/foo' }), - expect.objectContaining({ fullPath: '/' }) + expect.objectContaining({ fullPath: '/' }), + undefined ) }) @@ -44,7 +45,7 @@ describe('router.afterEach', () => { expect(spy).not.toHaveBeenCalled() }) - it('calls afterEach guards on push', async () => { + it('calls afterEach guards on multiple push', async () => { const spy = jest.fn() const router = createRouter({ routes }) await router.push('/nested') @@ -53,24 +54,15 @@ describe('router.afterEach', () => { expect(spy).toHaveBeenCalledTimes(1) expect(spy).toHaveBeenLastCalledWith( expect.objectContaining({ name: 'nested-home' }), - expect.objectContaining({ name: 'nested-default' }) + expect.objectContaining({ name: 'nested-default' }), + undefined ) await router.push('/nested') expect(spy).toHaveBeenLastCalledWith( expect.objectContaining({ name: 'nested-default' }), - expect.objectContaining({ name: 'nested-home' }) + expect.objectContaining({ name: 'nested-home' }), + undefined ) expect(spy).toHaveBeenCalledTimes(2) }) - - it('does not call afterEach if navigation is cancelled', async () => { - const spy = jest.fn() - const router = createRouter({ routes }) - router.afterEach(spy) - router.beforeEach((to, from, next) => { - next(false) // cancel the navigation - }) - await router.push('/foo').catch(err => {}) // ignore the error - expect(spy).not.toHaveBeenCalled() - }) }) diff --git a/__tests__/guards/navigationGuards.spec.ts b/__tests__/guards/navigationGuards.spec.ts index 2ade465a..7648737b 100644 --- a/__tests__/guards/navigationGuards.spec.ts +++ b/__tests__/guards/navigationGuards.spec.ts @@ -13,18 +13,20 @@ const from = { describe('guardToPromiseFn', () => { it('calls the guard with to, from and, next', async () => { const spy = jest.fn((to, from, next) => next()) - await expect(guardToPromiseFn(spy, to, from)()).resolves + await expect(guardToPromiseFn(spy, to, from)()).resolves.toEqual(undefined) expect(spy).toHaveBeenCalledWith(to, from, expect.any(Function)) }) it('resolves if next is called with no arguments', async () => { - await expect(guardToPromiseFn((to, from, next) => next(), to, from)()) - .resolves + await expect( + guardToPromiseFn((to, from, next) => next(), to, from)() + ).resolves.toEqual(undefined) }) it('resolves if next is called with true', async () => { - await expect(guardToPromiseFn((to, from, next) => next(true), to, from)()) - .resolves + await expect( + guardToPromiseFn((to, from, next) => next(true), to, from)() + ).resolves.toEqual(undefined) }) it('rejects if next is called with false', async () => { diff --git a/__tests__/router.spec.ts b/__tests__/router.spec.ts index 74355d64..361ac320 100644 --- a/__tests__/router.spec.ts +++ b/__tests__/router.spec.ts @@ -1,6 +1,6 @@ import fakePromise from 'faked-promise' import { createRouter, createMemoryHistory, createWebHistory } from '../src' -import { ErrorTypes } from '../src/errors' +import { NavigationFailureType } from '../src/errors' import { createDom, components, tick } from './utils' import { RouteRecordRaw, @@ -313,37 +313,37 @@ describe('Router', () => { describe('navigation cancelled', () => { async function checkNavigationCancelledOnPush( - target?: RouteLocationRaw | false | ((vm: any) => void) + target?: RouteLocationRaw | false | ((vm: any) => any) ) { const [p1, r1] = fakePromise() - const [p2, r2] = fakePromise() const history = createMemoryHistory() const router = createRouter({ history, routes }) router.beforeEach(async (to, from, next) => { if (to.name !== 'Param') return next() + // the first navigation gets passed target if (to.params.p === 'a') { await p1 - // @ts-ignore: for some reason it's not handling the string here - target == null ? next() : next(target) + target ? next(target) : next() } else { - await p2 + // the second one just passes next() } }) + const from = router.currentRoute.value const pA = router.push('/p/a') - const pB = router.push('/p/b') // we resolve the second navigation first then the first one // and the first navigation should be ignored because at that time // the second one will have already been resolved - r2() - await pB + await expect(router.push('/p/b')).resolves.toEqual(undefined) expect(router.currentRoute.value.fullPath).toBe('/p/b') r1() - try { - await pA - } catch (err) { - expect(err.type).toBe(ErrorTypes.NAVIGATION_CANCELLED) - } + await expect(pA).resolves.toEqual( + expect.objectContaining({ + to: expect.objectContaining({ path: '/p/a' }), + from, + type: NavigationFailureType.cancelled, + }) + ) expect(router.currentRoute.value.fullPath).toBe('/p/b') } @@ -445,7 +445,8 @@ describe('Router', () => { it('handles one redirect from route record', async () => { const history = createMemoryHistory() const router = createRouter({ history, routes }) - const loc = await router.push('/to-foo') + await expect(router.push('/to-foo')).resolves.toEqual(undefined) + const loc = router.currentRoute.value expect(loc.name).toBe('Foo') expect(loc.redirectedFrom).toMatchObject({ path: '/to-foo', @@ -469,7 +470,8 @@ describe('Router', () => { it('handles a double redirect from route record', async () => { const history = createMemoryHistory() const router = createRouter({ history, routes }) - const loc = await router.push('/to-foo2') + await expect(router.push('/to-foo2')).resolves.toEqual(undefined) + const loc = router.currentRoute.value expect(loc.name).toBe('Foo') expect(loc.redirectedFrom).toMatchObject({ path: '/to-foo2', @@ -479,7 +481,10 @@ describe('Router', () => { it('drops query and params on redirect if not provided', async () => { const history = createMemoryHistory() const router = createRouter({ history, routes }) - const loc = await router.push('/to-foo?hey=foo#fa') + await expect(router.push('/to-foo?hey=foo#fa')).resolves.toEqual( + undefined + ) + const loc = router.currentRoute.value expect(loc.name).toBe('Foo') expect(loc.query).toEqual({}) expect(loc.hash).toBe('') @@ -491,7 +496,8 @@ describe('Router', () => { it('allows object in redirect', async () => { const history = createMemoryHistory() const router = createRouter({ history, routes }) - const loc = await router.push('/to-foo-named') + await expect(router.push('/to-foo-named')).resolves.toEqual(undefined) + const loc = router.currentRoute.value expect(loc.name).toBe('Foo') expect(loc.redirectedFrom).toMatchObject({ path: '/to-foo-named', diff --git a/src/errors.ts b/src/errors.ts index f2661260..1685921a 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -28,14 +28,18 @@ export interface MatcherError extends RouterErrorBase { currentLocation?: MatcherLocation } -export interface NavigationError extends RouterErrorBase { +export enum NavigationFailureType { + cancelled = ErrorTypes.NAVIGATION_CANCELLED, + aborted = ErrorTypes.NAVIGATION_ABORTED, +} +export interface NavigationFailure extends RouterErrorBase { type: ErrorTypes.NAVIGATION_ABORTED | ErrorTypes.NAVIGATION_CANCELLED from: RouteLocationNormalized to: RouteLocationNormalized } export interface NavigationRedirectError - extends Omit { + extends Omit { type: ErrorTypes.NAVIGATION_GUARD_REDIRECT to: RouteLocationRaw } @@ -57,18 +61,16 @@ const ErrorTypeMessages = { to )}" via a navigation guard` }, - [ErrorTypes.NAVIGATION_ABORTED]({ from, to }: NavigationError) { + [ErrorTypes.NAVIGATION_ABORTED]({ from, to }: NavigationFailure) { return `Navigation aborted from "${from.fullPath}" to "${to.fullPath}" via a navigation guard` }, - [ErrorTypes.NAVIGATION_CANCELLED]({ from, to }: NavigationError) { + [ErrorTypes.NAVIGATION_CANCELLED]({ from, to }: NavigationFailure) { return `Navigation cancelled from "${from.fullPath}" to "${to.fullPath}" with a new \`push\` or \`replace\`` }, } // Possible internal errors -type RouterError = NavigationError | NavigationRedirectError | MatcherError -// Public errors, TBD -// export type PublicRouterError = NavigationError +type RouterError = NavigationFailure | NavigationRedirectError | MatcherError export function createRouterError( type: E['type'], diff --git a/src/index.ts b/src/index.ts index 6849db0a..8098ba7a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -34,6 +34,8 @@ export { ScrollBehavior, } from './router' +export { NavigationFailureType, NavigationFailure } from './errors' + export { onBeforeRouteLeave } from './navigationGuards' export { Link, useLink } from './components/Link' export { View } from './components/View' diff --git a/src/navigationGuards.ts b/src/navigationGuards.ts index 119a0685..2bbd0ea0 100644 --- a/src/navigationGuards.ts +++ b/src/navigationGuards.ts @@ -11,7 +11,7 @@ import { import { createRouterError, ErrorTypes, - NavigationError, + NavigationFailure, NavigationRedirectError, } from './errors' import { ComponentPublicInstance } from 'vue' @@ -63,10 +63,13 @@ export function guardToPromiseFn< ) => { if (valid === false) reject( - createRouterError(ErrorTypes.NAVIGATION_ABORTED, { - from, - to, - }) + createRouterError( + ErrorTypes.NAVIGATION_ABORTED, + { + from, + to, + } + ) ) else if (valid instanceof Error) { reject(valid) diff --git a/src/router.ts b/src/router.ts index 4e2ffa37..3278d66d 100644 --- a/src/router.ts +++ b/src/router.ts @@ -23,7 +23,12 @@ import { getSavedScroll, } from './utils/scroll' import { createRouterMatcher } from './matcher' -import { createRouterError, ErrorTypes, NavigationError } from './errors' +import { + createRouterError, + ErrorTypes, + NavigationFailure, + NavigationRedirectError, +} from './errors' import { applyToParams, isSameRouteRecord, @@ -95,8 +100,8 @@ export interface Router { resolve(to: RouteLocationRaw): RouteLocation & { href: string } - push(to: RouteLocationRaw): Promise - replace(to: RouteLocationRaw): Promise + push(to: RouteLocationRaw): Promise + replace(to: RouteLocationRaw): Promise beforeEach(guard: NavigationGuard): () => void afterEach(guard: PostNavigationGuard): () => void @@ -151,7 +156,6 @@ export function createRouter({ if (recordMatcher) { matcher.removeRoute(recordMatcher) } else if (__DEV__) { - // TODO: adapt if we allow Symbol as a name warn(`Cannot remove non-existent route "${String(name)}"`) } } @@ -224,7 +228,7 @@ export function createRouter({ } } - function push(to: RouteLocationRaw | RouteLocation): Promise { + function push(to: RouteLocationRaw | RouteLocation) { return pushWithRedirect(to, undefined) } @@ -236,17 +240,14 @@ export function createRouter({ async function pushWithRedirect( to: RouteLocationRaw | RouteLocation, redirectedFrom: RouteLocation | undefined - ): Promise { - const targetLocation: RouteLocation = (pendingLocation = - // Some functions will pass a normalized location and we don't need to resolve it again - typeof to === 'object' && 'matched' in to ? to : resolve(to)) + ): Promise { + const targetLocation: RouteLocation = (pendingLocation = resolve(to)) const from = currentRoute.value const data: HistoryState | undefined = (to as any).state // @ts-ignore: no need to check the string as force do not exist on a string const force: boolean | undefined = to.force - // TODO: should we throw an error as the navigation was aborted - if (!force && isSameRouteLocation(from, targetLocation)) return from + if (!force && isSameRouteLocation(from, targetLocation)) return const lastMatched = targetLocation.matched[targetLocation.matched.length - 1] @@ -263,41 +264,50 @@ export function createRouter({ const toLocation = targetLocation as RouteLocationNormalized toLocation.redirectedFrom = redirectedFrom + let failure: NavigationFailure | void // trigger all guards, throw if navigation is rejected try { await navigate(toLocation, from) } catch (error) { - // push was called while waiting in guards - // TODO: write tests + // a more recent navigation took place if (pendingLocation !== toLocation) { - triggerError( - createRouterError(ErrorTypes.NAVIGATION_CANCELLED, { + failure = createRouterError( + ErrorTypes.NAVIGATION_CANCELLED, + { from, to: toLocation, - }) + } ) - } - - if (error.type === ErrorTypes.NAVIGATION_GUARD_REDIRECT) { + } else if (error.type === ErrorTypes.NAVIGATION_ABORTED) { + failure = error as NavigationFailure + } else if (error.type === ErrorTypes.NAVIGATION_GUARD_REDIRECT) { // preserve the original redirectedFrom if any - return pushWithRedirect(error.to, redirectedFrom || toLocation) + return pushWithRedirect( + (error as NavigationRedirectError).to, + redirectedFrom || toLocation + ) + } else { + // unknown error, throws + triggerError(error, true) } - - // unknown error, throws - triggerError(error) } - finalizeNavigation( - toLocation as RouteLocationNormalizedLoaded, - from, - true, - // RouteLocationNormalized will give undefined - (to as RouteLocationRaw).replace === true, - data - ) + // if we fail we don't finalize the navigation + failure = + failure || + finalizeNavigation( + toLocation as RouteLocationNormalizedLoaded, + from, + true, + // RouteLocationNormalized will give undefined + (to as RouteLocationRaw).replace === true, + data + ) + + triggerAfterEach(toLocation as RouteLocationNormalizedLoaded, from, failure) - return currentRoute.value + return failure } async function navigate( @@ -379,14 +389,16 @@ export function createRouter({ // run the queue of per route beforeEnter guards await runGuardQueue(guards) + } - // TODO: add tests - // this should be done only if the navigation succeeds - // if we redirect, it shouldn't be done because we don't know - for (const record of leavingRecords as RouteRecordNormalized[]) { - // free the references - record.instances = {} - } + function triggerAfterEach( + to: RouteLocationNormalizedLoaded, + from: RouteLocationNormalizedLoaded, + failure?: NavigationFailure | void + ): void { + // navigation is confirmed, call afterGuards + // TODO: wrap with error handlers + for (const guard of afterGuards.list()) guard(to, from, failure) } /** @@ -400,22 +412,26 @@ export function createRouter({ isPush: boolean, replace?: boolean, data?: HistoryState - ) { + ): NavigationFailure | void { // a more recent navigation took place if (pendingLocation !== toLocation) { - return triggerError( - createRouterError(ErrorTypes.NAVIGATION_CANCELLED, { + return createRouterError( + ErrorTypes.NAVIGATION_CANCELLED, + { from, to: toLocation, - }), - isPush + } ) } - // remove registered guards from removed matched records const [leavingRecords] = extractChangingRecords(toLocation, from) for (const record of leavingRecords) { + // remove registered guards from removed matched records record.leaveGuards = [] + // free the references + + // TODO: add tests + record.instances = {} } // only consider as push if it's not the first navigation @@ -438,10 +454,7 @@ export function createRouter({ toLocation, from, savedScroll || (state && state.scroll) - ).catch(err => triggerError(err, false)) - - // navigation is confirmed, call afterGuards - for (const guard of afterGuards.list()) guard(toLocation, from) + ).catch(err => triggerError(err)) markAsReady() } @@ -457,39 +470,52 @@ export function createRouter({ saveScrollOnLeave(getScrollKey(from.fullPath, info.distance)) + let failure: NavigationFailure | void + try { + // TODO: refactor using then/catch because no need for async/await + try catch await navigate(toLocation, from) - finalizeNavigation( - // after navigation, all matched components are resolved - toLocation as RouteLocationNormalizedLoaded, - from, - false - ) } catch (error) { - // if a different navigation is happening, abort this one + // a more recent navigation took place if (pendingLocation !== toLocation) { - return triggerError( - createRouterError(ErrorTypes.NAVIGATION_CANCELLED, { + failure = createRouterError( + ErrorTypes.NAVIGATION_CANCELLED, + { from, to: toLocation, - }), - false + } ) - } - - if (error.type === ErrorTypes.NAVIGATION_GUARD_REDIRECT) { + } else if (error.type === ErrorTypes.NAVIGATION_ABORTED) { + failure = error as NavigationFailure + } else if (error.type === ErrorTypes.NAVIGATION_GUARD_REDIRECT) { + history.go(-info.distance, false) // the error is already handled by router.push we just want to avoid // logging the error - return pushWithRedirect(error.to, toLocation).catch(() => {}) - } - - // TODO: test on different browsers ensure consistent behavior - history.go(-info.distance, false) - // unrecognized error, transfer to the global handler - if (error.type !== ErrorTypes.NAVIGATION_ABORTED) { - triggerError(error, false) + return pushWithRedirect( + (error as NavigationRedirectError).to, + toLocation + ).catch(() => {}) + } else { + // TODO: test on different browsers ensure consistent behavior + history.go(-info.distance, false) + // unrecognized error, transfer to the global handler + return triggerError(error) } } + + failure = + failure || + finalizeNavigation( + // after navigation, all matched components are resolved + toLocation as RouteLocationNormalizedLoaded, + from, + false + ) + + // revert the navigation + if (failure) history.go(-info.distance, false) + + triggerAfterEach(toLocation as RouteLocationNormalizedLoaded, from, failure) }) // Initialization and Errors @@ -501,9 +527,10 @@ export function createRouter({ /** * Trigger errorHandlers added via onError and throws the error as well * @param error - error to throw - * @param shouldThrow - defaults to true. Pass false to not throw the error + * @param shouldThrow - defaults to false. Pass true rethrow the error + * @returns the error (unless shouldThrow is true) */ - function triggerError(error: any, shouldThrow: boolean = true): void { + function triggerError(error: any, shouldThrow: boolean = false): void { markAsReady(error) errorHandlers.list().forEach(handler => handler(error)) if (shouldThrow) throw error diff --git a/src/types/index.ts b/src/types/index.ts index 778a9913..5edb6728 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -277,7 +277,7 @@ export interface NavigationGuardCallback { (error: Error): void (location: RouteLocationRaw): void (valid: boolean): void - (cb: (vm: any) => void): void + (cb: NavigationGuardNextCallback): void } export type NavigationGuardNextCallback = (vm: any) => any @@ -293,7 +293,12 @@ export interface NavigationGuard { } export interface PostNavigationGuard { - (to: RouteLocationNormalized, from: RouteLocationNormalized): any + ( + to: RouteLocationNormalized, + from: RouteLocationNormalized, + // TODO: move these types to a different file + failure?: TODO + ): any } export * from './type-guards' diff --git a/src/utils/scroll.ts b/src/utils/scroll.ts index 7e50c6b9..950c1410 100644 --- a/src/utils/scroll.ts +++ b/src/utils/scroll.ts @@ -63,7 +63,7 @@ export function scrollToPosition(position: ScrollPosition): void { } } - if (normalizedPosition) { + if (isBrowser && normalizedPosition) { window.scrollTo(normalizedPosition.x, normalizedPosition.y) } } @@ -81,7 +81,7 @@ export function getScrollKey(path: string, distance: number): string { } export function saveScrollOnLeave(key: string) { - scrollPositions.set(key, computeScrollPosition()) + scrollPositions.set(key, isBrowser ? computeScrollPosition() : { x: 0, y: 0 }) } export function getSavedScroll(key: string) {