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

Lazy initialization of block variants. #45601

Closed
spacedmonkey opened this issue Nov 8, 2022 · 14 comments
Closed

Lazy initialization of block variants. #45601

spacedmonkey opened this issue Nov 8, 2022 · 14 comments
Labels
[Feature] Block Variations Block variations, including introducing new variations and variations as a feature [Type] Performance Related to performance efforts

Comments

@spacedmonkey
Copy link
Member

spacedmonkey commented Nov 8, 2022

What problem does this address?

Follow issue related to #45362 (review)

The template part block, loads block variants what load WP_Query and result in database queries. This means that when a block is registered ( on every page request ), that an mostly unnecessary database query is run. There needs to be a way to load lazily the block variants, only when needed.

Props @talldan @Mamaduka

What is your proposed solution?

@spacedmonkey spacedmonkey added [Type] Performance Related to performance efforts [Feature] Block Variations Block variations, including introducing new variations and variations as a feature labels Nov 8, 2022
@sybrew
Copy link

sybrew commented Jan 3, 2023

Blocks, or any derivative thereof (such as templates), shouldn't be part of the Post Type API, for they aren't Post Types.

In that, I think wp_template's foundation should be rewritten without considering Post Types but instead something bespoke --- perhaps having a new database table if deemed necessary. This is how you can get rid of the WP_Query request permanently.

I'm confident the Post Type API eased the integration for storing and filtering, but it should be a short-term strategy. Keeping the system as-is will cause it to be maintained into an indomitable blob of code debt.

@gziolo gziolo added [Block] Template Part Affects the Template Parts Block and removed [Block] Template Part Affects the Template Parts Block labels Feb 10, 2023
@spacedmonkey spacedmonkey changed the title Lazy initialization of block variables. Lazy initialization of block variants. Mar 20, 2023
@spacedmonkey
Copy link
Member Author

@talldan @Mamaduka @gziolo @sybrew Can you provide some context of when varients are needed for the template part block. At the moment, variants are loaded on EVERY page request. Is there anyway to only load them in the site editor? Is there any other places where these variants are needed.

@Mamaduka
Copy link
Member

@spacedmonkey, what's your plan for making the block variations lazily loadable? Changing type for $WP_Block_Type::$variations from array[] to array[]|callable and lazily initializing callables when needed?

A good start would be get_block_editor_server_block_settings and WP_REST_Block_Types_Controller. We could also modify WP_Block_Type::__get and handle lazy loading there; this probably would be the most backward-compatible way.

@talldan
Copy link
Contributor

talldan commented Apr 12, 2023

Can you provide some context of when varients are needed for the template part block.

Sorry for the slow reply. They're only needed where template parts can be edited, currently I believe that's only in the site editor.

@spacedmonkey
Copy link
Member Author

If you are right @talldan could we just register the variants when a user is logged in.

/**
* Registers the `core/template-part` block on the server.
*/
function register_block_core_template_part() {
register_block_type_from_metadata(
__DIR__ . '/template-part',
array(
'render_callback' => 'render_block_core_template_part',
'variations' => build_template_part_block_variations(),
)
);
}
add_action( 'init', 'register_block_core_template_part' );

function register_block_core_template_part() {
        $args = array(
            'render_callback' => 'render_block_core_template_part',
        );
        if ( current_user_can( 'edit_theme_options' ) ) {
               $args['variations'] = build_template_part_block_variations(),
        }
	register_block_type_from_metadata( __DIR__ . '/template-part', $args );
}

This seems like a simple change.

@Mamaduka
Copy link
Member

My preference would be the general solution via the Blocks API, instead of handling this per-block basis.

It would be easier to document, and we can always recommend lazy initialization for variables that are generated from DB.

cc @gziolo

@spacedmonkey
Copy link
Member Author

Using the blocks api sounds related to #22812

@Mamaduka
Copy link
Member

While it's definitely a related issue, I don't think it should be a requirement for the enhancement.

@spacedmonkey
Copy link
Member Author

I don't know how site editor ues these variants, so it is hard to make a recommendation. If you need to load this data via the REST API, I can help with that, but someone with javascript knowledge will need to help with making this change.

The long this is left, the more of a B/C break this will be, so I want to get this fixed ASAP.

@Mamaduka
Copy link
Member

There shouldn't be any JS involved in this enhancement.

The blocks are bootstrapped via get_block_editor_server_block_settings() on editor pages or loaded via API (WP_REST_Block_Types_Controller). Both places need to handle lazy block variation init. See my previous comment #45601 (comment).

The main problem we might face here is that we'll need to land changes in the core before the Gutenberg plugin can use it.

@spacedmonkey
Copy link
Member Author

@Mamaduka This change is going to be hard to do in gutenberg. Should I work on it in core then?

@Mamaduka
Copy link
Member

Correct, we can't change those files in the plugin. The core PR sounds good to me. We can conditionally provide variations values for blocks and use lazy loading.

@spacedmonkey
Copy link
Member Author

I put together a little POC PR - WordPress/wordpress-develop#4857.

It is a same breaking change. We need to investigation more.

@Mamaduka
Copy link
Member

This was resolved via WordPress/wordpress-develop#5718.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block Variations Block variations, including introducing new variations and variations as a feature [Type] Performance Related to performance efforts
Projects
None yet
Development

No branches or pull requests

5 participants