From e743ff12c9ed29030bed3d4cb7b4f047267eb323 Mon Sep 17 00:00:00 2001 From: Adam Silverstein Date: Mon, 9 Apr 2018 15:02:51 -0400 Subject: [PATCH 1/4] return a boolean from `hasHook` not the hook count --- packages/hooks/src/createHasHook.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/hooks/src/createHasHook.js b/packages/hooks/src/createHasHook.js index f471f7e08579d5..b20eacf2c4a4c8 100644 --- a/packages/hooks/src/createHasHook.js +++ b/packages/hooks/src/createHasHook.js @@ -13,13 +13,13 @@ function createHasHook( hooks ) { * * @param {string} hookName The name of the hook to check for. * - * @return {number} The number of handlers that are attached to + * @return {boolean} Whether there are handlers that are attached to * the given hook. */ return function hasHook( hookName ) { return hooks[ hookName ] - ? hooks[ hookName ].handlers.length - : 0; + ? true + : false; }; } From 2b3c8bfb69875676ad347387d7ffb7584e3065aa Mon Sep 17 00:00:00 2001 From: Adam Silverstein Date: Mon, 9 Apr 2018 15:15:19 -0400 Subject: [PATCH 2/4] simplify logic --- packages/hooks/src/createHasHook.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/hooks/src/createHasHook.js b/packages/hooks/src/createHasHook.js index b20eacf2c4a4c8..098be8033c3eec 100644 --- a/packages/hooks/src/createHasHook.js +++ b/packages/hooks/src/createHasHook.js @@ -17,9 +17,7 @@ function createHasHook( hooks ) { * the given hook. */ return function hasHook( hookName ) { - return hooks[ hookName ] - ? true - : false; + return hooks[ hookName ] ? true : false; }; } From b4b85687b3007cddd32ba9bf8f9e2147973d6eb4 Mon Sep 17 00:00:00 2001 From: Adam Silverstein Date: Mon, 9 Apr 2018 15:36:44 -0400 Subject: [PATCH 3/4] update tests to expect booleans from hasFilter/Action --- packages/hooks/src/test/index.test.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/hooks/src/test/index.test.js b/packages/hooks/src/test/index.test.js index bcd68e70461fcf..c53611bce18610 100644 --- a/packages/hooks/src/test/index.test.js +++ b/packages/hooks/src/test/index.test.js @@ -454,7 +454,7 @@ test( 'Test doingAction, didAction and hasAction.', () => { expect( doingAction( 'test.action' ) ).toBe( false ); expect( didAction( 'test.action' ) ).toBe( 0 ); - expect( hasAction( 'test.action' ) ).toBe( 0 ); + expect( hasAction( 'test.action' ) ).toBe( false ); addAction( 'test.action', 'my_callback', () => { actionCalls++; @@ -466,7 +466,7 @@ test( 'Test doingAction, didAction and hasAction.', () => { // Verify action added, not running yet. expect( doingAction( 'test.action' ) ).toBe( false ); expect( didAction( 'test.action' ) ).toBe( 0 ); - expect( hasAction( 'test.action' ) ).toBe( 1 ); + expect( hasAction( 'test.action' ) ).toBe( true ); doAction( 'test.action' ); @@ -474,7 +474,7 @@ test( 'Test doingAction, didAction and hasAction.', () => { expect( actionCalls ).toBe( 1 ); expect( doingAction( 'test.action' ) ).toBe( false ); expect( didAction( 'test.action' ) ).toBe( 1 ); - expect( hasAction( 'test.action' ) ).toBe( 1 ); + expect( hasAction( 'test.action' ) ).toBe( true ); expect( doingAction() ).toBe( false ); expect( doingAction( 'test.action' ) ).toBe( false ); expect( doingAction( 'notatest.action' ) ).toBe( false ); @@ -489,13 +489,13 @@ test( 'Test doingAction, didAction and hasAction.', () => { // Verify state is reset appropriately. expect( doingAction( 'test.action' ) ).toBe( false ); expect( didAction( 'test.action' ) ).toBe( 2 ); - expect( hasAction( 'test.action' ) ).toBe( 0 ); + expect( hasAction( 'test.action' ) ).toBe( true ); doAction( 'another.action' ); expect( doingAction( 'test.action' ) ).toBe( false ); // Verify hasAction returns 0 when no matching action. - expect( hasAction( 'notatest.action' ) ).toBe( 0 ); + expect( hasAction( 'notatest.action' ) ).toBe( false ); } ); test( 'Verify doingFilter, didFilter and hasFilter.', () => { @@ -514,8 +514,8 @@ test( 'Verify doingFilter, didFilter and hasFilter.', () => { expect( test ).toBe( 'someValue' ); expect( filterCalls ).toBe( 1 ); expect( didFilter( 'runtest.filter' ) ).toBe( 1 ); - expect( hasFilter( 'runtest.filter' ) ).toBe( 1 ); - expect( hasFilter( 'notatest.filter' ) ).toBe( 0 ); + expect( hasFilter( 'runtest.filter' ) ).toBe( true ); + expect( hasFilter( 'notatest.filter' ) ).toBe( false ); expect( doingFilter() ).toBe( false ); expect( doingFilter( 'runtest.filter' ) ).toBe( false ); expect( doingFilter( 'notatest.filter' ) ).toBe( false ); @@ -523,7 +523,7 @@ test( 'Verify doingFilter, didFilter and hasFilter.', () => { expect( removeAllFilters( 'runtest.filter' ) ).toBe( 1 ); - expect( hasFilter( 'runtest.filter' ) ).toBe( 0 ); + expect( hasFilter( 'runtest.filter' ) ).toBe( true ); expect( didFilter( 'runtest.filter' ) ).toBe( 1 ); } ); From d166c75bf564ea86eda5c89264bb50bb4cd14b5a Mon Sep 17 00:00:00 2001 From: Adam Silverstein Date: Mon, 9 Apr 2018 15:39:51 -0400 Subject: [PATCH 4/4] Clean up docs and return --- packages/hooks/src/createHasHook.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/hooks/src/createHasHook.js b/packages/hooks/src/createHasHook.js index 098be8033c3eec..de4ee3c17a793c 100644 --- a/packages/hooks/src/createHasHook.js +++ b/packages/hooks/src/createHasHook.js @@ -13,11 +13,10 @@ function createHasHook( hooks ) { * * @param {string} hookName The name of the hook to check for. * - * @return {boolean} Whether there are handlers that are attached to - * the given hook. + * @return {boolean} Whether there are handlers that are attached to the given hook. */ return function hasHook( hookName ) { - return hooks[ hookName ] ? true : false; + return hookName in hooks; }; }