Skip to content

Commit

Permalink
fix(abstract history): allow router.back in abstract mode when 2 cons…
Browse files Browse the repository at this point in the history
…ecutive same routes appear in history stack (#2771)

* fix(AbstractHistory): Fix router.back in abstract mode

Fix abstract mode's router.back() when 2 consecutive same routes appear in the history stack array

fix #2607

* refactor(history): use an Error object when navigatin to the same place
  • Loading branch information
Domino9697 authored and posva committed Jul 5, 2019
1 parent 64785a9 commit 8910979
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 25 deletions.
21 changes: 15 additions & 6 deletions src/history/abstract.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -34,10 +35,18 @@ export class AbstractHistory extends History {
return
}
const route = this.stack[targetIndex]
this.confirmTransition(route, () => {
this.index = targetIndex
this.updateRoute(route)
})
this.confirmTransition(
route,
() => {
this.index = targetIndex
this.updateRoute(route)
},
err => {
if (err instanceof NavigationDuplicated) {
this.index = targetIndex
}
}
)
}

getCurrentLocation () {
Expand Down
37 changes: 21 additions & 16 deletions src/history/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -103,7 +108,7 @@ export class History {
route.matched.length === current.matched.length
) {
this.ensureURL()
return abort()
return abort(new NavigationDuplicated(route))
}

const {
Expand Down
6 changes: 6 additions & 0 deletions src/history/errors.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export class NavigationDuplicated extends Error {
constructor () {
super('Navigating to current location is not allowed')
Object.setPrototypeOf(this, new.target.prototype)
}
}
36 changes: 33 additions & 3 deletions test/unit/specs/node.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,42 @@ 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('/', 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()
expect(router.history.current.path).toBe('/')
})
})

0 comments on commit 8910979

Please sign in to comment.