From: Eduardo San Martin Morote Date: Tue, 11 Jun 2019 15:08:08 +0000 (+0200) Subject: fix(history): cancel old navigations X-Git-Tag: v4.0.0-alpha.0~340 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4c5cb52f65c46e92dbcfdfe7103fcfc7665b3ee0;p=thirdparty%2Fvuejs%2Frouter.git fix(history): cancel old navigations --- diff --git a/__tests__/router.spec.js b/__tests__/router.spec.js index 224748f6..da7f2256 100644 --- a/__tests__/router.spec.js +++ b/__tests__/router.spec.js @@ -19,7 +19,7 @@ const routes = [ { path: '/to-foo', redirect: '/foo' }, { path: '/to-foo-named', redirect: { name: 'Foo' } }, { path: '/to-foo2', redirect: '/to-foo' }, - { path: '/p/:p', component: components.Bar }, + { path: '/p/:p', name: 'Param', component: components.Bar }, { path: '/to-p/:p', redirect: to => `/p/${to.params.p}` }, { path: '/inc-query-hash', @@ -77,110 +77,68 @@ describe('Router', () => { }) describe('navigation', () => { - it('cancels pending navigations if a newer one is finished on push', async () => { + async function checkNavigationCancelledOnPush(target, expectation) { const [p1, r1] = fakePromise() const [p2, r2] = fakePromise() const history = mockHistory() - const router = new Router({ - history, - routes: [ - { - path: '/a', - component: components.Home, - async beforeEnter(to, from, next) { - expect(from.fullPath).toBe('/') - await p1 - next() - }, - }, - { - path: '/b', - component: components.Foo, - name: 'Foo', - async beforeEnter(to, from, next) { - expect(from.fullPath).toBe('/') - await p2 - next() - }, - }, - ], + const router = new Router({ history, routes }) + router.beforeEach(async (to, from, next) => { + if (to.name !== 'Param') return next() + if (to.params.p === 'a') { + await p1 + next(target) + } else { + await p2 + next() + } }) - const pA = router.push('/a') - const pB = router.push('/b') + 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 + // and the first navigation should be ignored because at that time + // the second one will have already been resolved r2() await pB - expect(router.currentRoute.fullPath).toBe('/b') + expect(router.currentRoute.fullPath).toBe(expectation) r1() try { await pA } catch (err) { // TODO: expect error } - expect(router.currentRoute.fullPath).toBe('/b') - }) + expect(router.currentRoute.fullPath).toBe(expectation) + } - it('cancels pending navigations if a newer one is finished on user navigation (from history)', async () => { - const [p1, r1] = fakePromise() - const [p2, r2] = fakePromise() - const history = new AbstractHistory() - const router = new Router({ history, routes }) - // navigate first to add entries to the history stack - await router.push('/p/initial') - await router.push('/p/a') - await router.push('/p/b') - - router.beforeEach(async (to, from, next) => { - if (to.fullPath === '/p/initial') { - // because we delay navigation, we are coming from /p/b - expect(from.fullPath).toBe('/p/b') - await p1 - } else { - expect(from.fullPath).toBe('/p/b') - await p2 - } - next() - }) + it('cancels navigation abort if a newer one is finished on push', async () => { + await checkNavigationCancelledOnPush(false, '/p/b') + }) - // trigger to history.back() - history.back() - history.back() + it('cancels pending in-guard navigations if a newer one is finished on push', async () => { + await checkNavigationCancelledOnPush('/foo', '/p/b') + }) - expect(router.currentRoute.fullPath).toBe('/p/b') - // resolves the last call to history.back() first - // so we end up on /p/initial - r1() - await tick() - expect(router.currentRoute.fullPath).toBe('/p/initial') - // resolves the pending navigation, this should be cancelled - r2() - await tick() - expect(router.currentRoute.fullPath).toBe('/p/initial') + it('cancels pending navigations if a newer one is finished on push', async () => { + await checkNavigationCancelledOnPush(undefined, '/p/b') }) - it('cancels pending in-guard navigations if a newer one is finished on user navigation (from history)', async () => { + async function checkNavigationCancelledOnPopstate(target) { const [p1, r1] = fakePromise() const [p2, r2] = fakePromise() const history = new AbstractHistory() const router = new Router({ history, routes }) // navigate first to add entries to the history stack - await router.push('/p/initial') + await router.push('/foo') await router.push('/p/a') await router.push('/p/b') router.beforeEach(async (to, from, next) => { - console.log('going to', to.fullPath, 'from', from.fullPath) - if (to.fullPath === '/p/initial') { - console.log('waiting for p1') + if (to.name !== 'Param') return next() + if (to.fullPath === '/foo') { await p1 - console.log('done with p1') next() } else if (from.fullPath === '/p/b') { - console.log('waiting for p2') await p2 - console.log('done with p2') - next('/p/other-place') + next(target) } else { next() } @@ -195,11 +153,23 @@ describe('Router', () => { // so we end up on /p/initial r1() await tick() - expect(router.currentRoute.fullPath).toBe('/p/initial') + expect(router.currentRoute.fullPath).toBe('/foo') // resolves the pending navigation, this should be cancelled r2() await tick() - expect(router.currentRoute.fullPath).toBe('/p/initial') + expect(router.currentRoute.fullPath).toBe('/foo') + } + + it('cancels pending navigations if a newer one is finished on user navigation (from history)', async () => { + await checkNavigationCancelledOnPopstate(undefined) + }) + + it('cancels pending in-guard navigations if a newer one is finished on user navigation (from history)', async () => { + await checkNavigationCancelledOnPopstate('/p/other-place') + }) + + it('cancels navigation abort if a newer one is finished on user navigation (from history)', async () => { + await checkNavigationCancelledOnPush(undefined, '/p/b') }) }) diff --git a/src/router.ts b/src/router.ts index dd7c8b74..f04fadc5 100644 --- a/src/router.ts +++ b/src/router.ts @@ -40,6 +40,7 @@ export class Router { currentRoute: Readonly = START_LOCATION_NORMALIZED pendingLocation: Readonly = START_LOCATION_NORMALIZED private app: any + // TODO: should these be triggered before or after route.push().catch() private errorHandlers: ErrorHandler[] = [] constructor(options: RouterOptions) { @@ -95,15 +96,8 @@ export class Router { this.history.back(false) } } else { - // if we were going back, we push and discard the rest of the history - if (info.direction === NavigationDirection.back) { - this.history.push(from) - } else { - // TODO: go back because we cancelled, then - // or replace and not discard the rest of history. Check issues, there was one talking about this - // behaviour, maybe we can do better - this.history.back(false) - } + // TODO: write test + this.triggerError(error) } } }) @@ -234,11 +228,14 @@ export class Router { if (error instanceof NavigationGuardRedirect) { // push was called while waiting in guards if (this.pendingLocation !== toLocation) { + // TODO: trigger onError as well throw new NavigationCancelled(toLocation, this.currentRoute) } // TODO: setup redirect stack return this.push(error.to) } else { + // TODO: write tests + // triggerError as well throw error } }