From: Eduardo San Martin Morote Date: Thu, 2 Jul 2020 18:02:05 +0000 (+0200) Subject: feat(guards): next callback beforeRouteEnter X-Git-Tag: v4.0.0-beta.1~10 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d9dad0b9467fee9478406899043ee35f30cdf1fb;p=thirdparty%2Fvuejs%2Frouter.git feat(guards): next callback beforeRouteEnter --- diff --git a/__tests__/RouterView.spec.ts b/__tests__/RouterView.spec.ts index b7805d37..a23c5dd4 100644 --- a/__tests__/RouterView.spec.ts +++ b/__tests__/RouterView.spec.ts @@ -39,6 +39,7 @@ const routes = createRoutes({ { components: { default: components.Home }, instances: {}, + enterCallbacks: [], path: '/', props, }, @@ -56,6 +57,7 @@ const routes = createRoutes({ { components: { default: components.Foo }, instances: {}, + enterCallbacks: [], path: '/foo', props, }, @@ -73,12 +75,14 @@ const routes = createRoutes({ { components: { default: components.Nested }, instances: {}, + enterCallbacks: [], path: '/', props, }, { components: { default: components.Foo }, instances: {}, + enterCallbacks: [], path: 'a', props, }, @@ -96,18 +100,21 @@ const routes = createRoutes({ { components: { default: components.Nested }, instances: {}, + enterCallbacks: [], path: '/', props, }, { components: { default: components.Nested }, instances: {}, + enterCallbacks: [], path: 'a', props, }, { components: { default: components.Foo }, instances: {}, + enterCallbacks: [], path: 'b', props, }, @@ -122,7 +129,13 @@ const routes = createRoutes({ hash: '', meta: {}, matched: [ - { components: { foo: components.Foo }, instances: {}, path: '/', props }, + { + components: { foo: components.Foo }, + instances: {}, + enterCallbacks: [], + path: '/', + props, + }, ], }, withParams: { @@ -138,6 +151,7 @@ const routes = createRoutes({ components: { default: components.User }, instances: {}, + enterCallbacks: [], path: '/users/:id', props: { default: true }, }, @@ -156,6 +170,7 @@ const routes = createRoutes({ components: { default: components.WithProps }, instances: {}, + enterCallbacks: [], path: '/props/:id', props: { default: { id: 'foo', other: 'fixed' } }, }, @@ -175,6 +190,7 @@ const routes = createRoutes({ components: { default: components.WithProps }, instances: {}, + enterCallbacks: [], path: '/props/:id', props: { default: (to: RouteLocationNormalized) => ({ @@ -247,6 +263,7 @@ describe('RouterView', () => { { components: { default: components.User }, instances: {}, + enterCallbacks: [], path: '/users/:id', props, }, diff --git a/__tests__/errors.spec.ts b/__tests__/errors.spec.ts index 14139cf0..dd868bdd 100644 --- a/__tests__/errors.spec.ts +++ b/__tests__/errors.spec.ts @@ -101,8 +101,8 @@ describe('Errors & Navigation failures', () => { // should hang let navigationPromise = router.push('/foo') + expect(afterEach).toHaveBeenCalledTimes(0) await expect(router.push('/')).resolves.toEqual(undefined) - expect(afterEach).toHaveBeenCalledTimes(1) expect(onError).toHaveBeenCalledTimes(0) resolve() @@ -110,7 +110,8 @@ describe('Errors & Navigation failures', () => { expect(afterEach).toHaveBeenCalledTimes(2) expect(onError).toHaveBeenCalledTimes(0) - expect(afterEach).toHaveBeenLastCalledWith( + expect(afterEach).toHaveBeenNthCalledWith( + 1, expect.objectContaining({ path: '/foo' }), from, expect.objectContaining({ type: NavigationFailureType.cancelled }) @@ -159,18 +160,12 @@ describe('Errors & Navigation failures', () => { let navigationPromise = router.push('/bar') // goes from /foo to / + expect(afterEach).toHaveBeenCalledTimes(0) 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 }) @@ -179,11 +174,19 @@ describe('Errors & Navigation failures', () => { expect(afterEach).toHaveBeenCalledTimes(2) expect(onError).toHaveBeenCalledTimes(0) - expect(afterEach).toHaveBeenLastCalledWith( + expect(afterEach).toHaveBeenNthCalledWith( + 1, expect.objectContaining({ path: '/bar' }), from, expect.objectContaining({ type: NavigationFailureType.cancelled }) ) + + expect(afterEach).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ path: '/' }), + from, + undefined + ) }) it('next(false) triggers afterEach with history.back', async () => { diff --git a/__tests__/guards/beforeRouteEnter.spec.ts b/__tests__/guards/beforeRouteEnter.spec.ts index 01092274..1a0a3b7e 100644 --- a/__tests__/guards/beforeRouteEnter.spec.ts +++ b/__tests__/guards/beforeRouteEnter.spec.ts @@ -201,34 +201,4 @@ describe('beforeRouteEnter', () => { await p expect(router.currentRoute.value.fullPath).toBe('/foo') }) - - // TODO: wait until we have something working with keep-alive and transition first - it.skip('calls next callback', async done => { - const router = createRouter({ routes }) - beforeRouteEnter.mockImplementationOnce((to, from, next) => { - next(vm => { - expect(router.currentRoute.value.fullPath).toBe('/foo') - expect(vm).toBeTruthy() - done() - }) - }) - - await router.push('/') - await router.push('/guard/2') - }) - - it.skip('calls next callback after waiting', async done => { - const [promise, resolve] = fakePromise() - const router = createRouter({ routes }) - beforeRouteEnter.mockImplementationOnce(async (to, from, next) => { - await promise - next(vm => { - expect(router.currentRoute.value.fullPath).toBe('/foo') - expect(vm).toBeTruthy() - done() - }) - }) - router.push('/foo') - resolve() - }) }) diff --git a/__tests__/matcher/records.spec.ts b/__tests__/matcher/records.spec.ts index bc78ec17..3ff0aa4b 100644 --- a/__tests__/matcher/records.spec.ts +++ b/__tests__/matcher/records.spec.ts @@ -6,7 +6,7 @@ describe('normalizeRouteRecord', () => { path: '/home', component: {}, }) - expect(record).toEqual({ + expect(record).toMatchObject({ beforeEnter: undefined, children: [], aliasOf: undefined, @@ -31,7 +31,7 @@ describe('normalizeRouteRecord', () => { name: 'name', component: {}, }) - expect(record).toEqual({ + expect(record).toMatchObject({ beforeEnter, children: [{ path: '/child' }], components: { default: {} }, @@ -73,7 +73,7 @@ describe('normalizeRouteRecord', () => { name: 'name', components: { one: {} }, }) - expect(record).toEqual({ + expect(record).toMatchObject({ beforeEnter, children: [{ path: '/child' }], components: { one: {} }, diff --git a/__tests__/utils.ts b/__tests__/utils.ts index d93270fc..719823ed 100644 --- a/__tests__/utils.ts +++ b/__tests__/utils.ts @@ -53,6 +53,7 @@ export interface RouteRecordViewLoose > { leaveGuards?: any instances: Record + enterCallbacks: Function[] props: Record aliasOf: RouteRecordViewLoose | undefined } diff --git a/e2e/keep-alive/index.ts b/e2e/keep-alive/index.ts index 12d2cab2..a6705f78 100644 --- a/e2e/keep-alive/index.ts +++ b/e2e/keep-alive/index.ts @@ -21,12 +21,19 @@ const Foo: RouteComponent = { template: '
foo
' } const WithGuards: RouteComponent = { template: `
+

Enter Count {{ enterCount }}

Update Count {{ updateCount }}

Leave Count {{ leaveCount }}

`, + beforeRouteEnter(to, from, next) { + next(vm => { + ;(vm as any).enterCount++ + }) + }, + beforeRouteUpdate(to, from, next) { this.updateCount++ next() @@ -37,11 +44,13 @@ const WithGuards: RouteComponent = { }, setup() { + const enterCount = ref(0) const updateCount = ref(0) const leaveCount = ref(0) const router = useRouter() function reset() { + enterCount.value = 0 updateCount.value = 0 leaveCount.value = 0 } @@ -52,6 +61,7 @@ const WithGuards: RouteComponent = { return { reset, changeQuery, + enterCount, updateCount, leaveCount, } diff --git a/e2e/specs/keep-alive.js b/e2e/specs/keep-alive.js index a9e1c4e6..3f0e6bbc 100644 --- a/e2e/specs/keep-alive.js +++ b/e2e/specs/keep-alive.js @@ -21,8 +21,10 @@ module.exports = { .assert.containsText('#counter', '1') .click('li:nth-child(3) a') + .assert.containsText('#enter-count', '1') .assert.containsText('#update-count', '0') .click('#change-query') + .assert.containsText('#enter-count', '1') .assert.containsText('#update-count', '1') .back() .assert.containsText('#update-count', '2') @@ -30,6 +32,7 @@ module.exports = { .back() .assert.containsText('#counter', '1') .forward() + .assert.containsText('#enter-count', '2') .assert.containsText('#update-count', '2') .assert.containsText('#leave-count', '1') diff --git a/src/RouterView.ts b/src/RouterView.ts index 74ed892d..1041cfc4 100644 --- a/src/RouterView.ts +++ b/src/RouterView.ts @@ -75,7 +75,9 @@ export const RouterViewImpl = defineComponent({ const currentName = props.name const onVnodeMounted = () => { matchedRoute.instances[currentName] = viewRef.value - // TODO: trigger beforeRouteEnter hooks + matchedRoute.enterCallbacks.forEach(callback => + callback(viewRef.value!) + ) } const onVnodeUnmounted = () => { // remove the instance reference to prevent leak diff --git a/src/errors.ts b/src/errors.ts index fce067c2..1128d599 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -5,18 +5,25 @@ import { RouteLocationNormalized, } from './types' import { assign } from './utils' +import { PolySymbol } from './injectionSymbols' /** - * order is important to make it backwards compatible with v3 + * Flags so we can combine them when checking for multiple errors */ export const enum ErrorTypes { - MATCHER_NOT_FOUND = 0, - NAVIGATION_GUARD_REDIRECT = 1, - NAVIGATION_ABORTED = 2, - NAVIGATION_CANCELLED = 3, - NAVIGATION_DUPLICATED = 4, + // they must be literals to be used as values so we can't write + // 1 << 2 + MATCHER_NOT_FOUND = 1, + NAVIGATION_GUARD_REDIRECT = 2, + NAVIGATION_ABORTED = 4, + NAVIGATION_CANCELLED = 8, + NAVIGATION_DUPLICATED = 16, } +const NavigationFailureSymbol = PolySymbol( + __DEV__ ? 'navigation failure' : 'nf' +) + interface RouterErrorBase extends Error { type: ErrorTypes } @@ -85,14 +92,42 @@ export function createRouterError( if (__DEV__ || !__BROWSER__) { return assign( new Error(ErrorTypeMessages[type](params as any)), - { type }, + { + type, + [NavigationFailureSymbol]: true, + } as { type: typeof type }, params ) as E } else { - return assign(new Error(), { type }, params) as E + return assign( + new Error(), + { + type, + [NavigationFailureSymbol]: true, + } as { type: typeof type }, + params + ) as E } } +export function isNavigationFailure( + error: any, + type: ErrorTypes.NAVIGATION_GUARD_REDIRECT +): error is NavigationRedirectError +export function isNavigationFailure( + error: any, + type: ErrorTypes +): error is NavigationFailure +export function isNavigationFailure( + error: any, + type?: number +): error is NavigationFailure { + return ( + NavigationFailureSymbol in error && + (type == null || !!((error as NavigationFailure).type & type)) + ) +} + const propertiesToLog = ['params', 'query', 'hash'] as const function stringifyRoute(to: RouteLocationRaw): string { diff --git a/src/matcher/index.ts b/src/matcher/index.ts index 6a7a7e6b..fb31f584 100644 --- a/src/matcher/index.ts +++ b/src/matcher/index.ts @@ -323,6 +323,7 @@ export function normalizeRouteRecord( instances: {}, leaveGuards: [], updateGuards: [], + enterCallbacks: [], components: 'components' in record ? record.components || {} diff --git a/src/matcher/types.ts b/src/matcher/types.ts index d381fcf0..cbf0c39e 100644 --- a/src/matcher/types.ts +++ b/src/matcher/types.ts @@ -3,6 +3,7 @@ import { NavigationGuard, _RouteRecordBase, _RouteRecordProps, + NavigationGuardNextCallback, } from '../types' import { ComponentPublicInstance } from 'vue' @@ -21,6 +22,7 @@ export interface RouteRecordNormalized { beforeEnter: RouteRecordMultipleViews['beforeEnter'] leaveGuards: NavigationGuard[] updateGuards: NavigationGuard[] + enterCallbacks: NavigationGuardNextCallback[] instances: Record // can only be of of the same type as this record aliasOf: RouteRecordNormalized | undefined diff --git a/src/navigationGuards.ts b/src/navigationGuards.ts index c711d830..0585a189 100644 --- a/src/navigationGuards.ts +++ b/src/navigationGuards.ts @@ -91,8 +91,11 @@ export function guardToPromiseFn( guard: NavigationGuard, to: RouteLocationNormalized, from: RouteLocationNormalizedLoaded, - instance?: ComponentPublicInstance | undefined | null + instance?: ComponentPublicInstance | undefined | null, + record?: RouteRecordNormalized ): () => Promise { + // keep a reference to the enterCallbackArray to prevent pushing callbacks if a new navigation took place + const enterCallbackArray = record && record.enterCallbacks return () => new Promise((resolve, reject) => { const next: NavigationGuardNext = ( @@ -121,8 +124,12 @@ export function guardToPromiseFn( ) ) } else { - // TODO: call the in component enter callbacks. Maybe somewhere else - // record && record.enterCallbacks.push(valid) + if ( + record && + record.enterCallbacks === enterCallbackArray && + typeof valid === 'function' + ) + enterCallbackArray.push(valid) resolve() } } @@ -182,7 +189,9 @@ export function extractComponentsGuards( (rawComponent as any).__vccOpts || rawComponent const guard = options[guardType] guard && - guards.push(guardToPromiseFn(guard, to, from, record.instances[name])) + guards.push( + guardToPromiseFn(guard, to, from, record.instances[name], record) + ) } else { // start requesting the chunk already let componentPromise: Promise = (rawComponent as Lazy< @@ -215,7 +224,13 @@ export function extractComponentsGuards( const guard: NavigationGuard = resolvedComponent[guardType] return ( guard && - guardToPromiseFn(guard, to, from, record.instances[name])() + guardToPromiseFn( + guard, + to, + from, + record.instances[name], + record + )() ) }) ) diff --git a/src/router.ts b/src/router.ts index fc0da57b..01e80b4f 100644 --- a/src/router.ts +++ b/src/router.ts @@ -29,6 +29,7 @@ import { ErrorTypes, NavigationFailure, NavigationRedirectError, + isNavigationFailure, } from './errors' import { applyToParams, isBrowser, assign } from './utils' import { useCallbacks } from './utils/callbacks' @@ -365,6 +366,21 @@ export function createRouter(options: RouterOptions): Router { return typeof to === 'string' ? { path: to } : assign({}, to) } + function checkCanceledNavigation( + to: RouteLocationNormalized, + from: RouteLocationNormalized + ): NavigationFailure | void { + if (pendingLocation !== to) { + return createRouterError( + ErrorTypes.NAVIGATION_CANCELLED, + { + from, + to, + } + ) + } + } + function push(to: RouteLocationRaw | RouteLocation) { return pushWithRedirect(to) } @@ -456,19 +472,13 @@ export function createRouter(options: RouterOptions): Router { return (failure ? Promise.resolve(failure) : navigate(toLocation, from)) .catch((error: NavigationFailure | NavigationRedirectError) => { - // a more recent navigation took place - if (pendingLocation !== toLocation) { - return createRouterError( - ErrorTypes.NAVIGATION_CANCELLED, - { - from, - to: toLocation, - } - ) - } if ( - error.type === ErrorTypes.NAVIGATION_ABORTED || - error.type === ErrorTypes.NAVIGATION_GUARD_REDIRECT + isNavigationFailure( + error, + ErrorTypes.NAVIGATION_ABORTED | + ErrorTypes.NAVIGATION_CANCELLED | + ErrorTypes.NAVIGATION_GUARD_REDIRECT + ) ) { return error } @@ -477,7 +487,9 @@ export function createRouter(options: RouterOptions): Router { }) .then((failure: NavigationFailure | NavigationRedirectError | void) => { if (failure) { - if (failure.type === ErrorTypes.NAVIGATION_GUARD_REDIRECT) + if ( + isNavigationFailure(failure, ErrorTypes.NAVIGATION_GUARD_REDIRECT) + ) // preserve the original redirectedFrom if any return pushWithRedirect( // keep options @@ -507,6 +519,21 @@ export function createRouter(options: RouterOptions): Router { }) } + /** + * Helper to reject and skip all navigation guards if a new navigation happened + * @param to + * @param from + */ + function checkCanceledNavigationAndReject( + to: RouteLocationNormalized, + from: RouteLocationNormalized + ): Promise { + const error = checkCanceledNavigation(to, from) + return error ? Promise.reject(error) : Promise.resolve() + } + + // TODO: refactor the whole before guards by internally using router.beforeEach + function navigate( to: RouteLocationNormalized, from: RouteLocationNormalizedLoaded @@ -533,77 +560,105 @@ export function createRouter(options: RouterOptions): Router { } } - // run the queue of per route beforeRouteLeave guards - return runGuardQueue(guards) - .then(() => { - // check global guards beforeEach - guards = [] - for (const guard of beforeGuards.list()) { - guards.push(guardToPromiseFn(guard, to, from)) - } + const canceledNavigationCheck = checkCanceledNavigationAndReject.bind( + null, + to, + from + ) - return runGuardQueue(guards) - }) - .then(() => { - // check in components beforeRouteUpdate - guards = extractComponentsGuards( - to.matched.filter(record => from.matched.indexOf(record as any) > -1), - 'beforeRouteUpdate', - to, - from - ) + guards.push(canceledNavigationCheck) - for (const record of updatingRecords) { - for (const guard of record.updateGuards) { + // run the queue of per route beforeRouteLeave guards + return ( + runGuardQueue(guards) + .then(() => { + // check global guards beforeEach + guards = [] + for (const guard of beforeGuards.list()) { guards.push(guardToPromiseFn(guard, to, from)) } - } + guards.push(canceledNavigationCheck) - // run the queue of per route beforeEnter guards - return runGuardQueue(guards) - }) - .then(() => { - // check the route beforeEnter - guards = [] - for (const record of to.matched) { - // do not trigger beforeEnter on reused views - if (record.beforeEnter && from.matched.indexOf(record as any) < 0) { - if (Array.isArray(record.beforeEnter)) { - for (const beforeEnter of record.beforeEnter) - guards.push(guardToPromiseFn(beforeEnter, to, from)) - } else { - guards.push(guardToPromiseFn(record.beforeEnter, to, from)) + return runGuardQueue(guards) + }) + .then(() => { + // check in components beforeRouteUpdate + guards = extractComponentsGuards( + to.matched.filter( + record => from.matched.indexOf(record as any) > -1 + ), + 'beforeRouteUpdate', + to, + from + ) + + for (const record of updatingRecords) { + for (const guard of record.updateGuards) { + guards.push(guardToPromiseFn(guard, to, from)) } } - } + guards.push(canceledNavigationCheck) - // run the queue of per route beforeEnter guards - return runGuardQueue(guards) - }) - .then(() => { - // NOTE: at this point to.matched is normalized and does not contain any () => Promise - - // check in-component beforeRouteEnter - guards = extractComponentsGuards( - // the type doesn't matter as we are comparing an object per reference - to.matched.filter(record => from.matched.indexOf(record as any) < 0), - 'beforeRouteEnter', - to, - from - ) + // run the queue of per route beforeEnter guards + return runGuardQueue(guards) + }) + .then(() => { + // check the route beforeEnter + guards = [] + for (const record of to.matched) { + // do not trigger beforeEnter on reused views + if (record.beforeEnter && from.matched.indexOf(record as any) < 0) { + if (Array.isArray(record.beforeEnter)) { + for (const beforeEnter of record.beforeEnter) + guards.push(guardToPromiseFn(beforeEnter, to, from)) + } else { + guards.push(guardToPromiseFn(record.beforeEnter, to, from)) + } + } + } + guards.push(canceledNavigationCheck) - // run the queue of per route beforeEnter guards - return runGuardQueue(guards) - }) - .then(() => { - // check global guards beforeResolve - guards = [] - for (const guard of beforeResolveGuards.list()) { - guards.push(guardToPromiseFn(guard, to, from)) - } + // run the queue of per route beforeEnter guards + return runGuardQueue(guards) + }) + .then(() => { + // NOTE: at this point to.matched is normalized and does not contain any () => Promise + + // clear existing enterCallbacks, these are added by extractComponentsGuards + to.matched.forEach(record => (record.enterCallbacks = [])) + + // check in-component beforeRouteEnter + guards = extractComponentsGuards( + // the type doesn't matter as we are comparing an object per reference + to.matched.filter( + record => from.matched.indexOf(record as any) < 0 + ), + 'beforeRouteEnter', + to, + from + ) + guards.push(canceledNavigationCheck) - return runGuardQueue(guards) - }) + // run the queue of per route beforeEnter guards + return runGuardQueue(guards) + }) + .then(() => { + // check global guards beforeResolve + guards = [] + for (const guard of beforeResolveGuards.list()) { + guards.push(guardToPromiseFn(guard, to, from)) + } + guards.push(canceledNavigationCheck) + + return runGuardQueue(guards) + }) + // catch any navigation canceled + .catch(err => + isNavigationFailure(err, ErrorTypes.NAVIGATION_CANCELLED) + ? err + : Promise.reject(err) + ) + ) } function triggerAfterEach( @@ -629,15 +684,8 @@ export function createRouter(options: RouterOptions): Router { data?: HistoryState ): NavigationFailure | void { // a more recent navigation took place - if (pendingLocation !== toLocation) { - return createRouterError( - ErrorTypes.NAVIGATION_CANCELLED, - { - from, - to: toLocation, - } - ) - } + const error = checkCanceledNavigation(toLocation, from) + if (error) return error const [leavingRecords] = extractChangingRecords(toLocation, from) for (const record of leavingRecords) { @@ -699,20 +747,17 @@ export function createRouter(options: RouterOptions): Router { navigate(toLocation, from) .catch((error: NavigationFailure | NavigationRedirectError) => { - // a more recent navigation took place - if (pendingLocation !== toLocation) { - return createRouterError( - ErrorTypes.NAVIGATION_CANCELLED, - { - from, - to: toLocation, - } + if ( + isNavigationFailure( + error, + ErrorTypes.NAVIGATION_ABORTED | ErrorTypes.NAVIGATION_CANCELLED ) + ) { + return error } - if (error.type === ErrorTypes.NAVIGATION_ABORTED) { - return error as NavigationFailure - } - if (error.type === ErrorTypes.NAVIGATION_GUARD_REDIRECT) { + if ( + isNavigationFailure(error, ErrorTypes.NAVIGATION_GUARD_REDIRECT) + ) { routerHistory.go(-info.delta, false) // the error is already handled by router.push we just want to avoid // logging the error