[WIP] Patterns: Add a core selector to get user patterns

[glendaviesnz]

What?

Still experimental at this stage - exploring options for removing duplication of user patterns code between block editor and site editor by adding a new core selector

Why?

How?

Testing Instructions

Testing Instructions for Keyboard

Screenshots or screencast

[glendaviesnz]

@Mamaduka, @youknowriad, @talldan, @kevin940726, @aaronrobertshaw before I spend any time fixing up the typing issues on this PR, does anyone have any questions/concerns about pulling the parsing of the user patterns into a new selector like this.

Currently, in the site editor and block editor we are using calls to getEntityRecords( 'postType', 'wp_block' to get the user patterns and then duplicating the parsing required to make them usable in the inserter, etc. in both places. This PR abstracts that out into a new selector that handles the parsing in one place and can be used across both editors.

Memoizing with createSelector seems to work as expected and the selector only reruns if state.userPatternCategories or state.userPatterns change in my testing.

Let me know if you can see any problems with this approach.

[youknowriad]

Maybe I'm missing something but the selector was already in block-editor. Meaning it was already a "shared" selector somehow. So I'm wondering why we're adding this new config that probably duplicates some settings that are already there.

[glendaviesnz]

getUserPatterns in the block editor selectors is currently just a private method that is used by the getAllAllowedPatterns selector, so not shareable with site editor currently.

To minimise any backwards compat issues with changing existing selectors in block-editor, and moving away from the use of ReusableBlocks terminology it seemed like a good option was to create a new getUserPatterns selector in core-data and deprecate any selectors that are no longer used in block-editor. This also allows for an invalidateResolution( 'getUserPatterns' ) after adding/deleting user patterns, which is a bit more explicit than invalidateResolution( 'getEntityRecords', ['postType','wp_block] )

However, given that block-editor is already a dependency of edit-site another option is to just modify the selectors in the block editor store and use those in the site editor rather than adding completely new ones in core-data. It might be possible to make use of the getAllAllowedPatterns selector in the site editor instead to achieve something similar. I can take a look at doing that if you think that is a better approach.

[youknowriad]

what concerns me the most is that the block editor is already aware of these pattern in its settings and now we're adding another setting that basically is just another format of an existing one.

[glendaviesnz]

Makes sense, will reconsider in light of that.

[youknowriad]

I'm guessing this is the duplication that we're removing. Can you explain a bit where we're using this?

[glendaviesnz]

It essentially takes a wp_block entity record and maps it to an object that more closely resembles a theme/directory block pattern in order to allow both to be more easily merged into a single list of patterns under the categories in the site editor patterns page and post editor inserter.

[youknowriad]

Don't we have the patterns package now for this kind of things?

[glendaviesnz]

Yeh, maybe instead of pushing new settings into the block editor we can abstract the duplication out into a shared hook in the patterns package, or something. Will play around with some alternative approaches. Thanks for the feedback.

[jsnajdr]

Calling parse in a selector is potentially a problem because of block lazy loading (see #51778 and #53260). With block lazy loading, parsing content into blocks is an async function because the parser internally lazily loads the blocks it encounters in the content.

And selectors are synchronous, unless you want to also add a getUserPatterns resolver, so they can't really call async functions.

We'll either need to move content parsing out of the selector, or, at least, mark the getUserPatterns selector as experimental/private, so that we're not stuck with it forever.

You can also do a "thought experiment" where the parse function is already async (returns a promise). How does this PR need to change to accomodate that?

[glendaviesnz]

Ah, great to know this, thanks for this feedback, will have another think about the best approach.

[glendaviesnz]

I am going to explore some alternative approaches based on the feedback so will close this PR for now.