Make Navigation fallback selector private (#51413)

* Move selector to become private

* adds basic lock functionality

* remove useless lock-unlock file

* map private selectors to resolvers

* Unlock the other usage

* only create one fallback per session

* Fix core-data duplicate private opt-in

* Data: bind resolvers to selectors individually, support private selectors

---------

Co-authored-by: Andrei Draganescu <andrei.draganescu@automattic.com>
Co-authored-by: scruffian <ben@scruffian.com>
Co-authored-by: Jarda Snajdr <jsnajdr@gmail.com>

Author: 4 people

docs/reference-guides/data/data-core.md

@@ -329,18 +329,6 @@ _Returns_
 
 -   `any`: The entity record's save error.
 
-### getNavigationFallbackId
-
-Retrieve the fallback Navigation.
-
-_Parameters_
-
--   _state_ `State`: Data state.
-
-_Returns_
-
--   `EntityRecordKey | undefined`: The ID for the fallback Navigation post.
-
 ### getRawEntityRecord
 
 Returns the entity's record object by key, with its attributes mapped to their raw values.

packages/block-library/src/navigation/edit/index.js

@@ -69,7 +69,6 @@ import ManageMenusButton from './manage-menus-button';
 import MenuInspectorControls from './menu-inspector-controls';
 import DeletedNavigationWarning from './deleted-navigation-warning';
 import { unlock } from '../../lock-unlock';
-
 const { useBlockEditingMode } = unlock( blockEditorPrivateApis );
 
 function Navigation( {
@@ -224,7 +223,7 @@ function Navigation( {
 	// that automatically saves the menu as an entity when changes are made to the inner blocks.
 	const hasUnsavedBlocks = hasUncontrolledInnerBlocks && ! isEntityAvailable;
 
-	const { getNavigationFallbackId } = useSelect( coreStore );
+	const { getNavigationFallbackId } = unlock( useSelect( coreStore ) );
 
 	const navigationFallbackId = ! ( ref || hasUnsavedBlocks )
 		? getNavigationFallbackId()

packages/core-data/README.md

@@ -506,18 +506,6 @@ _Returns_
 
 -   `any`: The entity record's save error.
 
-### getNavigationFallbackId
-
-Retrieve the fallback Navigation.
-
-_Parameters_
-
--   _state_ `State`: Data state.
-
-_Returns_
-
--   `EntityRecordKey | undefined`: The ID for the fallback Navigation post.
-
 ### getRawEntityRecord
 
 Returns the entity's record object by key, with its attributes mapped to their raw values.

packages/core-data/src/index.js

@@ -13,6 +13,8 @@ import * as resolvers from './resolvers';
 import createLocksActions from './locks/actions';
 import { rootEntitiesConfig, getMethodName } from './entities';
 import { STORE_NAME } from './name';
+import { unlock } from './private-apis';
+import { getNavigationFallbackId } from './private-selectors';
 
 // The entity selectors/resolvers and actions are shortcuts to their generic equivalents
 // (getEntityRecord, getEntityRecords, updateEntityRecord, updateEntityRecords)
@@ -62,7 +64,10 @@ const storeConfig = () => ( {
  * @see https://github.com/WordPress/gutenberg/blob/HEAD/packages/data/README.md#createReduxStore
  */
 export const store = createReduxStore( STORE_NAME, storeConfig() );
-register( store );
+unlock( store ).registerPrivateSelectors( {
+	getNavigationFallbackId,
+} );
+register( store ); // Register store after unlocking private selectors to allow resolvers to use them.
 
 export { default as EntityProvider } from './entity-provider';
 export * from './entity-provider';

packages/core-data/src/private-selectors.ts

@@ -4,6 +4,7 @@
 import type { State, UndoEdit } from './selectors';
 
 type Optional< T > = T | undefined;
+type EntityRecordKey = string | number;
 
 /**
  * Returns the previous edit from the current undo offset
@@ -28,3 +29,15 @@ export function getUndoEdits( state: State ): Optional< UndoEdit[] > {
 export function getRedoEdits( state: State ): Optional< UndoEdit[] > {
 	return state.undo.list[ state.undo.list.length + state.undo.offset ];
 }
+
+/**
+ * Retrieve the fallback Navigation.
+ *
+ * @param state Data state.
+ * @return The ID for the fallback Navigation post.
+ */
+export function getNavigationFallbackId(
+	state: State
+): EntityRecordKey | undefined {
+	return state.navigationFallbackId;
+}

packages/core-data/src/selectors.ts

@@ -1257,18 +1257,6 @@ export function getBlockPatternCategories( state: State ): Array< any > {
 	return state.blockPatternCategories;
 }
 
-/**
- * Retrieve the fallback Navigation.
- *
- * @param state Data state.
- * @return The ID for the fallback Navigation post.
- */
-export function getNavigationFallbackId(
-	state: State
-): EntityRecordKey | undefined {
-	return state.navigationFallbackId;
-}
-
 /**
  * Returns the revisions of the current global styles theme.
  *

packages/data/src/redux-store/index.js

@@ -105,10 +105,10 @@ function createBindingCache( bind ) {
 	const cache = new WeakMap();
 
 	return {
-		get( item ) {
+		get( item, itemName ) {
 			let boundItem = cache.get( item );
 			if ( ! boundItem ) {
-				boundItem = bind( item );
+				boundItem = bind( item, itemName );
 				cache.set( item, boundItem );
 			}
 			return boundItem;
@@ -198,7 +198,7 @@ export default function createReduxStore( key, options ) {
 				get: ( target, prop ) => {
 					const privateAction = privateActions[ prop ];
 					return privateAction
-						? boundPrivateActions.get( privateAction )
+						? boundPrivateActions.get( privateAction, prop )
 						: actions[ prop ];
 				},
 			} );
@@ -210,16 +210,32 @@ export default function createReduxStore( key, options ) {
 
 			lock( actions, allActions );
 
-			function bindSelector( selector ) {
+			const resolvers = options.resolvers
+				? mapResolvers( options.resolvers )
+				: {};
+
+			function bindSelector( selector, selectorName ) {
 				if ( selector.isRegistrySelector ) {
 					selector.registry = registry;
 				}
 				const boundSelector = ( ...args ) => {
 					const state = store.__unstableOriginalGetState();
 					return selector( state.root, ...args );
 				};
-				boundSelector.hasResolver = false;
-				return boundSelector;
+
+				const resolver = resolvers[ selectorName ];
+				if ( ! resolver ) {
+					boundSelector.hasResolver = false;
+					return boundSelector;
+				}
+
+				return mapSelectorWithResolver(
+					boundSelector,
+					selectorName,
+					resolver,
+					store,
+					resolversCache
+				);
 			}
 
 			function bindMetadataSelector( selector ) {
@@ -231,35 +247,26 @@ export default function createReduxStore( key, options ) {
 				return boundSelector;
 			}
 
-			let selectors = {
+			const selectors = {
 				...mapValues( metadataSelectors, bindMetadataSelector ),
 				...mapValues( options.selectors, bindSelector ),
 			};
 
-			let resolvers;
-			if ( options.resolvers ) {
-				resolvers = mapResolvers( options.resolvers );
-				selectors = mapSelectorsWithResolvers(
-					selectors,
-					resolvers,
-					store,
-					resolversCache
-				);
-			}
-
 			const boundPrivateSelectors = createBindingCache( bindSelector );
 
 			// Pre-bind the private selectors that have been registered by the time of
 			// instantiation, so that registry selectors are bound to the registry.
-			for ( const privateSelector of Object.values( privateSelectors ) ) {
-				boundPrivateSelectors.get( privateSelector );
+			for ( const [ selectorName, selector ] of Object.entries(
+				privateSelectors
+			) ) {
+				boundPrivateSelectors.get( selector, selectorName );
 			}
 
 			const allSelectors = new Proxy( () => {}, {
 				get: ( target, prop ) => {
 					const privateSelector = privateSelectors[ prop ];
 					return privateSelector
-						? boundPrivateSelectors.get( privateSelector )
+						? boundPrivateSelectors.get( privateSelector, prop )
 						: selectors[ prop ];
 				},
 			} );
@@ -509,22 +516,24 @@ function mapResolvers( resolvers ) {
 }
 
 /**
- * Returns resolvers with matched selectors for a given namespace.
+ * Returns a selector with a matched resolver.
  * Resolvers are side effects invoked once per argument set of a given selector call,
  * used in ensuring that the data needs for the selector are satisfied.
  *
- * @param {Object} selectors      The current selectors to be modified.
- * @param {Object} resolvers      Resolvers to register.
+ * @param {Object} selector       The selector function to be bound.
+ * @param {string} selectorName   The selector name.
+ * @param {Object} resolver       Resolver to call.
  * @param {Object} store          The redux store to which the resolvers should be mapped.
  * @param {Object} resolversCache Resolvers Cache.
  */
-function mapSelectorsWithResolvers(
-	selectors,
-	resolvers,
+function mapSelectorWithResolver(
+	selector,
+	selectorName,
+	resolver,
 	store,
 	resolversCache
 ) {
-	function fulfillSelector( resolver, selectorName, args ) {
+	function fulfillSelector( args ) {
 		const state = store.getState();
 
 		if (
@@ -570,17 +579,10 @@ function mapSelectorsWithResolvers(
 		}, 0 );
 	}
 
-	return mapValues( selectors, ( selector, selectorName ) => {
-		const resolver = resolvers[ selectorName ];
-		if ( ! resolver ) {
-			return selector;
-		}
-
-		const selectorResolver = ( ...args ) => {
-			fulfillSelector( resolver, selectorName, args );
-			return selector( ...args );
-		};
-		selectorResolver.hasResolver = true;
-		return selectorResolver;
-	} );
+	const selectorResolver = ( ...args ) => {
+		fulfillSelector( args );
+		return selector( ...args );
+	};
+	selectorResolver.hasResolver = true;
+	return selectorResolver;
 }

packages/edit-site/src/components/sidebar-navigation-screen-navigation-menus/index.js

@@ -21,6 +21,7 @@ import { PRELOADED_NAVIGATION_MENUS_QUERY } from './constants';
 import { useLink } from '../routes/link';
 import SingleNavigationMenu from '../sidebar-navigation-screen-navigation-menu/single-navigation-menu';
 import useNavigationMenuHandlers from '../sidebar-navigation-screen-navigation-menu/use-navigation-menu-handlers';
+import { unlock } from '../../lock-unlock';
 
 // Copied from packages/block-library/src/navigation/edit/navigation-menu-selector.js.
 function buildMenuLabel( title, id, status ) {
@@ -41,6 +42,9 @@ function buildMenuLabel( title, id, status ) {
 	);
 }
 
+// Save a boolean to prevent us creating a fallback more than once per session.
+let hasCreatedFallback = false;
+
 export default function SidebarNavigationScreenNavigationMenus() {
 	const {
 		records: navigationMenus,
@@ -55,18 +59,22 @@ export default function SidebarNavigationScreenNavigationMenus() {
 	const isLoading =
 		isResolvingNavigationMenus && ! hasResolvedNavigationMenus;
 
-	const getNavigationFallbackId = useSelect(
-		( select ) => select( coreStore ).getNavigationFallbackId
-	);
+	const { getNavigationFallbackId } = unlock( useSelect( coreStore ) );
 
 	const firstNavigationMenu = navigationMenus?.[ 0 ];
 
+	// Save a boolean to prevent us creating a fallback more than once per session.
+	if ( firstNavigationMenu ) {
+		hasCreatedFallback = true;
+	}
+
 	// If there is no navigation menu found
 	// then trigger fallback algorithm to create one.
 	if (
 		! firstNavigationMenu &&
 		! isResolvingNavigationMenus &&
-		hasResolvedNavigationMenus
+		hasResolvedNavigationMenus &&
+		! hasCreatedFallback
 	) {
 		getNavigationFallbackId();
 	}