-
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
Shuffle: Don't call '__experimentalGetAllowedPatterns' for every block #64736
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
! patterns || | ||
patterns.length === 0 | ||
) { | ||
if ( categories.length === 0 || ! patterns || patterns.length === 0 ) { |
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.
The categories
will always be an array.
I considered adding a check in the |
Size Change: +6 B (0%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
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.
Neat optimization, thanks 👍 🚀
I'd only add a comment to explain why it's necessary.
const _patterns = __experimentalGetAllowedPatterns( rootBlock ); | ||
const _patterns = | ||
_categories.length > 0 | ||
? __experimentalGetAllowedPatterns( rootBlock ) |
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.
It's worth adding a comment why we're doing this, either here or wherever we consume the patterns
.
A comment can help in the future if someone is trying to use patterns
for something not related to the categories
, and they might be confused as to why patterns
is empty sometimes.
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.
Good idea. I added to the comment.
We should also try to improve the __experimentalGetAllowedPatterns
performance.
Flaky tests detected in d41cf87. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/10523227034
|
#64736) Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: wpsoul <wpsoul@git.wordpress.org>
* DataViews: Add missing styles and remove opinionated ones for generic usage (#64711) Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: ntsekouras <ntsekouras@git.wordpress.org> Co-authored-by: jameskoster <jameskoster@git.wordpress.org> * Shuffle: Don't call '__experimentalGetAllowedPatterns' for every block (#64736) Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: wpsoul <wpsoul@git.wordpress.org> * DataViews: hide sort direction control if there is no sortable fields (#64817) Co-authored-by: oandregal <oandregal@git.wordpress.org> Co-authored-by: ntsekouras <ntsekouras@git.wordpress.org> * Block Bindings: Refactor utils file. (#64740) * Refactor block binding utils * Add security checks Co-authored-by: cbravobernal <cbravobernal@git.wordpress.org> Co-authored-by: gziolo <gziolo@git.wordpress.org> Co-authored-by: SantosGuillamot <santosguillamot@git.wordpress.org> Co-authored-by: jsnajdr <jsnajdr@git.wordpress.org> * Interactivity API: Fix computeds without scope in Firefox (#64825) * Replace NO_SCOPE symbol with an object * Update changelog Co-authored-by: DAreRodz <darerodz@git.wordpress.org> Co-authored-by: luisherranz <luisherranz@git.wordpress.org> Co-authored-by: Marc-pi <mdxfr@git.wordpress.org> --------- Co-authored-by: Riad Benguella <benguella@gmail.com> Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: ntsekouras <ntsekouras@git.wordpress.org> Co-authored-by: jameskoster <jameskoster@git.wordpress.org> Co-authored-by: George Mamadashvili <georgemamadashvili@gmail.com> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: wpsoul <wpsoul@git.wordpress.org> Co-authored-by: André <583546+oandregal@users.noreply.github.com> Co-authored-by: oandregal <oandregal@git.wordpress.org> Co-authored-by: Carlos Bravo <37012961+cbravobernal@users.noreply.github.com> Co-authored-by: cbravobernal <cbravobernal@git.wordpress.org> Co-authored-by: gziolo <gziolo@git.wordpress.org> Co-authored-by: SantosGuillamot <santosguillamot@git.wordpress.org> Co-authored-by: jsnajdr <jsnajdr@git.wordpress.org> Co-authored-by: David Arenas <david.arenas@automattic.com> Co-authored-by: DAreRodz <darerodz@git.wordpress.org> Co-authored-by: luisherranz <luisherranz@git.wordpress.org> Co-authored-by: Marc-pi <mdxfr@git.wordpress.org>
I just cherry-picked this PR to the release/19.1 branch to get it included in the next release: #64876 |
WordPress#64736) Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: wpsoul <wpsoul@git.wordpress.org>
What?
Related #64732.
PR avoids calling the
__experimentalGetAllowedPatterns
selector when associated categories aren't available.Why?
When no categories exist, the memo callback returns early, and patterns are unnecessary. Additionally, calling
__experimentalGetAllowedPatterns
is expensive - #64219 (comment).Testing Instructions
Testing Instructions for Keyboard
Same.
Screenshots or screencast