-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[WIP] Patterns: Add a core selector to get user patterns #55985
Changes from all commits
c25a906
048ec3a
b25ff52
3d318fe
5957a0c
b9e6622
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -184,58 +184,19 @@ const selectPatterns = createSelector( | |
] | ||
); | ||
|
||
const patternBlockToPattern = ( patternBlock, categories ) => ( { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm guessing this is the duplication that we're removing. Can you explain a bit where we're using this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It essentially takes a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't we have the patterns package now for this kind of things? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
blocks: parse( patternBlock.content.raw, { | ||
__unstableSkipMigrationLogs: true, | ||
} ), | ||
...( patternBlock.wp_pattern_category.length > 0 && { | ||
categories: patternBlock.wp_pattern_category.map( | ||
( patternCategoryId ) => | ||
categories && categories.get( patternCategoryId ) | ||
? categories.get( patternCategoryId ).slug | ||
: patternCategoryId | ||
), | ||
} ), | ||
termLabels: patternBlock.wp_pattern_category.map( ( patternCategoryId ) => | ||
categories?.get( patternCategoryId ) | ||
? categories.get( patternCategoryId ).label | ||
: patternCategoryId | ||
), | ||
id: patternBlock.id, | ||
name: patternBlock.slug, | ||
syncStatus: patternBlock.wp_pattern_sync_status || PATTERN_SYNC_TYPES.full, | ||
title: patternBlock.title.raw, | ||
type: PATTERN_TYPES.user, | ||
patternBlock, | ||
} ); | ||
|
||
const selectUserPatterns = createSelector( | ||
( select, syncStatus, search = '' ) => { | ||
const { getEntityRecords, getIsResolving, getUserPatternCategories } = | ||
const { getIsResolving, getUserPatternCategories, getUserPatterns } = | ||
select( coreStore ); | ||
|
||
const query = { per_page: -1 }; | ||
const records = getEntityRecords( | ||
'postType', | ||
PATTERN_TYPES.user, | ||
query | ||
); | ||
let patterns = getUserPatterns(); | ||
const userPatternCategories = getUserPatternCategories(); | ||
const categories = new Map(); | ||
userPatternCategories.forEach( ( userCategory ) => | ||
categories.set( userCategory.id, userCategory ) | ||
); | ||
let patterns = records | ||
? records.map( ( record ) => | ||
patternBlockToPattern( record, categories ) | ||
) | ||
: EMPTY_PATTERN_LIST; | ||
|
||
const isResolving = getIsResolving( 'getEntityRecords', [ | ||
'postType', | ||
PATTERN_TYPES.user, | ||
query, | ||
] ); | ||
const isResolving = getIsResolving( 'getUserPatterns' ); | ||
|
||
if ( syncStatus ) { | ||
patterns = patterns.filter( | ||
|
@@ -257,14 +218,8 @@ const selectUserPatterns = createSelector( | |
}; | ||
}, | ||
( select ) => [ | ||
select( coreStore ).getEntityRecords( 'postType', PATTERN_TYPES.user, { | ||
per_page: -1, | ||
} ), | ||
select( coreStore ).getIsResolving( 'getEntityRecords', [ | ||
'postType', | ||
PATTERN_TYPES.user, | ||
{ per_page: -1 }, | ||
] ), | ||
select( coreStore ).getUserPatterns(), | ||
select( coreStore ).getIsResolving( 'getUserPatterns' ), | ||
select( coreStore ).getUserPatternCategories(), | ||
] | ||
); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 anasync
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?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, great to know this, thanks for this feedback, will have another think about the best approach.