From 358758d7d77a09e0ab6d99b2743cedb6ccb87978 Mon Sep 17 00:00:00 2001 From: Tomasz Szarek Date: Tue, 30 Jul 2019 21:20:18 +0200 Subject: [PATCH 1/4] fix: push/replace return promise close(#1769) --- src/index.js | 18 ++++++- test/unit/specs/api.spec.js | 99 +++++++++++++++++++++++++++---------- types/router.d.ts | 4 +- 3 files changed, 92 insertions(+), 29 deletions(-) diff --git a/src/index.js b/src/index.js index b3eb25dd3..e0c30c387 100644 --- a/src/index.js +++ b/src/index.js @@ -151,11 +151,25 @@ export default class VueRouter { } push (location: RawLocation, onComplete?: Function, onAbort?: Function) { - this.history.push(location, onComplete, onAbort) + // $flow-disable-line + if (!onComplete && !onAbort && typeof Promise !== 'undefined') { + return new Promise(resolve => { + this.history.push(location, resolve) + }) + } else { + this.history.push(location, onComplete, onAbort) + } } replace (location: RawLocation, onComplete?: Function, onAbort?: Function) { - this.history.replace(location, onComplete, onAbort) + // $flow-disable-line + if (!onComplete && !onAbort && typeof Promise !== 'undefined') { + return new Promise(resolve => { + this.history.replace(location, resolve) + }) + } else { + this.history.replace(location, onComplete, onAbort) + } } go (n: number) { diff --git a/test/unit/specs/api.spec.js b/test/unit/specs/api.spec.js index 1fba4977b..3c943473f 100644 --- a/test/unit/specs/api.spec.js +++ b/test/unit/specs/api.spec.js @@ -118,7 +118,7 @@ describe('router.addRoutes', () => { }) }) -describe('router.push/replace callbacks', () => { +describe('router.push/replace', () => { let calls = [] let router, spy1, spy2 @@ -151,38 +151,87 @@ describe('router.push/replace callbacks', () => { }, 1) }) }) + describe('callbacks', () => { + it('push complete', done => { + router.push('/foo', () => { + expect(calls).toEqual([1, 2, 3, 4]) + done() + }) + }) - it('push complete', done => { - router.push('/foo', () => { - expect(calls).toEqual([1, 2, 3, 4]) - done() + it('push abort', done => { + router.push('/foo', spy1, spy2) + router.push('/bar', () => { + expect(calls).toEqual([1, 1, 2, 2]) + expect(spy1).not.toHaveBeenCalled() + expect(spy2).toHaveBeenCalled() + done() + }) }) - }) - it('push abort', done => { - router.push('/foo', spy1, spy2) - router.push('/bar', () => { - expect(calls).toEqual([1, 1, 2, 2]) - expect(spy1).not.toHaveBeenCalled() - expect(spy2).toHaveBeenCalled() - done() + it('replace complete', done => { + router.replace('/foo', () => { + expect(calls).toEqual([1, 2, 3, 4]) + done() + }) }) - }) - it('replace complete', done => { - router.replace('/foo', () => { - expect(calls).toEqual([1, 2, 3, 4]) - done() + it('replace abort', done => { + router.replace('/foo', spy1, spy2) + router.replace('/bar', () => { + expect(calls).toEqual([1, 1, 2, 2]) + expect(spy1).not.toHaveBeenCalled() + expect(spy2).toHaveBeenCalled() + done() + }) }) }) - it('replace abort', done => { - router.replace('/foo', spy1, spy2) - router.replace('/bar', () => { - expect(calls).toEqual([1, 1, 2, 2]) - expect(spy1).not.toHaveBeenCalled() - expect(spy2).toHaveBeenCalled() - done() + describe('promises', () => { + it('push with callback return undefined', done => { + expect(router.push('/foo', done)).toEqual(undefined) + }) + + it('push complete', done => { + router.push('/foo') + .then(spy1) + .finally(() => { + expect(calls).toEqual([1, 2, 3, 4]) + expect(spy1).toHaveBeenCalledWith(router.currentRoute) + done() + }) + }) + + it('push abort', done => { + router.push('/foo') + router.push('/bar').finally(() => { + expect(calls).toEqual([1, 1, 2, 2]) + expect(spy1).not.toHaveBeenCalled() + done() + }) + }) + + it('replace with callback return undefined', done => { + expect(router.replace('/foo', done)).toEqual(undefined) + }) + + it('replace complete', done => { + router.replace('/foo') + .then(spy1) + .finally(() => { + expect(calls).toEqual([1, 2, 3, 4]) + expect(spy1).toHaveBeenCalledWith(router.currentRoute) + done() + }) + }) + + it('replace abort', done => { + router.replace('/foo') + router.replace('/bar').finally(() => { + expect(calls).toEqual([1, 1, 2, 2]) + expect(spy1).not.toHaveBeenCalled() + done() + }) }) }) }) diff --git a/types/router.d.ts b/types/router.d.ts index dc4516bbd..23f088b51 100644 --- a/types/router.d.ts +++ b/types/router.d.ts @@ -23,8 +23,8 @@ export declare class VueRouter { beforeEach (guard: NavigationGuard): Function; beforeResolve (guard: NavigationGuard): Function; afterEach (hook: (to: Route, from: Route) => any): Function; - push (location: RawLocation, onComplete?: Function, onAbort?: ErrorHandler): void; - replace (location: RawLocation, onComplete?: Function, onAbort?: ErrorHandler): void; + push (location: RawLocation, onComplete?: Function, onAbort?: ErrorHandler): Promise | void; + replace (location: RawLocation, onComplete?: Function, onAbort?: ErrorHandler): Promise | void; go (n: number): void; back (): void; forward (): void; From 2c80d97e471ae58ecfb0251b6d02a65eb6ae0d4a Mon Sep 17 00:00:00 2001 From: Tomasz Szarek Date: Wed, 31 Jul 2019 11:18:30 +0200 Subject: [PATCH 2/4] feat: organize tests and add overloading --- test/unit/specs/api.spec.js | 16 ++++++++-------- types/router.d.ts | 6 ++++-- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/test/unit/specs/api.spec.js b/test/unit/specs/api.spec.js index 3c943473f..c9718975e 100644 --- a/test/unit/specs/api.spec.js +++ b/test/unit/specs/api.spec.js @@ -152,6 +152,10 @@ describe('router.push/replace', () => { }) }) describe('callbacks', () => { + it('push with callback return undefined', done => { + expect(router.push('/foo', done)).toEqual(undefined) + }) + it('push complete', done => { router.push('/foo', () => { expect(calls).toEqual([1, 2, 3, 4]) @@ -169,6 +173,10 @@ describe('router.push/replace', () => { }) }) + it('replace with callback return undefined', done => { + expect(router.replace('/foo', done)).toEqual(undefined) + }) + it('replace complete', done => { router.replace('/foo', () => { expect(calls).toEqual([1, 2, 3, 4]) @@ -188,10 +196,6 @@ describe('router.push/replace', () => { }) describe('promises', () => { - it('push with callback return undefined', done => { - expect(router.push('/foo', done)).toEqual(undefined) - }) - it('push complete', done => { router.push('/foo') .then(spy1) @@ -211,10 +215,6 @@ describe('router.push/replace', () => { }) }) - it('replace with callback return undefined', done => { - expect(router.replace('/foo', done)).toEqual(undefined) - }) - it('replace complete', done => { router.replace('/foo') .then(spy1) diff --git a/types/router.d.ts b/types/router.d.ts index 23f088b51..054dca896 100644 --- a/types/router.d.ts +++ b/types/router.d.ts @@ -23,8 +23,10 @@ export declare class VueRouter { beforeEach (guard: NavigationGuard): Function; beforeResolve (guard: NavigationGuard): Function; afterEach (hook: (to: Route, from: Route) => any): Function; - push (location: RawLocation, onComplete?: Function, onAbort?: ErrorHandler): Promise | void; - replace (location: RawLocation, onComplete?: Function, onAbort?: ErrorHandler): Promise | void; + push (location: RawLocation, onComplete?: Function, onAbort?: ErrorHandler): void; + replace (location: RawLocation, onComplete?: Function, onAbort?: ErrorHandler): void; + push (location: RawLocation): Promise; + replace (location: RawLocation): Promise; go (n: number): void; back (): void; forward (): void; From b0e8ac58a09923ee96589af3db0be137e771ee67 Mon Sep 17 00:00:00 2001 From: Tomasz Szarek Date: Wed, 31 Jul 2019 11:24:06 +0200 Subject: [PATCH 3/4] fix: text names --- test/unit/specs/api.spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/specs/api.spec.js b/test/unit/specs/api.spec.js index c9718975e..aaeabab1d 100644 --- a/test/unit/specs/api.spec.js +++ b/test/unit/specs/api.spec.js @@ -152,7 +152,7 @@ describe('router.push/replace', () => { }) }) describe('callbacks', () => { - it('push with callback return undefined', done => { + it('push does not return a Promise when a callback is passed', done => { expect(router.push('/foo', done)).toEqual(undefined) }) @@ -173,7 +173,7 @@ describe('router.push/replace', () => { }) }) - it('replace with callback return undefined', done => { + it('replace does not return a Promise when a callback is passed', done => { expect(router.replace('/foo', done)).toEqual(undefined) }) From cf38fcd47dea128c313f62de742be4edc4e7f92a Mon Sep 17 00:00:00 2001 From: Tomasz Szarek Date: Wed, 31 Jul 2019 12:22:35 +0200 Subject: [PATCH 4/4] feat: Add promise rejection --- src/index.js | 8 +++--- test/unit/specs/api.spec.js | 6 +++-- test/unit/specs/error-handling.spec.js | 36 ++++++++++++++++---------- 3 files changed, 31 insertions(+), 19 deletions(-) diff --git a/src/index.js b/src/index.js index e0c30c387..e95ace6d1 100644 --- a/src/index.js +++ b/src/index.js @@ -153,8 +153,8 @@ export default class VueRouter { push (location: RawLocation, onComplete?: Function, onAbort?: Function) { // $flow-disable-line if (!onComplete && !onAbort && typeof Promise !== 'undefined') { - return new Promise(resolve => { - this.history.push(location, resolve) + return new Promise((resolve, reject) => { + this.history.push(location, resolve, reject) }) } else { this.history.push(location, onComplete, onAbort) @@ -164,8 +164,8 @@ export default class VueRouter { replace (location: RawLocation, onComplete?: Function, onAbort?: Function) { // $flow-disable-line if (!onComplete && !onAbort && typeof Promise !== 'undefined') { - return new Promise(resolve => { - this.history.replace(location, resolve) + return new Promise((resolve, reject) => { + this.history.replace(location, resolve, reject) }) } else { this.history.replace(location, onComplete, onAbort) diff --git a/test/unit/specs/api.spec.js b/test/unit/specs/api.spec.js index aaeabab1d..8611b1987 100644 --- a/test/unit/specs/api.spec.js +++ b/test/unit/specs/api.spec.js @@ -207,10 +207,11 @@ describe('router.push/replace', () => { }) it('push abort', done => { - router.push('/foo') + router.push('/foo').catch(spy2) router.push('/bar').finally(() => { expect(calls).toEqual([1, 1, 2, 2]) expect(spy1).not.toHaveBeenCalled() + expect(spy2).toHaveBeenCalled() done() }) }) @@ -226,10 +227,11 @@ describe('router.push/replace', () => { }) it('replace abort', done => { - router.replace('/foo') + router.replace('/foo').catch(spy2) router.replace('/bar').finally(() => { expect(calls).toEqual([1, 1, 2, 2]) expect(spy1).not.toHaveBeenCalled() + expect(spy2).toHaveBeenCalled() done() }) }) diff --git a/test/unit/specs/error-handling.spec.js b/test/unit/specs/error-handling.spec.js index b912dca62..6ac2944ff 100644 --- a/test/unit/specs/error-handling.spec.js +++ b/test/unit/specs/error-handling.spec.js @@ -4,7 +4,7 @@ import VueRouter from '../../../src/index' Vue.use(VueRouter) describe('error handling', () => { - it('onReady errors', () => { + it('onReady errors', done => { const router = new VueRouter() const err = new Error('foo') router.beforeEach(() => { throw err }) @@ -12,31 +12,39 @@ describe('error handling', () => { const onReady = jasmine.createSpy('ready') const onError = jasmine.createSpy('error') + const onPromiseReject = jasmine.createSpy('promise reject') router.onReady(onReady, onError) - router.push('/') - - expect(onReady).not.toHaveBeenCalled() - expect(onError).toHaveBeenCalledWith(err) + router.push('/').catch(onPromiseReject).finally(() => { + expect(onReady).not.toHaveBeenCalled() + expect(onError).toHaveBeenCalledWith(err) + expect(onPromiseReject).toHaveBeenCalled() + done() + }) }) - it('navigation errors', () => { + it('navigation errors', done => { const router = new VueRouter() const err = new Error('foo') const spy = jasmine.createSpy('error') + const spy1 = jasmine.createSpy('promise reject') router.onError(spy) router.push('/') router.beforeEach(() => { throw err }) - router.push('/foo') - expect(spy).toHaveBeenCalledWith(err) + router.push('/foo').catch(spy1).finally(() => { + expect(spy).toHaveBeenCalledWith(err) + expect(spy1).toHaveBeenCalled() + done() + }) }) - it('async component errors', () => { + it('async component errors', done => { const err = new Error('foo') const spy1 = jasmine.createSpy('error') const spy2 = jasmine.createSpy('errpr') + const spy3 = jasmine.createSpy('promise reject') const Comp = () => { throw err } const router = new VueRouter({ routes: [ @@ -47,9 +55,11 @@ describe('error handling', () => { router.onError(spy1) router.onReady(() => {}, spy2) - router.push('/') - - expect(spy1).toHaveBeenCalledWith(err) - expect(spy2).toHaveBeenCalledWith(err) + router.push('/').catch(spy3).finally(() => { + expect(spy1).toHaveBeenCalledWith(err) + expect(spy2).toHaveBeenCalledWith(err) + expect(spy3).toHaveBeenCalled() + done() + }) }) })