Skip to content
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

Issue-91 [FEATURE] Removes preloading of blocks on edit post screen #92

Merged
merged 10 commits into from
May 14, 2024

Conversation

mslinnea
Copy link
Member

@mslinnea mslinnea commented May 8, 2024

Summary

WordPress pre-loads all Reusable Blocks (Synced Patterns) on the edit post screen, site editor, and widget screen. For sites with hundreds of blocks, this is a lot of processing overhead, as the_content and all other filters run on these blocks. Disabling this behavior make the editing experience faster and less fragile for sites with a large number of blocks.

Notes for reviewers

None.

Other Information

  • I updated the README.md file for any new/updated features.
  • I updated the CHANGELOG.md file for any new/updated features.

Changelog entries

Added

Changed

Deprecated

Removed

Fixed

Security

@mslinnea mslinnea requested a review from a team as a code owner May 8, 2024 00:49
$paths,
function ( $v ) {
// Remove the blocks preload path for performance reasons.
return ! is_string( $v ) || ! str_starts_with( $v, '/wp/v2/blocks?context=edit' );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it best to wipe out preloading synced patterns on all sites? Is it ever beneficial for sites with fewer synced patterns?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish I could find a discussion about the possible benefit as I certainly don't want to cause any issues with this change. I'll ask around.

I do know that if only some blocks are pre-loaded (like only 10) that there are issues with searching for reusable blocks. So it seems that it has to be either an all or nothing approach.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the all-or-nothing part doesn't seem like a big deal to me, rather, should this logic kick in only after a site gets more than 20 (or whatever) synced patterns? And that number could be filterable, etc.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like how you're thinking @dlh01, but how about for MVP we launch it as is and put in an Enhancement issue to add a filterable threshold?
Sites can always turn off this feature with a filter.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo it's a sensible default to turn it off completely as long as it doesn't cause any issues with synced patterns/reusable blocks, and if individual sites want to preload, they can turn off the feature with the filter as @mogmarsh said.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to testing this on a very large traditional site, I also tested it on a block theme and had no issue using synced patterns.

Copy link
Member

@dlh01 dlh01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🌳 Some thoughts about how this feature could be tightened up, but no blocking feedback.

@mslinnea mslinnea requested a review from dlh01 May 14, 2024 16:02
Copy link
Member

@dlh01 dlh01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🌳

@mslinnea mslinnea merged commit 716f571 into main May 14, 2024
7 checks passed
@mslinnea mslinnea deleted the feature/disable-preloading branch May 14, 2024 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants