Block Editor: Avoid unbound requests in inserter selectors #66817
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
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. |
|
Size Change: -9 B (0%) Total Size: 1.84 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in a2a817b. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/13071176532
|
|
Initially, this test existed to be able to hide the "reusable blocks" tab in the inserter entirely. Some folks didn't want reusable blocks at all. Now, I'm not sure if there's a way for the "patterns" tab to be removed entirely from the inserter and whether the change here impacts it. |
|
Regarding Riad's comment above, if the check is needed somewhere an alternative could be to have a new light request/selector to query just for one item. We do something similar for media categories |
|
Where is this reusable blocks tab? If you don't want reusable blocks, remove
This would mean another selector-setting that we need to pass through block-editor settings. It would be great if we can avoid that. |
|
I'm glad I found this PR. I had a similar idea when we worked on fixes for recent performance regressions related to patterns. |
Mamaduka
force-pushed
the
fix/get-allowed-blocks
branch
from
65053ae to
3e758b0
Compare
Mamaduka
changed the title
|
Sorry for the forced push, but it was the easiest way to resolve conflicts and introduce additional changes. I've also removed My only worry here was direct insert when |
Mamaduka
reviewed
Jan 30, 2025
| @@ -2374,7 +2367,6 @@ export const getAllowedBlocks = createRegistrySelector( ( select ) => | |||
| }, | |||
| ( state, rootClientId ) => [ | |||
| getBlockTypes(), | |||
| unlock( select( STORE_NAME ) ).getReusableBlocks(), | |||
| ...getInsertBlockTypeDependants( select )( state, rootClientId ), | |||
There was a problem hiding this comment.
Because of this dependent method, we'll have to keep using createRegistrySelector. See #66833.
Mamaduka
force-pushed
the
fix/get-allowed-blocks
branch
from
3e758b0 to
a2a817b
Compare
t-hamano
reviewed
Feb 1, 2025
|
The only case I can think of is when the site disables all the blocks except I noticed that this change made the following test flaky - "Synced pattern › can be inserted after refresh". I need to investigate that before this is ready to land. |
|
Thanks for taking it over |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Easier to review without whitespace.
This is part of an effort to refactor
getReusableBlocksand make reusable blocks paginated.Why?
Currently,
getReusableBlocksmakes an unbound request, triggering the fetching of all reusable user blocks, page by page, until the last page.getAllowedBlocksis used for example in the block inserter button, which means that simply selecting an empty paragraph, which shows the inserter button, triggers a cascade of requests. This is problematic for sites that have thousands of reusable user blocks.How?
The check is currently only done to conditionally include
core/blockin the list of allowed blocks. Why not assume that all sites have at least one reusable block and eliminate this call entirely?Testing Instructions
Open the network tab. Select an empty paragraph, or simply create a new post (which also shows the inserter button). Notice the cascade of requests. With this PR the requests should be gone. If you want to try adding many reusable blocks, use this command:
Testing Instructions for Keyboard
Screenshots or screencast