refactor(history): use an Error object when navigatin to the same place

Author: posva

Diff for: src/history/abstract.js

@@ -2,10 +2,11 @@
 
 import type Router from '../index'
 import { History } from './base'
+import { NavigationDuplicated } from './errors'
 
 export class AbstractHistory extends History {
-  index: number;
-  stack: Array<Route>;
+  index: number
+  stack: Array<Route>
 
   constructor (router: Router, base: ?string) {
     super(router, base)
@@ -34,14 +35,18 @@ export class AbstractHistory extends History {
       return
     }
     const route = this.stack[targetIndex]
-    this.confirmTransition(route, () => {
-      this.index = targetIndex
-      this.updateRoute(route)
-    }, (err) => {
-      if (err === 'SAME_ROUTE') {
+    this.confirmTransition(
+      route,
+      () => {
         this.index = targetIndex
+        this.updateRoute(route)
+      },
+      err => {
+        if (err instanceof NavigationDuplicated) {
+          this.index = targetIndex
+        }
       }
-    })
+    )
   }
 
   getCurrentLocation () {

Diff for: src/history/base.js

@@ -11,24 +11,25 @@ import {
   flatMapComponents,
   resolveAsyncComponents
 } from '../util/resolve-components'
+import { NavigationDuplicated } from './errors'
 
 export class History {
-  router: Router;
-  base: string;
-  current: Route;
-  pending: ?Route;
-  cb: (r: Route) => void;
-  ready: boolean;
-  readyCbs: Array<Function>;
-  readyErrorCbs: Array<Function>;
-  errorCbs: Array<Function>;
+  router: Router
+  base: string
+  current: Route
+  pending: ?Route
+  cb: (r: Route) => void
+  ready: boolean
+  readyCbs: Array<Function>
+  readyErrorCbs: Array<Function>
+  errorCbs: Array<Function>
 
   // implemented by sub-classes
-  +go: (n: number) => void;
-  +push: (loc: RawLocation) => void;
-  +replace: (loc: RawLocation) => void;
-  +ensureURL: (push?: boolean) => void;
-  +getCurrentLocation: () => string;
+  +go: (n: number) => void
+  +push: (loc: RawLocation) => void
+  +replace: (loc: RawLocation) => void
+  +ensureURL: (push?: boolean) => void
+  +getCurrentLocation: () => string
 
   constructor (router: Router, base: ?string) {
     this.router = router
@@ -87,7 +88,11 @@ export class History {
   confirmTransition (route: Route, onComplete: Function, onAbort?: Function) {
     const current = this.current
     const abort = err => {
-      if (isError(err)) {
+      // after merging https://github.com/vuejs/vue-router/pull/2771 we
+      // When the user navigates through history through back/forward buttons
+      // we do not want to throw the error. We only throw it if directly calling
+      // push/replace. That's why it's not included in isError
+      if (!(err instanceof NavigationDuplicated) && isError(err)) {
         if (this.errorCbs.length) {
           this.errorCbs.forEach(cb => { cb(err) })
         } else {
@@ -103,7 +108,7 @@ export class History {
       route.matched.length === current.matched.length
     ) {
       this.ensureURL()
-      return abort('SAME_ROUTE')
+      return abort(new NavigationDuplicated(route))
     }
 
     const {

Diff for: src/history/errors.js

@@ -0,0 +1,6 @@
+export class NavigationDuplicated extends Error {
+  constructor () {
+    super('Navigating to current location is not allowed')
+    Object.setPrototypeOf(this, new.target.prototype)
+  }
+}

Diff for: test/unit/specs/node.spec.js

@@ -27,26 +27,39 @@ describe('Usage in Node', () => {
     const router = new VueRouter({
       routes: [
         { path: '/', component: Foo },
-        { path: '/bar', component: Bar, children: [
-          { path: 'baz', component: Baz }
-        ] }
+        {
+          path: '/bar',
+          component: Bar,
+          children: [{ path: 'baz', component: Baz }]
+        }
       ]
     })
     expect(router.getMatchedComponents('/')).toEqual([Foo])
     expect(router.getMatchedComponents('/bar/baz')).toEqual([Bar, Baz])
   })
 
   it('should navigate through history with same consecutive routes in history stack', () => {
+    const success = jasmine.createSpy('complete')
+    const error = jasmine.createSpy('error')
     const router = new VueRouter({
       routes: [
         { path: '/', component: { name: 'foo' }},
         { path: '/bar', component: { name: 'bar' }}
       ]
     })
-    router.push('/')
-    router.push('/bar')
-    router.push('/')
-    router.replace('/bar')
+    router.push('/', success, error)
+    expect(success).toHaveBeenCalledTimes(1)
+    expect(error).toHaveBeenCalledTimes(0)
+    router.push('/bar', success, error)
+    expect(success).toHaveBeenCalledTimes(2)
+    expect(error).toHaveBeenCalledTimes(0)
+    router.push('/', success, error)
+    expect(success).toHaveBeenCalledTimes(3)
+    expect(error).toHaveBeenCalledTimes(0)
+    router.replace('/bar', success, error)
+    expect(success).toHaveBeenCalledTimes(4)
+    expect(error).toHaveBeenCalledTimes(0)
+    spyOn(console, 'warn')
     router.back()
     expect(router.history.current.path).toBe('/bar')
     router.back()