Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make Navigation fallback selector private #51413

Merged
merged 8 commits into from
Jun 29, 2023
Merged

Make Navigation fallback selector private #51413

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
45b53e9
Move selector to become private
getdave Jun 12, 2023
8a3cf4c
adds basic lock functionality
draganescu Jun 26, 2023
77d7ffc
remove useless lock-unlock file
draganescu Jun 26, 2023
00844b5
map private selectors to resolvers
draganescu Jun 27, 2023
d64c73d
Unlock the other usage
getdave Jun 28, 2023
06ad9ce
only create one fallback per session
scruffian Jun 28, 2023
e990986
Fix core-data duplicate private opt-in
jsnajdr Jun 29, 2023
cea2b77
Data: bind resolvers to selectors individually, support private selec…
jsnajdr Jun 29, 2023
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
12 changes: 0 additions & 12 deletions docs/reference-guides/data/data-core.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 1 addition & 2 deletions packages/block-library/src/navigation/edit/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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( {
Expand Down Expand Up @@ -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()
Expand Down
12 changes: 0 additions & 12 deletions packages/core-data/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
7 changes: 6 additions & 1 deletion packages/core-data/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 './lock-unlock';
import { getNavigationFallbackId } from './private-selectors';

// The entity selectors/resolvers and actions are shortcuts to their generic equivalents
// (getEntityRecord, getEntityRecords, updateEntityRecord, updateEntityRecords)
Expand Down Expand Up @@ -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';
Expand Down
10 changes: 10 additions & 0 deletions packages/core-data/src/lock-unlock.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/**
* WordPress dependencies
*/
import { __dangerousOptInToUnstableAPIsOnlyForCoreModules } from '@wordpress/private-apis';

export const { lock, unlock } =
__dangerousOptInToUnstableAPIsOnlyForCoreModules(
'I know using unstable features means my plugin or theme will inevitably break on the next WordPress release.',
'@wordpress/data'
);
13 changes: 13 additions & 0 deletions packages/core-data/src/private-selectors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}
12 changes: 0 additions & 12 deletions packages/core-data/src/selectors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
8 changes: 7 additions & 1 deletion packages/data/src/redux-store/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ function createBindingCache( bind ) {
*/
export default function createReduxStore( key, options ) {
const privateActions = {};
const privateSelectors = {};
let privateSelectors = {};
const privateRegistrationFunctions = {
privateActions,
registerPrivateActions: ( actions ) => {
Expand Down Expand Up @@ -259,6 +259,12 @@ export default function createReduxStore( key, options ) {
store,
resolversCache
);
privateSelectors = mapSelectorsWithResolvers(
privateSelectors,
resolvers,
store,
resolversCache
);
Copy link
Member

Choose a reason for hiding this comment

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

great fix, and thanks for following up on this @draganescu

Copy link
Member

Choose a reason for hiding this comment

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

Yes, this is probably the first time we're creating a private selector with a resolver 👍 However, this fix won't work when you register a private selector after a store got instantiated:

const store = createReduxStore( name, { ..., resolvers: { foo } );
register( store ); // instance is created and resolvers bound
unlock( store ).registerPrivateSelectors( { foo } );

Here you supplied a resolver for foo, registered a private selector foo, but they won't be bound together. You'd have to move the register call after the private registration.

That's why the private selectors are bound lazily. I'll try to do a more perfect fix in a separate PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

I specifically moved the registration after to get this effect 🤷🏻

Copy link
Member

Choose a reason for hiding this comment

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

Yes, you had to move the registration, but you shouldn't have to, that's the bug 🙂

I think this PR is mergeable with the simple resolver fix. The correct solution turns out the be a rather big PR -- although it doesn't change a lot of code, it involves a lot of refactoring.

}

const boundPrivateSelectors = createBindingCache( bindSelector );
Expand Down