Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(fix #2607): Fix router.back in abstract mode when 2 consecutive same routes appear in history stack #2771

Merged
merged 2 commits into from
Jul 5, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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('/')
})
})