modify emitters so it handles non object or invalid object responses correctly. (https://github.com/woocommerce/woocommerce-blocks/pull/2249)

With these changes:

- If an observer returns an object wthout a type property an error is thrown and the emitter is aborted with an error type response.
- For anything else returned from an observer, it’s discarded.

Tests are updated for the new expectations
This commit is contained in:
Darren Ethier 2020-04-20 22:33:16 -04:00 committed by GitHub
parent a005649ab8
commit 493a826e44
2 changed files with 65 additions and 22 deletions

View File

@ -58,18 +58,29 @@ export const emitEvent = async ( observers, eventType, data ) => {
*/ */
export const emitEventWithAbort = async ( observers, eventType, data ) => { export const emitEventWithAbort = async ( observers, eventType, data ) => {
const observersByType = getObserversByPriority( observers, eventType ); const observersByType = getObserversByPriority( observers, eventType );
let emitterResponse = true;
for ( const observer of observersByType ) { for ( const observer of observersByType ) {
try { try {
const response = await Promise.resolve( observer.callback( data ) ); const response = await Promise.resolve( observer.callback( data ) );
if ( response !== true ) { if (
return response; typeof response === 'object' &&
typeof response.type === 'undefined'
) {
throw new Error(
'If you want to abort event emitter processing, your observer must return an object with a type property'
);
}
emitterResponse = typeof response === 'object' ? response : true;
if ( emitterResponse !== true ) {
return emitterResponse;
} }
} catch ( e ) { } catch ( e ) {
// we don't handle thrown errors but just console.log for // we don't handle thrown errors but just console.log for
// troubleshooting // troubleshooting
// eslint-disable-next-line no-console // eslint-disable-next-line no-console
console.error( e ); console.error( e );
return { type: 'error' };
} }
} }
return true; return emitterResponse;
}; };

View File

@ -43,10 +43,8 @@ describe( 'Testing emitters', () => {
} ); } );
} ); } );
describe( 'Testing emitEventWithAbort()', () => { describe( 'Testing emitEventWithAbort()', () => {
it( it( 'does not abort on any return value other than an object with a type property', async () => {
'aborts on non truthy value and does not invoke remaining ' + observerMocks.delete( 'observerPromiseWithReject' );
'observers',
async () => {
const observers = { test: observerMocks }; const observers = { test: observerMocks };
const response = await emitEventWithAbort( const response = await emitEventWithAbort(
observers, observers,
@ -55,17 +53,51 @@ describe( 'Testing emitters', () => {
); );
expect( console ).not.toHaveErrored(); expect( console ).not.toHaveErrored();
expect( observerB ).toHaveBeenCalledTimes( 1 ); expect( observerB ).toHaveBeenCalledTimes( 1 );
expect( expect( observerPromiseWithResolvedValue ).toHaveBeenCalled();
observerPromiseWithResolvedValue expect( response ).toBe( true );
).not.toHaveBeenCalled(); } );
expect( response ).toBe( 10 ); it( 'Aborts on a return value with an object that has a type property', async () => {
} const validObjectResponse = jest
.fn()
.mockReturnValue( { type: 'success' } );
observerMocks.set( 'observerValidObject', {
priority: 5,
callback: validObjectResponse,
} );
observerMocks.delete( 'observerPromiseWithReject' );
const observers = { test: observerMocks };
const response = await emitEventWithAbort(
observers,
'test',
'foo'
); );
expect( console ).not.toHaveErrored();
expect( validObjectResponse ).toHaveBeenCalledTimes( 1 );
expect( observerPromiseWithResolvedValue ).not.toHaveBeenCalled();
expect( response ).toEqual( { type: 'success' } );
} );
it( 'throws an error on an object returned from observer without a type property', async () => {
const failingObjectResponse = jest.fn().mockReturnValue( {} );
observerMocks.set( 'observerInvalidObject', {
priority: 5,
callback: failingObjectResponse,
} );
const observers = { test: observerMocks };
const response = await emitEventWithAbort(
observers,
'test',
'foo'
);
expect( console ).toHaveErrored();
expect( failingObjectResponse ).toHaveBeenCalledTimes( 1 );
expect( observerPromiseWithResolvedValue ).not.toHaveBeenCalled();
expect( response ).toEqual( { type: 'error' } );
} );
} ); } );
describe( 'Test Priority', () => { describe( 'Test Priority', () => {
it( 'executes observers in expected order by priority', async () => { it( 'executes observers in expected order by priority', async () => {
const a = jest.fn(); const a = jest.fn();
const b = jest.fn().mockReturnValue( false ); const b = jest.fn().mockReturnValue( { type: 'error' } );
const observers = { const observers = {
test: new Map( [ test: new Map( [
[ 'observerA', { priority: 200, callback: a } ], [ 'observerA', { priority: 200, callback: a } ],