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

Template Part Block: Use get_block_template() for template part lookup #55956

Open
ockham opened this issue Nov 8, 2023 · 4 comments · May be fixed by #55961
Open

Template Part Block: Use get_block_template() for template part lookup #55956

ockham opened this issue Nov 8, 2023 · 4 comments · May be fixed by #55961
Assignees
Labels
[Block] Template Part Affects the Template Parts Block [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.

Comments

@ockham
Copy link
Contributor

ockham commented Nov 8, 2023

What problem does this address?

The Template Part block has been duplicating functionality (by a low-level implementation of template part lookup from the database with a fallback to block theme provided files) that’s also present in various functions in Core’s block-template-utils.php file.

This has become a problem at least twice, when we needed Template Part blocks to apply template related filters that are only present in those higher-level methods from block-template-utils.php, leading to #52892 and #55811, where we’ve replaced some low-level logic with slightly higher-level methods.

This solved the problems at hand. However, there's still a risk that the template part lookup code will continue to diverge — e.g. when an enhancement or bugfix is introduced into the methods in block-template-utils.php, the Template Part block might not receive them.

What is your proposed solution?

AFAICT, there’s no conceptual need for the implementation of the Template Part block to deviate from what could essentially be done by a single get_block_template call.

The major obstacle seems to be the presence of the three actions fired by the Template Part block during rendering, introduced by #36884. However, we might be able to accommodate for that by moving those actions over to the respective helper functions in block-template-utils.php.

@ockham ockham added [Type] Enhancement A suggestion for improvement. [Block] Template Part Affects the Template Parts Block labels Nov 8, 2023
@ockham
Copy link
Contributor Author

ockham commented Nov 8, 2023

cc/ @johnbillion @gziolo @Mamaduka who have worked on this block :)

@Mamaduka
Copy link
Member

Mamaduka commented Nov 8, 2023

Good idea and improvement makes sense to me.

The queries used by get_block_template and render_block_core_template_part have some differences. We need to ensure that we do not introduce any performance regressions.

Differences (I can spot):

The major obstacle seems to be the presence of the three actions fired by the Template Part block during rendering, introduced by #36884

I think these still can be applied in render functions, as they are block render specific.

@ockham
Copy link
Contributor Author

ockham commented Nov 8, 2023

Good idea and improvement makes sense to me.

The queries used by get_block_template and render_block_core_template_part have some differences. We need to ensure that we do not introduce any performance regressions.

👍

Differences (I can spot):

  • The render_block_core_template_part only queries for published template parts.

Are you sure about that? 🤔 In the absence of a published template part, it falls back to theme file template parts, no?

} else {
$template_part_file_path = '';
// Else, if the template part was provided by the active theme,
// render the corresponding file content.
if ( 0 === validate_file( $attributes['slug'] ) ) {
$block_template = get_block_file_template( $template_part_id, 'wp_template_part' );

Yeah, good point; @gziolo also brought this up to me. I have to look more into the history of this change; maybe it's something that should be applied to the methods in block-template-utils.php as well? 🤔

The major obstacle seems to be the presence of the three actions fired by the Template Part block during rendering, introduced by #36884

I think these still can be applied in render functions, as they are block render specific.

Yeah, maybe. They do need a bit of context that we're otherwise encapsulating in get_block_template, e.g. whether we're dealing with a template part from the DB or from a file. But that info can probably be inferred from the returned WP_Block_Template object's source field (e.g. 'custom' === $block_template->source).

@Mamaduka
Copy link
Member

Mamaduka commented Nov 8, 2023

Are you sure about that? 🤔 In the absence of a published template part, it falls back to theme file template parts, no?

This is what I meant by published status:

But that info can probably be inferred from the returned WP_Block_Template object

Exactly. The WP_Block_Template built from a post will have the wp_id property set.

@ockham ockham linked a pull request Nov 8, 2023 that will close this issue
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Template Part Affects the Template Parts Block [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants