From 6637b54470eaaa69b6d6d58b5cfb2a07e6c78d26 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 14 May 2021 11:22:37 +0100 Subject: [PATCH 1/7] Moving managing data for Navigation entities into hook --- .../src/navigation/placeholder.js | 72 +----------- .../src/navigation/use-navigation-entities.js | 109 ++++++++++++++++++ 2 files changed, 114 insertions(+), 67 deletions(-) create mode 100644 packages/block-library/src/navigation/use-navigation-entities.js diff --git a/packages/block-library/src/navigation/placeholder.js b/packages/block-library/src/navigation/placeholder.js index ccf46229e2a14..03b942f3b30bd 100644 --- a/packages/block-library/src/navigation/placeholder.js +++ b/packages/block-library/src/navigation/placeholder.js @@ -10,7 +10,7 @@ import { MenuItem, Spinner, } from '@wordpress/components'; -import { useSelect } from '@wordpress/data'; + import { forwardRef, useCallback, @@ -19,12 +19,11 @@ import { } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; import { navigation, chevronDown, Icon } from '@wordpress/icons'; -import { store as coreStore } from '@wordpress/core-data'; /** * Internal dependencies */ - +import useNavigationEntities from './use-navigation-entities'; import PlaceholderPreview from './placeholder-preview'; import menuItemsToBlocks from './menu-items-to-blocks'; @@ -34,76 +33,15 @@ function NavigationPlaceholder( { onCreate }, ref ) { const [ isCreatingFromMenu, setIsCreatingFromMenu ] = useState( false ); const { - pages, isResolvingPages, - hasResolvedPages, menus, isResolvingMenus, - hasResolvedMenus, menuItems, hasResolvedMenuItems, - } = useSelect( - ( select ) => { - const { - getEntityRecords, - getMenus, - getMenuItems, - isResolving, - hasFinishedResolution, - } = select( coreStore ); - const pagesParameters = [ - 'postType', - 'page', - { - parent: 0, - order: 'asc', - orderby: 'id', - per_page: -1, - }, - ]; - const menusParameters = [ { per_page: -1 } ]; - const hasSelectedMenu = selectedMenu !== undefined; - const menuItemsParameters = hasSelectedMenu - ? [ - { - menus: selectedMenu, - per_page: -1, - }, - ] - : undefined; - - return { - pages: getEntityRecords( ...pagesParameters ), - isResolvingPages: isResolving( - 'getEntityRecords', - pagesParameters - ), - hasResolvedPages: hasFinishedResolution( - 'getEntityRecords', - pagesParameters - ), - menus: getMenus( ...menusParameters ), - isResolvingMenus: isResolving( 'getMenus', menusParameters ), - hasResolvedMenus: hasFinishedResolution( - 'getMenus', - menusParameters - ), - menuItems: hasSelectedMenu - ? getMenuItems( ...menuItemsParameters ) - : undefined, - hasResolvedMenuItems: hasSelectedMenu - ? hasFinishedResolution( - 'getMenuItems', - menuItemsParameters - ) - : false, - }; - }, - [ selectedMenu ] - ); + hasPages, + hasMenus, + } = useNavigationEntities( selectedMenu ); - const hasPages = !! ( hasResolvedPages && pages?.length ); - const hasMenus = !! ( hasResolvedMenus && menus?.length ); const isLoading = isResolvingPages || isResolvingMenus; const createFromMenu = useCallback( () => { diff --git a/packages/block-library/src/navigation/use-navigation-entities.js b/packages/block-library/src/navigation/use-navigation-entities.js new file mode 100644 index 0000000000000..c16ea883189ef --- /dev/null +++ b/packages/block-library/src/navigation/use-navigation-entities.js @@ -0,0 +1,109 @@ +/** + * WordPress dependencies + */ +import { useSelect } from '@wordpress/data'; +import { store as coreStore } from '@wordpress/core-data'; + +/** + * @typedef {Object} NavigationEntitiesData + * @property {Array|undefined} pages - a collection of WP Post entity objects of post type "Page". + * @property {boolean} isResolvingPages - indicates whether the request to fetch pages is currently resolving. + * @property {boolean} hasResolvedPages - indicates whether the request to fetch pages has finished resolving. + * @property {Array|undefined} menus - a collection of Menu entity objects. + * @property {boolean} isResolvingMenus - indicates whether the request to fetch menus is currently resolving. + * @property {boolean} hasResolvedMenus - indicates whether the request to fetch menus has finished resolving. + * @property {Array|undefined} menusItems - a collection of Menu Item entity objects for the current menuId. + * @property {boolean} hasResolvedMenuItems - indicates whether the request to fetch menuItems has finished resolving. + */ + +/** + * Manages fetching and resolution state for all entities required + * for the Navigation block. + * + * @param {number} menuId the menu for which to retrieve menuItem data. + * @return { NavigationEntitiesData } the entity data. + */ +export default function useNavigationEntities( menuId ) { + const { + pages, + isResolvingPages, + hasResolvedPages, + menus, + isResolvingMenus, + hasResolvedMenus, + menuItems, + hasResolvedMenuItems, + } = useSelect( + ( select ) => { + const { + getEntityRecords, + getMenus, + getMenuItems, + isResolving, + hasFinishedResolution, + } = select( coreStore ); + + const pagesParameters = [ + 'postType', + 'page', + { + parent: 0, + order: 'asc', + orderby: 'id', + per_page: -1, + }, + ]; + const menusParameters = [ { per_page: -1 } ]; + const hasSelectedMenu = menuId !== undefined; + const menuItemsParameters = hasSelectedMenu + ? [ + { + menus: menuId, + per_page: -1, + }, + ] + : undefined; + + return { + pages: getEntityRecords( ...pagesParameters ), + isResolvingPages: isResolving( + 'getEntityRecords', + pagesParameters + ), + hasResolvedPages: hasFinishedResolution( + 'getEntityRecords', + pagesParameters + ), + menus: getMenus( ...menusParameters ), + isResolvingMenus: isResolving( 'getMenus', menusParameters ), + hasResolvedMenus: hasFinishedResolution( + 'getMenus', + menusParameters + ), + menuItems: hasSelectedMenu + ? getMenuItems( ...menuItemsParameters ) + : undefined, + hasResolvedMenuItems: hasSelectedMenu + ? hasFinishedResolution( + 'getMenuItems', + menuItemsParameters + ) + : false, + }; + }, + [ menuId ] + ); + + return { + pages, + isResolvingPages, + hasResolvedPages, + menus, + isResolvingMenus, + hasResolvedMenus, + menuItems, + hasResolvedMenuItems, + hasPages: !! ( hasResolvedPages && pages?.length ), + hasMenus: !! ( hasResolvedMenus && menus?.length ), + }; +} From 86c4f7d7c44dd6fa5d6035367ef4ac6b3defd5b1 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 14 May 2021 11:36:00 +0100 Subject: [PATCH 2/7] Refactor to individual hooks for pages and menus entities --- .../src/navigation/use-navigation-entities.js | 78 ++++++++++++------- 1 file changed, 51 insertions(+), 27 deletions(-) diff --git a/packages/block-library/src/navigation/use-navigation-entities.js b/packages/block-library/src/navigation/use-navigation-entities.js index c16ea883189ef..74cefb1690974 100644 --- a/packages/block-library/src/navigation/use-navigation-entities.js +++ b/packages/block-library/src/navigation/use-navigation-entities.js @@ -14,6 +14,8 @@ import { store as coreStore } from '@wordpress/core-data'; * @property {boolean} hasResolvedMenus - indicates whether the request to fetch menus has finished resolving. * @property {Array|undefined} menusItems - a collection of Menu Item entity objects for the current menuId. * @property {boolean} hasResolvedMenuItems - indicates whether the request to fetch menuItems has finished resolving. + * @property {boolean} hasPages - indicates whether there is currently any data for pages. + * @property {boolean} hasMenus - indicates whether there is currently any data for menus. */ /** @@ -24,10 +26,14 @@ import { store as coreStore } from '@wordpress/core-data'; * @return { NavigationEntitiesData } the entity data. */ export default function useNavigationEntities( menuId ) { + return { + ...usePageEntities(), + ...useMenuEntities( menuId ), + }; +} + +function useMenuEntities( menuId ) { const { - pages, - isResolvingPages, - hasResolvedPages, menus, isResolvingMenus, hasResolvedMenus, @@ -36,23 +42,12 @@ export default function useNavigationEntities( menuId ) { } = useSelect( ( select ) => { const { - getEntityRecords, getMenus, getMenuItems, isResolving, hasFinishedResolution, } = select( coreStore ); - const pagesParameters = [ - 'postType', - 'page', - { - parent: 0, - order: 'asc', - orderby: 'id', - per_page: -1, - }, - ]; const menusParameters = [ { per_page: -1 } ]; const hasSelectedMenu = menuId !== undefined; const menuItemsParameters = hasSelectedMenu @@ -65,15 +60,6 @@ export default function useNavigationEntities( menuId ) { : undefined; return { - pages: getEntityRecords( ...pagesParameters ), - isResolvingPages: isResolving( - 'getEntityRecords', - pagesParameters - ), - hasResolvedPages: hasFinishedResolution( - 'getEntityRecords', - pagesParameters - ), menus: getMenus( ...menusParameters ), isResolvingMenus: isResolving( 'getMenus', menusParameters ), hasResolvedMenus: hasFinishedResolution( @@ -95,15 +81,53 @@ export default function useNavigationEntities( menuId ) { ); return { - pages, - isResolvingPages, - hasResolvedPages, menus, isResolvingMenus, hasResolvedMenus, menuItems, hasResolvedMenuItems, - hasPages: !! ( hasResolvedPages && pages?.length ), hasMenus: !! ( hasResolvedMenus && menus?.length ), }; } + +function usePageEntities() { + const { pages, isResolvingPages, hasResolvedPages } = useSelect( + ( select ) => { + const { + getEntityRecords, + isResolving, + hasFinishedResolution, + } = select( coreStore ); + + const pagesParameters = [ + 'postType', + 'page', + { + parent: 0, + order: 'asc', + orderby: 'id', + per_page: -1, + }, + ]; + + return { + pages: getEntityRecords( ...pagesParameters ), + isResolvingPages: isResolving( + 'getEntityRecords', + pagesParameters + ), + hasResolvedPages: hasFinishedResolution( + 'getEntityRecords', + pagesParameters + ), + }; + } + ); + + return { + pages, + isResolvingPages, + hasResolvedPages, + hasPages: !! ( hasResolvedPages && pages?.length ), + }; +} From 31b2ffb635da2d6689a40d3ff49602a1e69634a1 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 14 May 2021 11:42:12 +0100 Subject: [PATCH 3/7] Refactor menuItem to seperate entity --- .../src/navigation/use-navigation-entities.js | 53 ++++++++++++------- 1 file changed, 33 insertions(+), 20 deletions(-) diff --git a/packages/block-library/src/navigation/use-navigation-entities.js b/packages/block-library/src/navigation/use-navigation-entities.js index 74cefb1690974..2d15fe6b20836 100644 --- a/packages/block-library/src/navigation/use-navigation-entities.js +++ b/packages/block-library/src/navigation/use-navigation-entities.js @@ -28,27 +28,50 @@ import { store as coreStore } from '@wordpress/core-data'; export default function useNavigationEntities( menuId ) { return { ...usePageEntities(), - ...useMenuEntities( menuId ), + ...useMenuEntities(), + ...useMenuItemEntities( menuId ), }; } -function useMenuEntities( menuId ) { +function useMenuEntities() { const { menus, isResolvingMenus, hasResolvedMenus, menuItems, hasResolvedMenuItems, - } = useSelect( + } = useSelect( ( select ) => { + const { getMenus, isResolving, hasFinishedResolution } = select( + coreStore + ); + + const menusParameters = [ { per_page: -1 } ]; + + return { + menus: getMenus( ...menusParameters ), + isResolvingMenus: isResolving( 'getMenus', menusParameters ), + hasResolvedMenus: hasFinishedResolution( + 'getMenus', + menusParameters + ), + }; + } ); + + return { + menus, + isResolvingMenus, + hasResolvedMenus, + menuItems, + hasResolvedMenuItems, + hasMenus: !! ( hasResolvedMenus && menus?.length ), + }; +} + +function useMenuItemEntities( menuId ) { + const { menuItems, hasResolvedMenuItems } = useSelect( ( select ) => { - const { - getMenus, - getMenuItems, - isResolving, - hasFinishedResolution, - } = select( coreStore ); + const { getMenuItems, hasFinishedResolution } = select( coreStore ); - const menusParameters = [ { per_page: -1 } ]; const hasSelectedMenu = menuId !== undefined; const menuItemsParameters = hasSelectedMenu ? [ @@ -60,12 +83,6 @@ function useMenuEntities( menuId ) { : undefined; return { - menus: getMenus( ...menusParameters ), - isResolvingMenus: isResolving( 'getMenus', menusParameters ), - hasResolvedMenus: hasFinishedResolution( - 'getMenus', - menusParameters - ), menuItems: hasSelectedMenu ? getMenuItems( ...menuItemsParameters ) : undefined, @@ -81,12 +98,8 @@ function useMenuEntities( menuId ) { ); return { - menus, - isResolvingMenus, - hasResolvedMenus, menuItems, hasResolvedMenuItems, - hasMenus: !! ( hasResolvedMenus && menus?.length ), }; } From 924a537cb7d0fd62be34e0b64c38a8103087b4ba Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 14 May 2021 12:32:40 +0100 Subject: [PATCH 4/7] Perf optimise with empty dep array on useSelect --- .../block-library/src/navigation/use-navigation-entities.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/block-library/src/navigation/use-navigation-entities.js b/packages/block-library/src/navigation/use-navigation-entities.js index 2d15fe6b20836..75cb3b7650033 100644 --- a/packages/block-library/src/navigation/use-navigation-entities.js +++ b/packages/block-library/src/navigation/use-navigation-entities.js @@ -55,7 +55,7 @@ function useMenuEntities() { menusParameters ), }; - } ); + }, [] ); return { menus, @@ -134,7 +134,8 @@ function usePageEntities() { pagesParameters ), }; - } + }, + [] ); return { From 7a1af8dd1d687ebeb364dd7f240211d839d75d74 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 14 May 2021 12:34:30 +0100 Subject: [PATCH 5/7] Move has* into the select --- .../src/navigation/use-navigation-entities.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/block-library/src/navigation/use-navigation-entities.js b/packages/block-library/src/navigation/use-navigation-entities.js index 75cb3b7650033..b425b22894521 100644 --- a/packages/block-library/src/navigation/use-navigation-entities.js +++ b/packages/block-library/src/navigation/use-navigation-entities.js @@ -40,6 +40,7 @@ function useMenuEntities() { hasResolvedMenus, menuItems, hasResolvedMenuItems, + hasMenus, } = useSelect( ( select ) => { const { getMenus, isResolving, hasFinishedResolution } = select( coreStore @@ -54,6 +55,7 @@ function useMenuEntities() { 'getMenus', menusParameters ), + hasMenus: !! ( hasResolvedMenus && menus?.length ), }; }, [] ); @@ -63,7 +65,7 @@ function useMenuEntities() { hasResolvedMenus, menuItems, hasResolvedMenuItems, - hasMenus: !! ( hasResolvedMenus && menus?.length ), + hasMenus, }; } @@ -104,7 +106,7 @@ function useMenuItemEntities( menuId ) { } function usePageEntities() { - const { pages, isResolvingPages, hasResolvedPages } = useSelect( + const { pages, isResolvingPages, hasResolvedPages, hasPages } = useSelect( ( select ) => { const { getEntityRecords, @@ -133,6 +135,7 @@ function usePageEntities() { 'getEntityRecords', pagesParameters ), + hasPages: !! ( hasResolvedPages && pages?.length ), }; }, [] @@ -142,6 +145,6 @@ function usePageEntities() { pages, isResolvingPages, hasResolvedPages, - hasPages: !! ( hasResolvedPages && pages?.length ), + hasPages, }; } From c8997e736805099d5444cf834bb091d4e2aab26d Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 14 May 2021 12:35:57 +0100 Subject: [PATCH 6/7] Tidy up menu items from menus hook --- .../src/navigation/use-navigation-entities.js | 42 ++++++++----------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/packages/block-library/src/navigation/use-navigation-entities.js b/packages/block-library/src/navigation/use-navigation-entities.js index b425b22894521..d46d0eda976f6 100644 --- a/packages/block-library/src/navigation/use-navigation-entities.js +++ b/packages/block-library/src/navigation/use-navigation-entities.js @@ -34,37 +34,31 @@ export default function useNavigationEntities( menuId ) { } function useMenuEntities() { - const { - menus, - isResolvingMenus, - hasResolvedMenus, - menuItems, - hasResolvedMenuItems, - hasMenus, - } = useSelect( ( select ) => { - const { getMenus, isResolving, hasFinishedResolution } = select( - coreStore - ); + const { menus, isResolvingMenus, hasResolvedMenus, hasMenus } = useSelect( + ( select ) => { + const { getMenus, isResolving, hasFinishedResolution } = select( + coreStore + ); - const menusParameters = [ { per_page: -1 } ]; + const menusParameters = [ { per_page: -1 } ]; - return { - menus: getMenus( ...menusParameters ), - isResolvingMenus: isResolving( 'getMenus', menusParameters ), - hasResolvedMenus: hasFinishedResolution( - 'getMenus', - menusParameters - ), - hasMenus: !! ( hasResolvedMenus && menus?.length ), - }; - }, [] ); + return { + menus: getMenus( ...menusParameters ), + isResolvingMenus: isResolving( 'getMenus', menusParameters ), + hasResolvedMenus: hasFinishedResolution( + 'getMenus', + menusParameters + ), + hasMenus: !! ( hasResolvedMenus && menus?.length ), + }; + }, + [] + ); return { menus, isResolvingMenus, hasResolvedMenus, - menuItems, - hasResolvedMenuItems, hasMenus, }; } From 2c88d090d010dad1dd9ff8af5ae0f98bc95fe279 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 14 May 2021 12:40:26 +0100 Subject: [PATCH 7/7] Fix error caused by moving has* within the useSelect --- .../src/navigation/use-navigation-entities.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/block-library/src/navigation/use-navigation-entities.js b/packages/block-library/src/navigation/use-navigation-entities.js index d46d0eda976f6..03dceb167e9a9 100644 --- a/packages/block-library/src/navigation/use-navigation-entities.js +++ b/packages/block-library/src/navigation/use-navigation-entities.js @@ -34,7 +34,7 @@ export default function useNavigationEntities( menuId ) { } function useMenuEntities() { - const { menus, isResolvingMenus, hasResolvedMenus, hasMenus } = useSelect( + const { menus, isResolvingMenus, hasResolvedMenus } = useSelect( ( select ) => { const { getMenus, isResolving, hasFinishedResolution } = select( coreStore @@ -49,7 +49,6 @@ function useMenuEntities() { 'getMenus', menusParameters ), - hasMenus: !! ( hasResolvedMenus && menus?.length ), }; }, [] @@ -59,7 +58,7 @@ function useMenuEntities() { menus, isResolvingMenus, hasResolvedMenus, - hasMenus, + hasMenus: !! ( hasResolvedMenus && menus?.length ), }; } @@ -100,7 +99,7 @@ function useMenuItemEntities( menuId ) { } function usePageEntities() { - const { pages, isResolvingPages, hasResolvedPages, hasPages } = useSelect( + const { pages, isResolvingPages, hasResolvedPages } = useSelect( ( select ) => { const { getEntityRecords, @@ -120,7 +119,7 @@ function usePageEntities() { ]; return { - pages: getEntityRecords( ...pagesParameters ), + pages: getEntityRecords( ...pagesParameters ) || null, isResolvingPages: isResolving( 'getEntityRecords', pagesParameters @@ -129,7 +128,6 @@ function usePageEntities() { 'getEntityRecords', pagesParameters ), - hasPages: !! ( hasResolvedPages && pages?.length ), }; }, [] @@ -139,6 +137,6 @@ function usePageEntities() { pages, isResolvingPages, hasResolvedPages, - hasPages, + hasPages: !! ( hasResolvedPages && pages?.length ), }; }