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

Block: Give traverse_and_serialize_block(s)' post-callback access to block instance #5525

Conversation

ockham
Copy link
Contributor

@ockham ockham commented Oct 18, 2023

Both the $pre_ and $post_callback functions that are given as arguments to traverse_and_serialize_block(s) receive a reference to the current block as its first argument. However, while any changes that the "pre" callback makes to the block are actually respected during serialization, the same isn't true for the "post" callback: Any changes that it makes are only applied after the block has already been serialized. For applications like #59412, we probably want to change that.

Testing instructions

The behavior is covered by the newly added unit test. You can verify that it fails on trunk by cherry-picking a8c6633 there, and running npm run test:php -- --group=blocks.

Trac ticket: https://core.trac.wordpress.org/ticket/59669


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.

@ockham ockham self-assigned this Oct 18, 2023
@ockham ockham requested a review from gziolo October 18, 2023 13:08
@ockham ockham marked this pull request as ready for review October 18, 2023 13:08
@ockham ockham changed the title Update/traverse and serialize blocks give post callback access to block Block: Give traverse_and_serialize_block(s)' post-callback access to block instance Oct 18, 2023
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

That makes a lot of sense, and improves the implementation in a very elegant way. It’s probably also an edge case but definitely important for the planned follow-up work with Block Hooks.

@ockham
Copy link
Contributor Author

ockham commented Oct 18, 2023

Committed to Core trunk in https://core.trac.wordpress.org/changeset/56970.

@ockham ockham closed this Oct 18, 2023
@ockham ockham deleted the update/traverse-and-serialize-blocks-give-post-callback-access-to-block branch October 18, 2023 19:31
@ockham
Copy link
Contributor Author

ockham commented Nov 1, 2023

Backported to the 6.4 branch in https://core.trac.wordpress.org/changeset/57043.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants