From f2dfe4c356f7e4240d6a8e61707cfd5a593fe549 Mon Sep 17 00:00:00 2001 From: Ben Armston Date: Thu, 24 Sep 2015 16:06:38 +0100 Subject: [PATCH 1/4] Throw error when a promise is rejected This change makes it possible for client code to distinguish between rejected and failed promises when attaching then, or catch handlers. With the previous behaviour it was impossible to make this distinction (at least using the promise API). If the old behaviour is desired, a client can add a catch handler which returns the error before dispatching: ``` dispatch(myPromise.catch((err) => err)) ``` --- src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index 571d332..9371f01 100644 --- a/src/index.js +++ b/src/index.js @@ -59,7 +59,7 @@ export default function promiseMiddleware(resolvedName, rejectedName) { payload: error, meta: newAction.payload }); - return error; + throw error; } ); }; From 1fa699b82cce6f40c660248cb34d21e03c308dee Mon Sep 17 00:00:00 2001 From: Ben Armston Date: Thu, 24 Sep 2015 17:29:27 +0100 Subject: [PATCH 2/4] Update tests for rejection of rejected promises - Add returns to expectations including eventually. Apparently, this is required for the test framework to properly test the expectation. See the first section of http://chaijs.com/plugins/chai-as-promised. - Catch exception thrown now that we're using await with a rejected promise. --- src/__tests__/promiseMiddleware-test.js | 26 +++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/src/__tests__/promiseMiddleware-test.js b/src/__tests__/promiseMiddleware-test.js index 8226475..f37bf26 100644 --- a/src/__tests__/promiseMiddleware-test.js +++ b/src/__tests__/promiseMiddleware-test.js @@ -88,14 +88,20 @@ describe('promiseMiddleware', () => { }); it('dispatches reject action with arguments', async () => { - await dispatch({ - type: 'ACTION_TYPE_REJECT', - payload: { - promise: Promise.reject(err), - foo3: 'bar3', - foo4: 'bar4' - } - }); + try { + await dispatch({ + type: 'ACTION_TYPE_REJECT', + payload: { + promise: Promise.reject(err), + foo3: 'bar3', + foo4: 'bar4' + } + }); + } catch (e) { + // We're not interested in the rejection. We just need to wait until all + // dispatching is done. + true; + } expect(baseDispatch.calledTwice).to.be.true; @@ -133,7 +139,7 @@ describe('promiseMiddleware', () => { foo2: 'bar2' } }); - expect(dispatchedResult).to.eventually.equal(foobar); + return expect(dispatchedResult).to.eventually.equal(foobar); }); it('reject the original promise from dispatch', () => { @@ -146,7 +152,7 @@ describe('promiseMiddleware', () => { foo2: 'bar2' } }); - expect(dispatchedResult).to.eventually.be.rejectedWith(err); + return expect(dispatchedResult).to.eventually.be.rejectedWith(err); }); it('returns the reject and resolve strings with default values', () => { From 0317e088ed6502e48d2542ca9a68f7213775f7ef Mon Sep 17 00:00:00 2001 From: Ben Armston Date: Tue, 6 Oct 2015 17:39:35 +0100 Subject: [PATCH 3/4] Don't clobber action meta Previously, if an action had both meta data and payload, the metadata would be clobbered with the payload once the promise had either been rejected or resolved. Now, the original payload is added to the metadata under the key `payload` preserving any meta already present; unless there was meta under the key `payload` which seems unlikely. This is a breaking API change. --- src/__tests__/promiseMiddleware-test.js | 50 ++++++++++++++++++++++++- src/index.js | 24 +++++++++++- 2 files changed, 70 insertions(+), 4 deletions(-) diff --git a/src/__tests__/promiseMiddleware-test.js b/src/__tests__/promiseMiddleware-test.js index f37bf26..c6f4022 100644 --- a/src/__tests__/promiseMiddleware-test.js +++ b/src/__tests__/promiseMiddleware-test.js @@ -82,7 +82,9 @@ describe('promiseMiddleware', () => { type: resolve('ACTION_TYPE_RESOLVE'), payload: foobar, meta: { - foo2: 'bar2' + payload: { + foo2: 'bar2' + } } }); }); @@ -108,13 +110,57 @@ describe('promiseMiddleware', () => { expect(baseDispatch.secondCall.args[0]).to.deep.equal({ type: reject('ACTION_TYPE_REJECT'), payload: err, + meta: { + payload: { + foo3: 'bar3', + foo4: 'bar4' + } + } + }); + }); + + it('does not overwrite any meta arguments', async () => { + await dispatch({ + type: 'ACTION_TYPE_RESOLVE', + payload: { + promise: Promise.resolve(foobar), + foo2: 'bar2' + }, + meta: { + foo3: 'bar3' + } + }); + + expect(baseDispatch.calledTwice).to.be.true; + + expect(baseDispatch.secondCall.args[0]).to.deep.equal({ + type: resolve('ACTION_TYPE_RESOLVE'), + payload: foobar, meta: { foo3: 'bar3', - foo4: 'bar4' + payload: { + foo2: 'bar2' + } } }); }); + it('does not include empty meta payload attribute', async () => { + await dispatch({ + type: 'ACTION_TYPE_RESOLVE', + payload: { + promise: Promise.resolve(foobar) + } + }); + + expect(baseDispatch.calledTwice).to.be.true; + + expect(baseDispatch.secondCall.args[0]).to.deep.equal({ + type: resolve('ACTION_TYPE_RESOLVE'), + payload: foobar + }); + }); + it('returns the original promise from dispatch', () => { let promiseDispatched = new Promise(() => {}); diff --git a/src/index.js b/src/index.js index 9371f01..24efe1a 100644 --- a/src/index.js +++ b/src/index.js @@ -43,13 +43,33 @@ export default function promiseMiddleware(resolvedName, rejectedName) { dispatch(newAction); + // Create a base for the next action containing the metadata. + let nextActionBase = { + meta: { + ...action.meta, + payload: { + ...newAction.payload + } + } + } + + if (Object.keys(nextActionBase.meta.payload).length === 0) { + // No arguments were given beside the promise, no need to include them + // in the meta. + delete nextActionBase.meta.payload; + } + if (Object.keys(nextActionBase.meta).length === 0) { + // No meta was included either, remove all meta. + delete nextActionBase.meta; + } + // (2) Listen to promise and dispatch payload with new actionName return action.payload.promise.then( (result) => { dispatch({ type: resolve(action.type, resolvedName), payload: result, - meta: newAction.payload + ...nextActionBase }); return result; }, @@ -57,7 +77,7 @@ export default function promiseMiddleware(resolvedName, rejectedName) { dispatch({ type: reject(action.type, rejectedName), payload: error, - meta: newAction.payload + ...nextActionBase }); throw error; } From 426e9475a208b39eb2124999453fa1ea107db76c Mon Sep 17 00:00:00 2001 From: Ben Armston Date: Tue, 6 Oct 2015 17:50:12 +0100 Subject: [PATCH 4/4] Add missing semicolon --- src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index 24efe1a..772b6ac 100644 --- a/src/index.js +++ b/src/index.js @@ -51,7 +51,7 @@ export default function promiseMiddleware(resolvedName, rejectedName) { ...newAction.payload } } - } + }; if (Object.keys(nextActionBase.meta.payload).length === 0) { // No arguments were given beside the promise, no need to include them