Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Data: Restore dispatch actions returning actions that are a Promise #14830

Merged
merged 12 commits into from
Apr 12, 2019
3 changes: 2 additions & 1 deletion packages/data/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

### 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)
- 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)

Expand Down
21 changes: 16 additions & 5 deletions packages/data/src/components/with-dispatch/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 } /> );
Expand Down Expand Up @@ -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' ) {
Expand All @@ -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 } /> );
Expand Down
6 changes: 1 addition & 5 deletions packages/data/src/namespace-store/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
mapValues,
} from 'lodash';
import combineReducers from 'turbo-combine-reducers';
import isPromise from 'is-promise';

/**
* WordPress dependencies
Expand Down Expand Up @@ -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 );
Expand Down
37 changes: 22 additions & 15 deletions packages/data/src/namespace-store/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,19 +110,19 @@ describe( 'controls', () => {
} );
} );
it( 'action generator returning a yielded promise control descriptor ' +
'resolves as expected', () => {
'resolves as expected', async () => {
const withPromise = registry.dispatch( 'store' ).withPromise();
expect( withPromise ).resolves.toEqual( 10 );
await expect( withPromise ).resolves.toEqual( 10 );
} );
it( 'action generator yielding normal action objects resolves as ' +
'expected', () => {
'expected', async () => {
const withNormal = registry.dispatch( 'store' ).withNormal();
expect( withNormal ).resolves.toBeUndefined();
await expect( withNormal ).resolves.toBeUndefined();
} );
it( 'action generator returning a non action like value', () => {
it( 'action generator returning a non action like value', async () => {
const withNonActionLikeValue = registry.dispatch( 'store' )
.withNonActionLikeValue();
expect( withNonActionLikeValue ).resolves.toEqual( 10 );
await expect( withNonActionLikeValue ).resolves.toEqual( 10 );
} );
it( 'normal dispatch action throwing error because no action ' +
'returned', () => {
Expand All @@ -131,8 +131,10 @@ describe( 'controls', () => {
'Actions must be plain objects. Use custom middleware for async actions.'
);
} );
it( 'returns undefined for normal dispatch action', () => {
expect( registry.dispatch( 'store' ).normal() ).toBeUndefined();
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 ' +
Expand All @@ -152,18 +154,23 @@ describe( 'controls', () => {
actions,
} );
} );
it( 'normal action returns undefined', () => {
expect( registry.dispatch( 'store' ).normal() ).toBeUndefined();
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 undefined', () => {
expect( registry.dispatch( 'store' ).withPromiseAndAction() )
it( 'action with promise resolving to action returning ' +
'action object', async () => {
await expect( registry.dispatch( 'store' ).withPromiseAndAction() )
.resolves
.toBeUndefined();
.toEqual( { type: 'WITH_PROMISE' } );
} );
it( 'action with promise returning non action throws error', () => {
it( 'action with promise returning non action throws error', async () => {
const dispatchedAction = registry.dispatch( 'store' )
.withPromiseAndNonAction();
expect( dispatchedAction ).resolves.toBe( 10 );
await expect( dispatchedAction ).rejects.toThrow(
'Actions must be plain objects. Use custom middleware for async actions.'
);
} );
} );
} );
13 changes: 9 additions & 4 deletions packages/data/src/test/registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) => {
Expand All @@ -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 );
} );
Expand Down