Skip to content
This repository has been archived by the owner on Jul 9, 2018. It is now read-only.

TRY: Support an optional namespace parameter for hasAction/hasFilter #106

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions packages/hooks/src/createHasHook.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,24 @@
* @param {Object} hooks Stored hooks, keyed by hook name.
*
* @return {Function} Function that returns whether any handlers are
* attached to a particular hook.
* attached to a particular hook and optional namespace.
*/
function createHasHook( hooks ) {
/**
* Returns how many handlers are attached for the given hook.
* Returns whether any handlers are attached for the given hookName and optional namespace.
*
* @param {string} hookName The name of the hook to check for.
* @param {string} hookName The name of the hook to check for.
* @param {string} namespace Optional. The unique namespace identifying the callback in the form `vendor/plugin/function`.
*
* @return {boolean} Whether there are handlers that are attached to the given hook.
*/
return function hasHook( hookName ) {
return function hasHook( hookName, namespace ) {
// Use the namespace if provided.
if ( 'undefined' !== typeof namespace ){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style: Space before opening curly brace.

return hookName in hooks &&
hooks[ hookName ].handlers.some( hook => hook.namespace === namespace );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}

return hookName in hooks;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: Regardless of whether namespace is provided, we have this same condition apply, so rather than duplicate, we could move it as a primary check of the function:

if ( ! ( hookName in hooks ) ) {
	return false;
}

if ( 'undefined' === typeof namespace ) {
	return true;
}

return hooks[ hookName ].handlers.some( ( hook ) => {
	return hook.namespace === namespace;
} );

};
}
Expand Down
40 changes: 40 additions & 0 deletions packages/hooks/src/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -692,3 +692,43 @@ test( 'removing a filter triggers a hookRemoved action passing all callback deta
'my_callback3'
);
} );

test( 'checking hasAction and hasFilter with named callbacks', () => {

// hasAction tests
addAction( 'test.action', 'my_callback', () => {} );
expect( hasAction( 'test.action', 'not_my_callback' ) ).toBe( false );
expect( hasAction( 'test.action', 'my_callback' ) ).toBe( true );

// test removing
removeAction( 'test.action', 'my_callback' );
expect( hasAction( 'test.action', 'my_callback' ) ).toBe( false );

// test removeAll
addAction( 'test.action', 'my_callback', () => {} );
addAction( 'test.action', 'my_second_callback', () => {} );
expect( hasAction( 'test.action', 'my_callback' ) ).toBe( true );
expect( hasAction( 'test.action', 'my_callback' ) ).toBe( true );
removeAllActions( 'test.action' );
expect( hasAction( 'test.action', 'my_callback' ) ).toBe( false );
expect( hasAction( 'test.action', 'my_callback' ) ).toBe( false );

// hasFilter tests
addFilter( 'test.filter', 'my_callback', () => {} );
expect( hasFilter( 'test.filter', 'not_my_callback' ) ).toBe( false );
expect( hasFilter( 'test.filter', 'my_callback' ) ).toBe( true );

// test removing
removeFilter( 'test.filter', 'my_callback' );
expect( hasFilter( 'test.filter', 'my_callback' ) ).toBe( false );

// test removeAll
addFilter( 'test.filter', 'my_callback', () => {} );
addFilter( 'test.filter', 'my_second_callback', () => {} );
expect( hasFilter( 'test.filter', 'my_callback' ) ).toBe( true );
expect( hasFilter( 'test.filter', 'my_second_callback' ) ).toBe( true );
removeAllFilters( 'test.filter' );
expect( hasFilter( 'test.filter', 'my_callback' ) ).toBe( false );
expect( hasFilter( 'test.filter', 'my_second_callback' ) ).toBe( false );

} );