-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Parsing patterns when idling (performance follow-up for inserting patterns into containers) #29444
Changes from 15 commits
c0edc39
4298360
c1d85b4
b0b6b43
39bc7eb
34e4aad
ac25ca5
0e0e2b5
4df0a4e
8f07730
9a200cd
a8b3d46
b12d4d6
2945982
eb4e1e0
f402bd5
fb07d35
1a176d2
cac8f34
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 |
---|---|---|
|
@@ -15,7 +15,6 @@ import { | |
filter, | ||
mapKeys, | ||
orderBy, | ||
every, | ||
} from 'lodash'; | ||
import createSelector from 'rememo'; | ||
|
||
|
@@ -1757,13 +1756,18 @@ export const __experimentalGetAllowedBlocks = createSelector( | |
] | ||
); | ||
|
||
const __experimentalGetParsedPatterns = createSelector( | ||
( state ) => { | ||
export const __experimentalGetParsedPattern = createSelector( | ||
( state, patternName ) => { | ||
const patterns = state.settings.__experimentalBlockPatterns; | ||
return map( patterns, ( pattern ) => ( { | ||
const pattern = patterns.find( ( { name } ) => name === patternName ); | ||
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. You already have the pattern from 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. The goal is to have a selector function that can be memoized easily. The pattern name as parameter is immutable. If we used the pattern object then it might happen that we provide a modified/reconstructed pattern object that has a different reference. |
||
if ( ! pattern ) { | ||
return null; | ||
} | ||
|
||
return { | ||
...pattern, | ||
contentBlocks: parse( pattern.content ), | ||
} ) ); | ||
blocks: parse( pattern.content ), | ||
}; | ||
}, | ||
( state ) => [ state.settings.__experimentalBlockPatterns ] | ||
); | ||
|
@@ -1778,17 +1782,19 @@ const __experimentalGetParsedPatterns = createSelector( | |
*/ | ||
export const __experimentalGetAllowedPatterns = createSelector( | ||
( state, rootClientId = null ) => { | ||
const patterns = __experimentalGetParsedPatterns( state ); | ||
|
||
const patterns = state.settings.__experimentalBlockPatterns; | ||
if ( ! rootClientId ) { | ||
return patterns; | ||
} | ||
|
||
const patternsAllowed = filter( patterns, ( { contentBlocks } ) => { | ||
return every( contentBlocks, ( { name } ) => | ||
const parsedPatterns = patterns.map( ( { name } ) => | ||
__experimentalGetParsedPattern( state, name ) | ||
); | ||
const patternsAllowed = filter( parsedPatterns, ( { blocks } ) => | ||
blocks.every( ( { name } ) => | ||
canInsertBlockType( state, name, rootClientId ) | ||
); | ||
} ); | ||
) | ||
); | ||
|
||
return patternsAllowed; | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { useSelect, select } from '@wordpress/data'; | ||
import { useEffect } from '@wordpress/element'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import { store as blockEditorStore } from '../store'; | ||
|
||
const requestIdleCallback = ( () => { | ||
if ( typeof window === 'undefined' ) { | ||
return ( callback ) => { | ||
setTimeout( () => callback( Date.now() ), 0 ); | ||
}; | ||
} | ||
|
||
return window.requestIdleCallback || window.requestAnimationFrame; | ||
} )(); | ||
|
||
const cancelIdleCallback = ( () => { | ||
if ( typeof window === 'undefined' ) { | ||
return clearTimeout; | ||
} | ||
|
||
return window.cancelIdleCallback || window.cancelAnimationFrame; | ||
} )(); | ||
|
||
export function usePreParsePatterns() { | ||
const patterns = useSelect( | ||
( _select ) => | ||
_select( blockEditorStore ).getSettings() | ||
.__experimentalBlockPatterns, | ||
[] | ||
); | ||
|
||
useEffect( () => { | ||
if ( ! patterns?.length ) { | ||
return; | ||
} | ||
|
||
let handle; | ||
let index = -1; | ||
|
||
const callback = () => { | ||
index++; | ||
if ( index >= patterns.length ) { | ||
return; | ||
} | ||
|
||
const marker = `[usePreParsePatterns] process ${ index } ${ patterns[ index ]?.name }`; | ||
window.console.time( marker ); | ||
select( blockEditorStore ).__experimentalGetParsedPattern( | ||
patterns[ index ].name, | ||
index | ||
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. Is this second argument correct? 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. No, that's not needed anymore. Thanks! |
||
); | ||
window.console.timeEnd( marker ); | ||
|
||
handle = requestIdleCallback( callback ); | ||
}; | ||
|
||
window.console.time( '[usePreParsePatterns] initiate' ); | ||
window.console.timeEnd( '[usePreParsePatterns] initiate' ); | ||
|
||
handle = requestIdleCallback( callback ); | ||
return () => cancelIdleCallback( handle ); | ||
}, [ patterns ] ); | ||
|
||
return null; | ||
} |
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.
For demonstration purposes only. I'll remove this once the PR is approved.
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.
Can we remove that code, in order to get accurate numbers in the job and judge whether the PR allows us to get the previous loading numbers?