From a7aa52e0ef1c37afbf3c6fce1614e88ea9402b9f Mon Sep 17 00:00:00 2001 From: Darren Ethier Date: Thu, 4 Apr 2019 16:23:12 -0400 Subject: [PATCH 01/12] restore dispatch actions returning actions that are a Promise --- packages/data/src/namespace-store/index.js | 6 +++++- .../data/src/namespace-store/test/index.js | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/packages/data/src/namespace-store/index.js b/packages/data/src/namespace-store/index.js index 3cdbc7a65a348..28e43b6f58fb2 100644 --- a/packages/data/src/namespace-store/index.js +++ b/packages/data/src/namespace-store/index.js @@ -8,6 +8,7 @@ import { mapValues, } from 'lodash'; import combineReducers from 'turbo-combine-reducers'; +import isPromise from 'is-promise'; /** * WordPress dependencies @@ -188,7 +189,10 @@ function mapSelectors( selectors, store, registry ) { */ function mapActions( actions, store ) { const createBoundAction = ( action ) => ( ...args ) => { - store.dispatch( action( ...args ) ); + const result = store.dispatch( action( ...args ) ); + if ( isPromise( result ) ) { + return result; + } }; return mapValues( actions, createBoundAction ); diff --git a/packages/data/src/namespace-store/test/index.js b/packages/data/src/namespace-store/test/index.js index a3ee80577ece1..eb97505a9920a 100644 --- a/packages/data/src/namespace-store/test/index.js +++ b/packages/data/src/namespace-store/test/index.js @@ -1,3 +1,8 @@ +/** + * External dependencies + */ +import isPromise from 'is-promise'; + /** * Internal dependencies */ @@ -80,4 +85,18 @@ describe( 'controls', () => { registry.select( 'store' ).getItems(); } ); + it( 'returns undefined when action is dispatched unless it is a ' + + 'promise', () => { + const actions = { + withPromise: () => new Promise( ( resolve ) => resolve( {} ) ), + normal: () => ( { type: 'NORMAL' } ), + }; + registry.registerStore( 'store', { + reducer: () => {}, + actions, + } ); + expect( isPromise( registry.dispatch( 'store' ).withPromise() ) ) + .toBe( true ); + expect( registry.dispatch( 'store' ).normal() ).toBeUndefined(); + } ); } ); From 4c81ff9b0ff783485e3748d036eba35c78f873a1 Mon Sep 17 00:00:00 2001 From: Darren Ethier Date: Thu, 4 Apr 2019 19:51:30 -0400 Subject: [PATCH 02/12] expand tests to cover all possible dispatch action responses --- .../data/src/namespace-store/test/index.js | 64 +++++++++++++++---- 1 file changed, 50 insertions(+), 14 deletions(-) diff --git a/packages/data/src/namespace-store/test/index.js b/packages/data/src/namespace-store/test/index.js index eb97505a9920a..55dcba9968aff 100644 --- a/packages/data/src/namespace-store/test/index.js +++ b/packages/data/src/namespace-store/test/index.js @@ -1,8 +1,3 @@ -/** - * External dependencies - */ -import isPromise from 'is-promise'; - /** * Internal dependencies */ @@ -85,18 +80,59 @@ describe( 'controls', () => { registry.select( 'store' ).getItems(); } ); - it( 'returns undefined when action is dispatched unless it is a ' + - 'promise', () => { + describe( 'various action types have expected response and resolve as ' + + 'expected', () => { const actions = { - withPromise: () => new Promise( ( resolve ) => resolve( {} ) ), + *withPromise() { + yield { type: 'SOME_ACTION' }; + return yield { type: 'TEST_PROMISE' }; + }, + *withNormal() { + yield { type: 'SOME_ACTION' }; + yield { type: 'SOME_OTHER_ACTION' }; + }, + *withNonActionLikeValue() { + yield { type: 'SOME_ACTION' }; + return 10; + }, + normalShouldFail: () => 10, normal: () => ( { type: 'NORMAL' } ), }; - registry.registerStore( 'store', { - reducer: () => {}, - actions, + beforeEach( () => { + registry.registerStore( 'store', { + reducer: () => {}, + controls: { + TEST_PROMISE() { + return new Promise( ( resolve ) => resolve( 10 ) ); + }, + }, + actions, + } ); + } ); + it( 'action generator returning a yielded promise control descriptor ' + + 'resolves as expected', () => { + const withPromise = registry.dispatch( 'store' ).withPromise(); + expect( withPromise ).resolves.toEqual( 10 ); + } ); + it( 'action generator yielding normal action objects resolves as ' + + 'expected', () => { + const withNormal = registry.dispatch( 'store' ).withNormal(); + expect( withNormal ).resolves.toBeUndefined(); + } ); + it( 'action generator returning a non action like value', () => { + const withNonActionLikeValue = registry.dispatch( 'store' ) + .withNonActionLikeValue(); + expect( withNonActionLikeValue ).resolves.toEqual( 10 ); + } ); + it( 'normal dispatch action throwing error because no action ' + + 'returned', () => { + const testDispatch = () => registry.dispatch( 'store' ).normalShouldFail(); + expect( testDispatch ).toThrow( + 'Actions must be plain objects. Use custom middleware for async actions.' + ); + } ); + it( 'returns undefined for normal dispatch action', () => { + expect( registry.dispatch( 'store' ).normal() ).toBeUndefined(); } ); - expect( isPromise( registry.dispatch( 'store' ).withPromise() ) ) - .toBe( true ); - expect( registry.dispatch( 'store' ).normal() ).toBeUndefined(); } ); } ); From 53e61f6db83f470bc0fd167e8951e993a3e456d3 Mon Sep 17 00:00:00 2001 From: Darren Ethier Date: Thu, 4 Apr 2019 19:51:53 -0400 Subject: [PATCH 03/12] expand jsdoc for wp.data.dispatch calls --- packages/data/src/index.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/data/src/index.js b/packages/data/src/index.js index ae786ee99b4ca..ab32d7e2375e9 100644 --- a/packages/data/src/index.js +++ b/packages/data/src/index.js @@ -79,6 +79,9 @@ export const select = defaultRegistry.select; * Given the name of a registered store, returns an object of the store's action creators. * Calling an action creator will cause it to be dispatched, updating the state value accordingly. * + * Note: If the action creator is a generator then a promise is returned. + * Otherwise, the dispatch will return `undefined`. + * * @param {string} name Store name * * @example From 0de7797a5dcd1ee540114897412a815540398f24 Mon Sep 17 00:00:00 2001 From: Darren Ethier Date: Thu, 4 Apr 2019 19:52:11 -0400 Subject: [PATCH 04/12] Add changelog entry --- packages/data/CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/data/CHANGELOG.md b/packages/data/CHANGELOG.md index 34cfc2e0096e1..9f32e3d9fe17b 100644 --- a/packages/data/CHANGELOG.md +++ b/packages/data/CHANGELOG.md @@ -1,3 +1,9 @@ +## Master + +### Bug Fix + +- Restore functionality of action-generators returning a Promise. Clarify intent and behaviour for `wp.data.dispatch` behaviour ([#14830](https://github.com/WordPress/gutenberg/pull/14830) + ## 4.3.0 (2019-03-06) ### Enhancements From fbc7e7f467840a51577f01811beb05d6b377f11e Mon Sep 17 00:00:00 2001 From: Darren Ethier Date: Thu, 4 Apr 2019 19:52:23 -0400 Subject: [PATCH 05/12] update docs --- packages/data/README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/data/README.md b/packages/data/README.md index c496d0ef6dbf0..6761f0656139e 100644 --- a/packages/data/README.md +++ b/packages/data/README.md @@ -332,6 +332,9 @@ _Returns_ Given the name of a registered store, returns an object of the store's action creators. Calling an action creator will cause it to be dispatched, updating the state value accordingly. +Note: If the action creator is a generator then a promise is returned. +Otherwise, the dispatch will return `undefined`. + _Usage_ ```js From 31c171b82f8d5badf8e60639a3df12e774ec775f Mon Sep 17 00:00:00 2001 From: Darren Ethier Date: Sat, 6 Apr 2019 08:50:37 -0400 Subject: [PATCH 06/12] more test coverage (without controls) --- .../data/src/namespace-store/test/index.js | 33 ++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/packages/data/src/namespace-store/test/index.js b/packages/data/src/namespace-store/test/index.js index 55dcba9968aff..0865c3f4d31d1 100644 --- a/packages/data/src/namespace-store/test/index.js +++ b/packages/data/src/namespace-store/test/index.js @@ -81,7 +81,7 @@ describe( 'controls', () => { registry.select( 'store' ).getItems(); } ); describe( 'various action types have expected response and resolve as ' + - 'expected', () => { + 'expected with controls middleware', () => { const actions = { *withPromise() { yield { type: 'SOME_ACTION' }; @@ -135,4 +135,35 @@ describe( 'controls', () => { expect( registry.dispatch( 'store' ).normal() ).toBeUndefined(); } ); } ); + describe( 'action type resolves as expected with just promise ' + + 'middleware', () => { + const actions = { + normal: () => ( { type: 'NORMAL' } ), + withPromiseAndAction: () => new Promise( + ( resolve ) => resolve( { type: 'WITH_PROMISE' } ) + ), + withPromiseAndNonAction: () => new Promise( + ( resolve ) => resolve( 10 ) + ), + }; + beforeEach( () => { + registry.registerStore( 'store', { + reducer: () => {}, + actions, + } ); + } ); + it( 'normal action returns undefined', () => { + expect( registry.dispatch( 'store' ).normal() ).toBeUndefined(); + } ); + it( 'action with promise resolving to action returning undefined', () => { + expect( registry.dispatch( 'store' ).withPromiseAndAction() ) + .resolves + .toBeUndefined(); + } ); + it( 'action with promise returning non action throws error', () => { + const dispatchedAction = registry.dispatch( 'store' ) + .withPromiseAndNonAction(); + expect( dispatchedAction ).resolves.toBe( 10 ); + } ); + } ); } ); From 3861ca77e63171de27082657286f42b7696ff43a Mon Sep 17 00:00:00 2001 From: Darren Ethier Date: Sat, 6 Apr 2019 08:52:24 -0400 Subject: [PATCH 07/12] update docs --- packages/data/README.md | 4 ++-- packages/data/src/index.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/data/README.md b/packages/data/README.md index 6761f0656139e..1e8b717df9c5f 100644 --- a/packages/data/README.md +++ b/packages/data/README.md @@ -332,8 +332,8 @@ _Returns_ Given the name of a registered store, returns an object of the store's action creators. Calling an action creator will cause it to be dispatched, updating the state value accordingly. -Note: If the action creator is a generator then a promise is returned. -Otherwise, the dispatch will return `undefined`. +Note: If the action creator is a generator or returns a promise, a promise is +returned. Otherwise, the dispatch will return `undefined`. _Usage_ diff --git a/packages/data/src/index.js b/packages/data/src/index.js index ab32d7e2375e9..ea5912ec08a98 100644 --- a/packages/data/src/index.js +++ b/packages/data/src/index.js @@ -79,8 +79,8 @@ export const select = defaultRegistry.select; * Given the name of a registered store, returns an object of the store's action creators. * Calling an action creator will cause it to be dispatched, updating the state value accordingly. * - * Note: If the action creator is a generator then a promise is returned. - * Otherwise, the dispatch will return `undefined`. + * Note: If the action creator is a generator or returns a promise, a promise is + * returned. Otherwise, the dispatch will return `undefined`. * * @param {string} name Store name * From 9c9943b8d7725c395d66301779da53eb6bb1c037 Mon Sep 17 00:00:00 2001 From: Darren Ethier Date: Thu, 11 Apr 2019 15:12:38 -0400 Subject: [PATCH 08/12] Always return a promise on dispatch --- packages/data/src/namespace-store/index.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/data/src/namespace-store/index.js b/packages/data/src/namespace-store/index.js index 28e43b6f58fb2..73e6560baf8e9 100644 --- a/packages/data/src/namespace-store/index.js +++ b/packages/data/src/namespace-store/index.js @@ -8,7 +8,6 @@ import { mapValues, } from 'lodash'; import combineReducers from 'turbo-combine-reducers'; -import isPromise from 'is-promise'; /** * WordPress dependencies @@ -189,10 +188,7 @@ function mapSelectors( selectors, store, registry ) { */ function mapActions( actions, store ) { const createBoundAction = ( action ) => ( ...args ) => { - const result = store.dispatch( action( ...args ) ); - if ( isPromise( result ) ) { - return result; - } + return Promise.resolve( store.dispatch( action( ...args ) ) ); }; return mapValues( actions, createBoundAction ); From 9675f41f842e2b8e1906df9b00c26f5d9ca5f6d9 Mon Sep 17 00:00:00 2001 From: Darren Ethier Date: Thu, 11 Apr 2019 15:13:15 -0400 Subject: [PATCH 09/12] update test expectations --- .../components/with-dispatch/test/index.js | 21 ++++++++--- .../data/src/namespace-store/test/index.js | 37 +++++++++++-------- packages/data/src/test/registry.js | 13 +++++-- 3 files changed, 47 insertions(+), 24 deletions(-) diff --git a/packages/data/src/components/with-dispatch/test/index.js b/packages/data/src/components/with-dispatch/test/index.js index c7a6fe9ac034f..98f225fad2c40 100644 --- a/packages/data/src/components/with-dispatch/test/index.js +++ b/packages/data/src/components/with-dispatch/test/index.js @@ -34,8 +34,13 @@ describe( 'withDispatch', () => { return { increment: () => { - const actionReturnedFromDispatch = _dispatch( 'counter' ).increment( count ); - expect( actionReturnedFromDispatch ).toBe( undefined ); + const actionReturnedFromDispatch = Promise.resolve( _dispatch( 'counter' ).increment( count ) ); + expect( actionReturnedFromDispatch ).resolves.toEqual( + { + type: 'increment', + count, + } + ); }, }; } )( ( props ) =>