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

__experimentalBlockPatterns: can we load them with REST API? #39055

Closed
jsnajdr opened this issue Feb 24, 2022 · 9 comments
Closed

__experimentalBlockPatterns: can we load them with REST API? #39055

jsnajdr opened this issue Feb 24, 2022 · 9 comments
Labels
[Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced

Comments

@jsnajdr
Copy link
Member

jsnajdr commented Feb 24, 2022

The HTML document generated for the /wp-admin/post.php page, i.e., the block editor, contains a very large (200-500kB depending on site setup and plugins) inline script with block pattern definitions:

<script id='wp-edit-post-js-after'>
wp.editPost.initializeEditor('editor', "post", 1, {
  ...
  __experimentalBlockPatterns: { ...really big json }
} );
</script>

On one my testing site (Core + Jetpack + Woo) the size of the block patterns JSON is 220kB, on another (WP.com Atomic) it's 510kB. Most of the size are .content attributes with the pattern's HTML content.

The question is whether this patterns list needs to be sent statically on every editor load and whether it needs to block the editor initialization. Could it be a REST endpoint instead, called asynchronously only when it's needed? If I understand things correctly, we don't need the block patterns list to parse and display a post, but only when showing the block inserter UI or when editing a specific pattern?

The __experimentalBlockPatternCategories has a similar issue and a REST endpoint would suit it much better, but from a performance standpoint, it's not a pressing issue.

@Mamaduka Mamaduka added the [Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced label Feb 25, 2022
@youknowriad
Copy link
Contributor

I just wanted to mention, that it's exactly why I kept it as "experimental" to begin with. REST API is a better path here 👍

@jsnajdr
Copy link
Member Author

jsnajdr commented Feb 25, 2022

@youknowriad The block-editor package, however, doesn't want to depend on core-data or on any WordPress REST endpoints. Would you have any advice on how to make the "await getBlockPatterns()" functionality generic enough and how to wire it up to actual WP REST code? Is there any prior art on that, where a similar problem was solved?

@youknowriad
Copy link
Contributor

@jsnajdr I think the block-editor should still have a setting, that setting could one of two things: a list of patterns like today, or a function to fetch patterns (I think we use something similar in reusable blocks).

@jsnajdr
Copy link
Member Author

jsnajdr commented Feb 28, 2022

Thanks, reusable blocks indeed seem to be a very good blueprint for this. I'm trying to draft a patch now.

@jsnajdr
Copy link
Member Author

jsnajdr commented Mar 16, 2022

There's one additional performance issue with block pattern registry: theme and feature patterns are downloaded with a w.org REST request and then registered, and this happens when the PHP server creates the editor's HTML page.

That adds 1-2 seconds to the response time, and slows down the editor initialization. The response is cached for one hour, so only some loads will be slowed down, but it's still an issue.

Downloading and registering current theme patterns is not even limited to the editor pages, but is hooked to the init hook and happens for every page, even site frontend. See #38323 where this was recently added (cc @ntsekouras).

The solution is to move all this downloading and registering to the wp/v2/block-patterns/patterns REST endpoint, and remove it from everywhere else.

@ntsekouras
Copy link
Contributor

ntsekouras commented Mar 16, 2022

Downloading and registering current theme patterns is not even limited to the editor pages, but is hooked to the init hook and happens for every page, even site frontend. See #38323 where this was recently added (cc @ntsekouras).

Oh... sorry about that.. Do you know what hook I should have used? Maybe a check with something like get_current_screen()->is_block_editor() would be okay?

@jsnajdr
Copy link
Member Author

jsnajdr commented Mar 16, 2022

The code that loads the featured patterns uses the current_screen action and has checks whether the screen is block or site editor:

https://github.com/WordPress/gutenberg/blob/trunk/lib/compat/wordpress-5.9/block-patterns.php#L47-L59

Doing the theme patterns load the same way would be a good fix until all the loads are moved to the REST API endpoint, where they really belong. I'll be working on that in #39185 which is currently in draft state.

@ntsekouras
Copy link
Contributor

Doing the theme patterns load the same way would be a good fix until all the loads are moved to the REST API endpoint, where they really belong. I'll be working on that in #39185 which is currently in draft state.

Thanks @jsnajdr . I'll open a quick PR about that now.

@jordesign
Copy link
Contributor

Looks like this can be closed now - @ntsekouras please feel free to reopen if that's not the case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced
Projects
None yet
Development

No branches or pull requests

5 participants