feat(router): handle cancelled navigations

diff --git a/__tests__/router.spec.js b/__tests__/router.spec.js
index 6fc4beb252334f24ca7daba5f0022d21b1e23dc7..8c8ae1ebf91806e7d7d50a06db6ff7adf8f63174 100644 (file)
--- a/__tests__/router.spec.js
+++ b/__tests__/router.spec.js
@@ -1,6 +1,7 @@
 // @ts-check
 require('./helper')
 const expect = require('expect')
+const fakePromise = require('faked-promise')
 const { HTML5History } = require('../src/history/html5')
 const { Router } = require('../src/router')
 const { createDom, components } = require('./utils')
@@ -74,6 +75,50 @@ describe('Router', () => {
     })
   })
 
+  describe('navigation', () => {
+    it('waits before navigating in an array of beforeEnter', async () => {
+      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) {
+              await p1
+              next()
+            },
+          },
+          {
+            path: '/b',
+            component: components.Foo,
+            name: 'Foo',
+            async beforeEnter(to, from, next) {
+              await p2
+              next()
+            },
+          },
+        ],
+      })
+      const pA = router.push('/a')
+      const pB = router.push('/b')
+      // we resolve the second navigation first then the first one
+      // and the first navigation should be ignored
+      r2()
+      await pB
+      expect(router.currentRoute.fullPath).toBe('/b')
+      r1()
+      try {
+        await pA
+      } catch (err) {
+        // TODO: expect error
+      }
+      expect(router.currentRoute.fullPath).toBe('/b')
+    })
+  })
+
   describe('matcher', () => {
     it('handles one redirect from route record', async () => {
       const history = mockHistory()

diff --git a/src/errors.ts b/src/errors.ts
index d95a45fadb42a31fa768786361956ec2115e9a6d..60666d4353f530af9454c6ee05ac551a49ab2a0c 100644 (file)
--- a/src/errors.ts
+++ b/src/errors.ts
@@ -42,6 +42,45 @@ export class NavigationGuardRedirect extends Error {
   }
 }
 
+/**
+ * Navigation aborted by next(false)
+ */
+export class NavigationAborted extends Error {
+  to: RouteLocationNormalized
+  from: RouteLocationNormalized
+  constructor(to: RouteLocationNormalized, from: RouteLocationNormalized) {
+    super(
+      `Navigation aborted from "${from.fullPath}" to "${
+        to.fullPath
+      }" via a navigation guard`
+    )
+    Object.setPrototypeOf(this, new.target.prototype)
+
+    this.from = from
+    this.to = to
+  }
+}
+
+/**
+ * Navigation canceled by the user by pushing/replacing a new location
+ * TODO: is the name good?
+ */
+export class NavigationCancelled extends Error {
+  to: RouteLocationNormalized
+  from: RouteLocationNormalized
+  constructor(to: RouteLocationNormalized, from: RouteLocationNormalized) {
+    super(
+      `Navigation cancelled from "${from.fullPath}" to "${
+        to.fullPath
+      }" with a new \`push\` or \`replace\``
+    )
+    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

diff --git a/src/router.ts b/src/router.ts
index 9f304567c87b2449f3e532ba04c19f5c59cd6d42..a9ee23d554fd63397800afb87d1d0a2550e76543 100644 (file)
--- a/src/router.ts
+++ b/src/router.ts
@@ -19,7 +19,11 @@ import {
 } from './types/index'
 
 import { guardToPromiseFn, extractComponentsGuards } from './utils'
-import { NavigationGuardRedirect } from './errors'
+import {
+  NavigationGuardRedirect,
+  NavigationAborted,
+  NavigationCancelled,
+} from './errors'
 
 export interface RouterOptions {
   history: BaseHistory
@@ -32,6 +36,7 @@ export class Router {
   private beforeGuards: NavigationGuard[] = []
   private afterGuards: PostNavigationGuard[] = []
   currentRoute: Readonly<RouteLocationNormalized> = START_LOCATION_NORMALIZED
+  pendingLocation: Readonly<RouteLocationNormalized> = START_LOCATION_NORMALIZED
   private app: any
 
   constructor(options: RouterOptions) {
@@ -60,8 +65,17 @@ export class Router {
         // preserve history when moving forward
         if (error instanceof NavigationGuardRedirect) {
           this.push(error.to)
+        } else if (error instanceof NavigationAborted) {
+          // TODO: test on different browsers ensure consistent behavior
+          if (info.direction === NavigationDirection.back) {
+            this.history.forward(false)
+          } 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)
+          }
         } else {
-          // TODO: handle abort and redirect correctly
           // if we were going back, we push and discard the rest of the history
           if (info.direction === NavigationDirection.back) {
             this.history.push(from)
@@ -193,11 +207,16 @@ export class Router {
       return this.currentRoute
 
     const toLocation: RouteLocationNormalized = location
+    this.pendingLocation = toLocation
     // trigger all guards, throw if navigation is rejected
     try {
       await this.navigate(toLocation, this.currentRoute)
     } catch (error) {
       if (error instanceof NavigationGuardRedirect) {
+        // push was called while waiting in guards
+        if (this.pendingLocation !== toLocation) {
+          throw new NavigationCancelled(toLocation, this.currentRoute)
+        }
         // TODO: setup redirect stack
         return this.push(error.to)
       } else {
@@ -205,6 +224,11 @@ export class Router {
       }
     }
 
+    // push was called while waiting in guards
+    if (this.pendingLocation !== toLocation) {
+      throw new NavigationCancelled(toLocation, this.currentRoute)
+    }
+
     // change URL
     if (to.replace === true) this.history.replace(url)
     else this.history.push(url)

diff --git a/src/utils/guardToPromiseFn.ts b/src/utils/guardToPromiseFn.ts
index 4d67744fe753bb3407f92fd4ac7b0454a230b0fd..6493a6d2a20a8587910540aad8bd19c4df607bbf 100644 (file)
--- a/src/utils/guardToPromiseFn.ts
+++ b/src/utils/guardToPromiseFn.ts
@@ -6,7 +6,7 @@ import {
 } from '../types'
 
 import { isRouteLocation } from './index'
-import { NavigationGuardRedirect } from '../errors'
+import { NavigationGuardRedirect, NavigationAborted } from '../errors'
 
 export function guardToPromiseFn(
   guard: NavigationGuard,
@@ -18,9 +18,8 @@ export function guardToPromiseFn(
       const next: NavigationGuardCallback = (
         valid?: boolean | RouteLocation
       ) => {
-        // TODO: better error
         // TODO: handle callback
-        if (valid === false) reject(new Error('Aborted'))
+        if (valid === false) reject(new NavigationAborted(to, from))
         else if (isRouteLocation(valid)) {
           reject(new NavigationGuardRedirect(to, valid))
         } else resolve()