Data: Restore dispatch actions returning actions that are a Promise (W…

…ordPress#14830)

All dispatched action creators will now return a Promise.

packages/data/CHANGELOG.md

@@ -1,3 +1,10 @@
+## Master
+
+### Bug Fix
+
+- Restore functionality of action-generators returning a Promise.  Clarify intent and behaviour for `wp.data.dispatch` behaviour. Dispatch actions now always
+ return a promise ([#14830](https://github.com/WordPress/gutenberg/pull/14830)
+
 ## 4.3.0 (2019-03-06)
 
 ### Enhancements

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: Action creators returned by the dispatch will return a promise when
+they are called.
+
 _Usage_
 
 ```js

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 ) => <button onClick={ props.increment } /> );
@@ -120,7 +125,8 @@ describe( 'withDispatch', () => {
 		expect( secondRegistryAction ).toHaveBeenCalledTimes( 2 );
 	} );
 
-	it( 'always calls select with the latest state in the handler passed to the component', () => {
+	it( 'always calls select with the latest state in the handler passed to ' +
+		'the component', () => {
 		const store = registry.registerStore( 'counter', {
 			reducer: ( state = 0, action ) => {
 				if ( action.type === 'update' ) {
@@ -142,8 +148,13 @@ describe( 'withDispatch', () => {
 				update: () => {
 					const innerCount = _select( 'counter' ).getCount();
 					expect( innerCount ).toBe( outerCount );
-					const actionReturnedFromDispatch = _dispatch( 'counter' ).update( innerCount + 1 );
-					expect( actionReturnedFromDispatch ).toBe( undefined );
+					const actionReturnedFromDispatch = Promise.resolve(
+						_dispatch( 'counter' ).update( innerCount + 1 )
+					);
+					expect( actionReturnedFromDispatch ).resolves.toEqual( {
+						type: 'update',
+						count: innerCount + 1,
+					} );
 				},
 			};
 		} )( ( props ) => <button onClick={ props.update } /> );

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: Action creators returned by the dispatch will return a promise when
+ * they are called.
+ *
  * @param {string} name Store name
  *
  * @example

packages/data/src/namespace-store/index.js

@@ -188,7 +188,7 @@ function mapSelectors( selectors, store, registry ) {
  */
 function mapActions( actions, store ) {
 	const createBoundAction = ( action ) => ( ...args ) => {
-		store.dispatch( action( ...args ) );
+		return Promise.resolve( store.dispatch( action( ...args ) ) );
 	};
 
 	return mapValues( actions, createBoundAction );

packages/data/src/namespace-store/test/index.js

@@ -80,4 +80,97 @@ describe( 'controls', () => {
 
 		registry.select( 'store' ).getItems();
 	} );
+	describe( 'various action types have expected response and resolve as ' +
+		'expected with controls middleware', () => {
+		const actions = {
+			*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' } ),
+		};
+		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', async () => {
+			const withPromise = registry.dispatch( 'store' ).withPromise();
+			await expect( withPromise ).resolves.toEqual( 10 );
+		} );
+		it( 'action generator yielding normal action objects resolves as ' +
+			'expected', async () => {
+			const withNormal = registry.dispatch( 'store' ).withNormal();
+			await expect( withNormal ).resolves.toBeUndefined();
+		} );
+		it( 'action generator returning a non action like value', async () => {
+			const withNonActionLikeValue = registry.dispatch( 'store' )
+				.withNonActionLikeValue();
+			await 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 action object for normal dispatch action', async () => {
+			await expect( registry.dispatch( 'store' ).normal() )
+				.resolves
+				.toEqual( { type: 'NORMAL' } );
+		} );
+	} );
+	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 action object', async () => {
+			await expect( registry.dispatch( 'store' ).normal() )
+				.resolves
+				.toEqual( { type: 'NORMAL' } );
+		} );
+		it( 'action with promise resolving to action returning ' +
+			'action object', async () => {
+			await expect( registry.dispatch( 'store' ).withPromiseAndAction() )
+				.resolves
+				.toEqual( { type: 'WITH_PROMISE' } );
+		} );
+		it( 'action with promise returning non action throws error', async () => {
+			const dispatchedAction = registry.dispatch( 'store' )
+				.withPromiseAndNonAction();
+			await expect( dispatchedAction ).rejects.toThrow(
+				'Actions must be plain objects. Use custom middleware for async actions.'
+			);
+		} );
+	} );
 } );

packages/data/src/test/registry.js

@@ -541,7 +541,7 @@ describe( 'createRegistry', () => {
 	} );
 
 	describe( 'dispatch', () => {
-		it( 'registers actions to the public API', () => {
+		it( 'registers actions to the public API', async () => {
 			const increment = ( count = 1 ) => ( { type: 'increment', count } );
 			const store = registry.registerStore( 'counter', {
 				reducer: ( state = 0, action ) => {
@@ -554,9 +554,14 @@ describe( 'createRegistry', () => {
 					increment,
 				},
 			} );
-
-			const dispatchResult = registry.dispatch( 'counter' ).increment(); // state = 1
-			expect( dispatchResult ).toBe( undefined ); // Actions are implementation detail.
+			// state = 1
+			const dispatchResult = await registry.dispatch( 'counter' ).increment();
+			await expect( dispatchResult ).toEqual(
+				{
+					type: 'increment',
+					count: 1,
+				}
+			);
 			registry.dispatch( 'counter' ).increment( 4 ); // state = 5
 			expect( store.getState() ).toBe( 5 );
 		} );