Inject theme attribute during serialization #5192
Conversation
ockham
force-pushed
the
update/inject-theme-attr-during-serialization
branch
from
7c952fc
to
5a92288
Compare
There was a problem hiding this comment.
Everything works as expected. The refactoring looks like discussed in the linked WordPress trac ticket.
The only thing I’d like to rise is the code practices around unit tests. WordPress core has this guiding principle to put every individual test scenario in a seperate test function to ensure that failing assertion doesn't prevent further code from running. I currently see 3 different test case that even start with a code comment explaining what's being tested. The nice part is that all use cases I can think of are covered.
|
For visibility, this is the performance impact from this PR included on CI https://github.com/WordPress/wordpress-develop/actions/runs/6173429870/job/16778816255?pr=5192: 
|
|
Thank you very much! |
|
Committted to Core in https://core.trac.wordpress.org/changeset/56578. |
Tests splitted with https://core.trac.wordpress.org/changeset/56584. |
Rather than using
_inject_theme_attribute_in_block_template_contentto inject thethemeattribute into all Template Part blocks found in a given file-based Block Template, introduce a new function tentatively called_inject_theme_attribute_in_template_part_block, and use that as second argument toserialize_blocks()(introduced in #5187) in order to inject said attribute during tree traversal for serialization.This allows for a more modular approach that will eventually be extended to implement automatic insertion of hooked blocks (see).
Note that we're guarding
_build_block_template_result_from_file()(i.e. the callsite of_inject_theme_attribute_in_template_part_blockand previously of_inject_theme_attribute_in_block_template_content) against regressions through additional unit test coverage added in #5186.Trac ticket: https://core.trac.wordpress.org/ticket/59338
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.