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.

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

Try migrate link suggestions to use Core Data #57582

Draft
wants to merge 2 commits into
base: trunk
Choose a base branch
from
Draft
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: 15 additions & 0 deletions docs/reference-guides/data/data-core.md
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,21 @@ _Returns_

- `any`: The entity record's save error.

### getLinkSuggestions

Fetch link suggestions for a given search term.

_Parameters_

- _state_ Data state.
- _search_ Search term.
- _searchOptions_
- _settings_

_Returns_

- Link suggestions.

Comment on lines +377 to +391
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should make this private. But I wasn't sure how to unlock a resolveSelect.

### getRawEntityRecord

Returns the entity's record object by key, with its attributes mapped to their raw values.
Expand Down
15 changes: 15 additions & 0 deletions packages/core-data/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -695,6 +695,21 @@ _Returns_

- `any`: The entity record's save error.

### getLinkSuggestions

Fetch link suggestions for a given search term.

_Parameters_

- _state_ Data state.
- _search_ Search term.
- _searchOptions_
- _settings_

_Returns_

- Link suggestions.

### getRawEntityRecord

Returns the entity's record object by key, with its attributes mapped to their raw values.
Expand Down
13 changes: 13 additions & 0 deletions packages/core-data/src/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -623,6 +623,18 @@ export function defaultTemplates( state = {}, action ) {
return state;
}

export function linkSuggestions( state = {}, action ) {
switch ( action.type ) {
case 'RECEIVE_LINK_SUGGESTIONS':
return {
...state,
[ action.search ]: action.suggestions,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think this indexing strategy is good enough. A search is a simple string but the API allows for searching by different entities such as post, page...etc. So if we just index by the search you could accidentally receives results for the wrong content type.

};
}

return state;
}

export default combineReducers( {
terms,
users,
Expand All @@ -644,4 +656,5 @@ export default combineReducers( {
userPatternCategories,
navigationFallbackId,
defaultTemplates,
linkSuggestions,
} );
12 changes: 12 additions & 0 deletions packages/core-data/src/resolvers.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { STORE_NAME } from './name';
import { getOrLoadEntitiesConfig, DEFAULT_ENTITY_KEY } from './entities';
import { forwardResolver, getNormalizedCommaSeparable } from './utils';
import { getSyncProvider } from './sync';
import { __experimentalFetchLinkSuggestions as fetchLinkSuggestions } from './fetch';

/**
* Requests authors from the REST API.
Expand Down Expand Up @@ -668,6 +669,17 @@ export const getUserPatternCategories =
} );
};

export const getLinkSuggestions =
( search, searchOptions, settings ) =>
async ( { dispatch } ) => {
const suggestions = await fetchLinkSuggestions(
search,
searchOptions,
settings
);
dispatch( { type: 'RECEIVE_LINK_SUGGESTIONS', search, suggestions } );
};

export const getNavigationFallbackId =
() =>
async ( { dispatch, select } ) => {
Expand Down
18 changes: 18 additions & 0 deletions packages/core-data/src/selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1491,3 +1491,21 @@ export const getRevision = createSelector(
];
}
);

/**
* Fetch link suggestions for a given search term.
*
* @param state Data state.
* @param search Search term.
* @param searchOptions
* @param settings
* @return Link suggestions.
*/
export function getLinkSuggestions(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be a private API.

state,
search = '',
searchOptions,
settings
) {
return state.linkSuggestions[ search ];
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@
* WordPress dependencies
*/
import { Platform, useMemo, useCallback } from '@wordpress/element';
import { useDispatch, useSelect } from '@wordpress/data';
import { useDispatch, useSelect, resolveSelect } from '@wordpress/data';
import {
store as coreStore,
__experimentalFetchLinkSuggestions as fetchLinkSuggestions,
__experimentalFetchUrlData as fetchUrlData,
} from '@wordpress/core-data';
import { __ } from '@wordpress/i18n';
Expand Down Expand Up @@ -215,6 +214,10 @@ function useBlockEditorSettings( settings, postType, postId ) {
[ saveEntityRecord, userCanCreatePages ]
);

const syncGetLinkSuggestions = useCallback( async ( ...args ) => {
return await resolveSelect( coreStore ).getLinkSuggestions( ...args );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we make this private how do we unlock a resolveSelect?

Copy link
Member

Choose a reason for hiding this comment

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

Why not use a normal select call in a useSelect?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we did this then we'd effectively pass something like getEntityRecords as a setting. We can't pass the result of a selector call because we need different args per invocation.

I think I tried doing this and found it didn't work. I could be wrong - would you expect it to?

}, [] );

const forceDisableFocusMode = settings.focusMode === false;

return useMemo(
Expand All @@ -232,8 +235,7 @@ function useBlockEditorSettings( settings, postType, postId ) {
__experimentalBlockPatterns: blockPatterns,
__experimentalBlockPatternCategories: blockPatternCategories,
__experimentalUserPatternCategories: userPatternCategories,
__experimentalFetchLinkSuggestions: ( search, searchOptions ) =>
fetchLinkSuggestions( search, searchOptions, settings ),
__experimentalFetchLinkSuggestions: syncGetLinkSuggestions,
inserterMediaCategories,
__experimentalFetchRichUrlData: fetchUrlData,
// Todo: This only checks the top level post, not the post within a template or any other entity that can be edited.
Expand Down Expand Up @@ -279,6 +281,7 @@ function useBlockEditorSettings( settings, postType, postId ) {
postType,
setIsInserterOpened,
getPostLinkProps,
syncGetLinkSuggestions,
]
);
}
Expand Down
Loading