Only register anchor attribute for dynamic blocks with render callback.

[Soean]

What?

The anchor attribute should only be registered for the dynamic blocks. Otherwise static blocks will get this attribute which will break this blocks.

Why?

Fixes #48232

[t-hamano]

The way to check render_callback would be to add an extra comment to some static blocks.
Please see: #48232 (comment)

[github-actions]

Flaky tests detected in 0ac424a.
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/4259941579
📝 Reported issues:

• [Flaky Test] results in two remaining blocks and positions the caret at the end of the second block #39805 in specs/editor/various/block-deletion.test.js
• [Flaky Test] should make bold after split and merge #40308 in specs/editor/various/rich-text.test.js
• [Flaky Test] should move the blocks down #39691 in specs/editor/various/block-editor-keyboard-shortcuts.test.js

[Mamaduka]

I have rebased the branch to fix failing performance tests.

@t-hamano, did you get a chance to test this solution?

[t-hamano]

@Mamaduka

I have not tested this PR yet, but $block_type->is_dynamic() only determines if the render_callback property is callable. However, there are some static blocks that have the render_callback property but don't require the anchor attribute (Heading, Image, Cover, etc.).

Therefore, my approach is as described in this comment:

• Explicitly define the anchor attribute in the dynamic block's block.json
• Add the following description to the block.json schema and documentation

  For the dynamic block that doesn't save any markup with the save function, it is necessary to define the anchor property with type string as attributes.

@aaronrobertshaw

I'm sure you are familiar with block support and backward compatibility, so if you have any ideas on what approach would work best, please let us know 🙏

[aaronrobertshaw]

I'm sure you are familiar with block support and backward compatibility, so if you have any ideas on what approach would work best, please let us know

Thanks for the ping @t-hamano 👍

Unfortunately, I'm not familiar at all with the anchor support. In general though, because we can't forcibly update saved content for blocks, each block support itself needs to ensure it maintains backward compatibility.

You make a great point regarding the blocks that do save content but still have render callbacks. As such, the straightforward check of is_dynamic() doesn't appear to satisfy the BC requirement fully.

The blocks with both a callback and saved content are an issue, however, I still consider these to be dynamic given they get altered at runtime. I know its semantics but this lack of a clear delineation might lead to additional confusion. Particularly, if we are using the term "dynamic" in the proposed documentation and block.json changes.

All that said, I can't think of any elegant solutions, this late on a Friday evening 🙂

One alternative to requiring block authors to manually define and maintain an anchor attribute might be a tweak to the block.json supports config. Maybe something could be added or supported there that flags the anchor attribute needs registering. This would make the anchor support behave a little differently to others, but it already does, so maybe its not a blocker?

[t-hamano]

@aaronrobertshaw

Thank you for the advice!

The blocks with both a callback and saved content are an issue, however, I still consider these to be dynamic given they get altered at runtime. I know its semantics but this lack of a clear delineation might lead to additional confusion. Particularly, if we are using the term "dynamic" in the proposed documentation and block.json changes.

Yes, agree. I think that the word "dynamic" in the block can mean either of the following, depending on the context:

• The save property of registerBlockType is null and is rendered server-side
• Blocks with render_callback function

I believe the former is what is meant by the term "dynamic block" in this case. I think it would be better to update the anchor property of block.json to the following description instead of using the word "dynamic":

From:

Anchors let you link directly to a specific block on a page. This property adds a field to define an id for the block and a button to copy the direct link.

To:

Anchors let you link directly to a specific block on a page. This property adds a field to define an id for the block and a button to copy the direct link. Blocks for which the save callback function returns NULL must explicitly define the anchor attribute to store the anchor as an attribute in the database.

@Soean
I would like to submit another PR based on this description.

[t-hamano]

@Soean
I would like to close this PR as I will attempt to resolve this issue in another PR, #48438. Thank you for attempting to fix the issue 🙏