-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Block Bindings: Fix back-compat layer #71691
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
Conversation
e1ad423 to
ec404de
Compare
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Looks like this introduced a small regression. I'll look into it now. |
I didn't spot any regressions. I can confirm it fixes the issue that I reported with the Image block in Pattern Overrides. |
I should've added what it is: One of the unit tests is failing reproducibly (since the change now returns a capitalized |
|
It sounds like a test issue which needs to be fixed but it shouldn't require re-testing on my side 👍🏻 |
|
Flaky tests detected in 6a488fe. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/17775412209
|
|
@kmanijak kindly DM'ed me to tell me that the latest GB version seems to throw warnings related to the bug that this PR fixes:
I've added the "Backport to Gutenberg Minor Release" accordingly. |
|
Looks like this was never put in a minor? I'm removing the label since it's now in the major versions. |
What?
Fix a bug in the Block Bindings compat layer (which was introduced in #71389).
Why?
See #71483 (comment). In short, pattern overrides would break for the Image block if Block Bindings support was added for another Image block attribute (like
captionin case of #71483).How?
When updating
$instance->attributes, merge the computed attributes with$instance->attributesinstead of$instance->parsed_block['attrs']. (In hindsight, this one was a silly mistake.)Additionally, the logic in the code relies on the fact that
array_mergedoesn't do recursive merges (but replaces$instance->attributes['metadata']with$computed_attributes['metadata'], thus overriding block bindings that have already been processed). See the inline comments for more details.Testing Instructions
See #71483 (comment): Check out that PR's branch locally and try to reproduced the bug. Then, cherry-pick the commit from this branch on top. Verify that the bug is fixed.
Note that I've opted to file a separate PR to fix this bug instead of including the fix in #71483 for added visibility, and to avoid e.g. losing the bugfix in case #71483 gets reverted.