From 7aba8a934995fb3994280b6adac24e2a10425669 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Balint=20Farkas=20Werb=C3=A1nszky?= Date: Fri, 4 Oct 2019 16:16:10 +0300 Subject: [PATCH 1/3] action subscribers would receive a call even when the action failed (promise.reject, js error) This way the action subscribers of the 'after' would not miss the end of the action. The true/false value might be useful for after action handlers, but the result might be too tempting to be used for modification for example, might be that too much info is handled out in this way. The same goes for the success-side of the after subscribers too. --- src/store.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/store.js b/src/store.js index b32d7b0ad..09a8253a8 100644 --- a/src/store.js +++ b/src/store.js @@ -148,7 +148,7 @@ export class Store { try { this._actionSubscribers .filter(sub => sub.after) - .forEach(sub => sub.after(action, this.state)) + .forEach(sub => sub.after(action, this.state, /*isError:*/false, res)) } catch (e) { if (process.env.NODE_ENV !== 'production') { console.warn(`[vuex] error in after action subscribers: `) @@ -157,6 +157,19 @@ export class Store { } return res }) + .catch(errorResult => { + try { + this._actionSubscribers + .filter(sub => sub.after) + .forEach(sub => sub.after(action, this.state, /*isError:*/true, errorResult)) + } catch (e) { + if (process.env.NODE_ENV !== 'production') { + console.warn(`[vuex] error in after action subscribers: `) + console.error(e) + } + } + throw errorResult + }) } subscribe (fn) { From bd2e4639d88e57ded551a8c35b3cb97f3679d21a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Balint=20Farkas=20Werb=C3=A1nszky?= Date: Mon, 7 Oct 2019 12:03:21 +0200 Subject: [PATCH 2/3] Update store.js eslint --- src/store.js | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/store.js b/src/store.js index 09a8253a8..5e58855fb 100644 --- a/src/store.js +++ b/src/store.js @@ -148,7 +148,7 @@ export class Store { try { this._actionSubscribers .filter(sub => sub.after) - .forEach(sub => sub.after(action, this.state, /*isError:*/false, res)) + .forEach(sub => sub.after(action, this.state, /* isError: */false, res)) } catch (e) { if (process.env.NODE_ENV !== 'production') { console.warn(`[vuex] error in after action subscribers: `) @@ -157,19 +157,19 @@ export class Store { } return res }) - .catch(errorResult => { - try { - this._actionSubscribers - .filter(sub => sub.after) - .forEach(sub => sub.after(action, this.state, /*isError:*/true, errorResult)) - } catch (e) { - if (process.env.NODE_ENV !== 'production') { - console.warn(`[vuex] error in after action subscribers: `) - console.error(e) + .catch(errorResult => { + try { + this._actionSubscribers + .filter(sub => sub.after) + .forEach(sub => sub.after(action, this.state, /* isError: */true, errorResult)) + } catch (e) { + if (process.env.NODE_ENV !== 'production') { + console.warn(`[vuex] error in after action subscribers: `) + console.error(e) + } } - } - throw errorResult - }) + throw errorResult + }) } subscribe (fn) { From c89fe9f5584e1991be8260ae9ff97f7a370c5c4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Balint=20Farkas=20Werb=C3=A1nszky?= Date: Mon, 7 Oct 2019 12:53:17 +0200 Subject: [PATCH 3/3] Update modules.spec.js To test the reject and resolve software paths --- test/unit/modules.spec.js | 42 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/test/unit/modules.spec.js b/test/unit/modules.spec.js index a776fb582..d90af3c02 100644 --- a/test/unit/modules.spec.js +++ b/test/unit/modules.spec.js @@ -669,12 +669,12 @@ describe('Modules', () => { ) }) - it('action before/after subscribers', (done) => { + it('action before/after subscribers with resolve()', (done) => { const beforeSpy = jasmine.createSpy() const afterSpy = jasmine.createSpy() const store = new Vuex.Store({ actions: { - [TEST]: () => Promise.resolve() + [TEST]: () => Promise.resolve('resolve') }, plugins: [ store => { @@ -694,7 +694,43 @@ describe('Modules', () => { Vue.nextTick(() => { expect(afterSpy).toHaveBeenCalledWith( { type: TEST, payload: 2 }, - store.state + store.state, + false, + 'resolve' + ) + done() + }) + }) + }) + + it('action before/after subscribers with reject()', (done) => { + const beforeSpy = jasmine.createSpy() + const afterSpy = jasmine.createSpy() + const store = new Vuex.Store({ + actions: { + [TEST]: () => Promise.reject('reject') + }, + plugins: [ + store => { + store.subscribeAction({ + before: beforeSpy, + after: afterSpy + }) + } + ] + }) + store.dispatch(TEST, 2) + expect(beforeSpy).toHaveBeenCalledWith( + { type: TEST, payload: 2 }, + store.state + ) + expect(afterSpy).not.toHaveBeenCalled() + Vue.nextTick(() => { + expect(afterSpy).toHaveBeenCalledWith( + { type: TEST, payload: 2 }, + store.state, + false, + 'reject' ) done() })