fix(history): cancel old navigations

diff --git a/__tests__/router.spec.js b/__tests__/router.spec.js
index 224748f6d2ed3ce57b081aaf415da9669b07f3e8..da7f2256048c711329e6e301689ecbca22a22bf4 100644 (file)
--- 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 dd7c8b7497b6a846034d45955ab29c407c69c2ac..f04fadc520e1112951229f310e618e9865fe047d 100644 (file)
--- a/src/router.ts
+++ b/src/router.ts
@@ -40,6 +40,7 @@ export class Router {
   currentRoute: Readonly<RouteLocationNormalized> = START_LOCATION_NORMALIZED
   pendingLocation: Readonly<RouteLocationNormalized> = 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
       }
     }