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

Plugin: Ensure that Block Hooks work correctly after landing in WP core #54651

Merged
merged 2 commits into from
Sep 25, 2023

Conversation

gziolo
Copy link
Member

@gziolo gziolo commented Sep 20, 2023

What?

Part of #53987.

Includes necessary changes to ensure that Block Hooks continues working correctly after Porting PHP code to Core.

Why?

At the moment, hooked blocks get inserted twice:

Screenshot 2023-09-25 at 11 29 49

How?

Everything should continue working as in WordPress core. Hooked blocks should be inserted only once:

Screenshot 2023-09-25 at 11 31 55

The challenge is that it has to be tested with WordPress 6.3 and WordPress 6.4 alpha.

Other than that all PHP unit tests should pass.

Testing Instructions

  1. Activate Twenty Twenty-Three theme.
  2. Download Like Button plugin that contains the hooked block with the Comment Template block.
  3. Install and activate the plugin.
  4. Open the site and navigate to the single posts page.
  5. Ensure that there are comments and the Like Button is rendered once for every comment.

@gziolo gziolo self-assigned this Sep 20, 2023
@gziolo gziolo requested a review from ockham September 20, 2023 07:35
@gziolo gziolo added the Gutenberg Plugin Issues or PRs related to Gutenberg Plugin management related efforts label Sep 20, 2023
@github-actions
Copy link

github-actions bot commented Sep 20, 2023

This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress.

If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged.

If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack.

Thank you! ❤️

View changed files
❔ lib/compat/wordpress-6.4/block-hooks.php
❔ lib/compat/wordpress-6.4/class-gutenberg-rest-block-patterns-controller.php
❔ phpunit/tests/blocks/renderHookedBlocks.php

@gziolo gziolo added the [Type] Task Issues or PRs that have been broken down into an individual action to take label Sep 20, 2023
@gziolo gziolo mentioned this pull request Sep 20, 2023
19 tasks
@gziolo gziolo force-pushed the update/block-hooks-core-detection branch from cd80553 to b90afcb Compare September 21, 2023 08:43
@gziolo
Copy link
Member Author

gziolo commented Sep 21, 2023

I don't think we need to cover with unit tests the polyfill for Block Hooks anymore so I removed them with b90afcb.

I don't think we run E2E tests with the older major versions of WordPress, so we should just test it manually with WordPress 6.3 before landing the PR.

@github-actions
Copy link

Flaky tests detected in b90afcb.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6259312996
📝 Reported issues:

@gziolo
Copy link
Member Author

gziolo commented Sep 22, 2023

I opened a PR against core that documents the latest behavior for edge cases when dealing with Block Hooks at positions firstChild and lastChild: WordPress/wordpress-develop#5279. The behavior differs from what we have in the polyfill developed for the Gutenberg plugin, so we will still have to patch it. It also means that the tests removed with b90afcb could be useful. I will see if we can keep them by moving gutenberg_serialize_blocks outside of the check against WP 6.4 version.

@gziolo gziolo force-pushed the update/block-hooks-core-detection branch from b90afcb to 3f56b22 Compare September 25, 2023 08:39
@gziolo
Copy link
Member Author

gziolo commented Sep 25, 2023

I brought back unit tests in 7a04903 and updated them to work with the polyfill developed for older versions of WordPress. I applied all necessary updates to handling fistChild and lastChild with 3f56b22 to cover the case when there are no blocks.

@gziolo gziolo marked this pull request as ready for review September 25, 2023 08:52
Copy link
Contributor

@ockham ockham left a comment

Choose a reason for hiding this comment

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

  • Code looks good ✅
  • Tested the plugin zip produced by this PR in a local dev environment running Core trunk. Verified that hooked blocks are correctly inserted both on the frontend and in the editor ✅
  • Tested the plugin with a throwaway test site with WP 6.3. Verified that hooked blocks are correctly inserted both on the frontend and in the editor ✅

Thank you! :shipit:

@gziolo gziolo merged commit 1010ca2 into trunk Sep 25, 2023
52 checks passed
@gziolo gziolo deleted the update/block-hooks-core-detection branch September 25, 2023 09:37
@gziolo gziolo added the Backport to Gutenberg RC Pull request that needs to be backported to a Gutenberg release candidate (RC) label Sep 25, 2023
@github-actions github-actions bot added this to the Gutenberg 16.8 milestone Sep 25, 2023
@gziolo gziolo modified the milestones: Gutenberg 16.8, Gutenberg 16.7 Sep 25, 2023
@gziolo
Copy link
Member Author

gziolo commented Sep 25, 2023

@mikachan, it would be nice to backport this fix to the Gutenberg plugin 16.7 release (#54621). We hope to run a call for testing for the Block Hooks feature, and this could interfere in case someone tests Gutenberg 16.7 and WordPress 6.4 Beta 1 together.

@mikachan
Copy link
Member

I just cherry-picked this PR to the release/16.7 branch to get it included in the next release: 0790b09

mikachan pushed a commit that referenced this pull request Sep 25, 2023
…re (#54651)

* Plugin: Ensure that Block Hooks work correctly after landing in WP core

* Ensure that unit tests cover gutenberg_serialize_blocks polyfill
@mikachan mikachan removed the Backport to Gutenberg RC Pull request that needs to be backported to a Gutenberg release candidate (RC) label Sep 25, 2023
@ockham
Copy link
Contributor

ockham commented Sep 26, 2023

@mikachan Any chance we can also include this in 6.4? Not super urgent, so it's okay if we miss Beta 1, but would be handy to get into e.g. Beta 2. (See WordPress/wordpress-develop#5307)

@ockham ockham added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Sep 26, 2023
@mikachan mikachan removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Sep 26, 2023
@mikachan
Copy link
Member

Yes! This PR should be included in 6.4 Beta 1, I'll confirm if not and re-add the "Backport to WP Beta/RC" label.

@scruffian
Copy link
Contributor

This PR seems to break template parts: #55202

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Gutenberg Plugin Issues or PRs related to Gutenberg Plugin management related efforts [Type] Task Issues or PRs that have been broken down into an individual action to take
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants