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

Blook Hooks: Validation errors are thrown if you attempt to use blockHooks in a block with a save function. #54796

Closed
ndiego opened this issue Sep 25, 2023 · 7 comments
Labels
[Feature] Block API API that allows to express the block paradigm. [Type] Bug An existing feature does not function as intended

Comments

@ndiego
Copy link
Member

ndiego commented Sep 25, 2023

Description

I was testing the new Block Hooks feature with a custom block I created that has a render function as well as a save function (i.e. neither dynamic nor static).

I applied the following to the block.json file, and the book hook technically works as expected.

"blockHooks": {
	"core/comment-template": "lastChild"
},

However, in the Editor, a block validation occurs due to the save function. See screenshots below.

It might be good to handle these errors more gracefully, or perhaps prevent blocks with a save function from working altogether?

Step-by-step reproduction instructions

  1. Use a custom block that contains a save function (or use mine))
  2. Add the above code to the block.json file.
  3. Using TT3, navigate to the Comments template part in the Site Editor (make sure any customizations are cleared), and observe the validation errors.

Screenshots, screen recording, code snippet

image image

Environment info

  • WordPress 6.3
  • Gutenberg 16.7 RC2

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

No

@ndiego ndiego added [Type] Bug An existing feature does not function as intended [Feature] Block API API that allows to express the block paradigm. labels Sep 25, 2023
@ndiego
Copy link
Member Author

ndiego commented Sep 25, 2023

cc @ockham @gziolo

@gziolo
Copy link
Member

gziolo commented Sep 26, 2023

It might be good to handle these errors more gracefully, or perhaps prevent blocks with a save function from working altogether?

That's a good point. I would say that for a start, we should allow only dynamic blocks with render_callback (render applies, too). There is no way to use save in PHP to automatically inject the block with the expected HTML syntax.

More broadly, I believe there should be a way of saying - save function doesn't use HTML to store any attributes (it might not be a strict requirement), so we can use it as a fallback. In effect, it should NOT be used for block validation. In particular, the block referenced in the description, will get reported as invalid when someone opens the editor after the current year changes. The reason is that the block gets saved to the database the following syntax after it gets inserted into the editor in the regular flow:

<p class="outermost-copyright-date-block">© 2017-2023</p>

When someone opens the editor in 2024, then the save will produce:

<p class="outermost-copyright-date-block">© 2017-2024</p>

This is enough to trigger the block invalid warning.

I hope that we can improve handling for dynamic data like the current year with #54536.

@bph bph moved this from Triage to Needs Dev / Todo in WordPress 6.4 Editor Tasks Sep 30, 2023
@ockham
Copy link
Contributor

ockham commented Oct 2, 2023

Interesting, that's a pattern that hadn't occurred to me 🤔

A simple WordPress block plugin that utilizes both a render.php file and a save.js file. On the front end, the render.php file takes precedence, but if the plugin is ever disabled, the save.js file serves as a fallback, so a copyright date is still displayed.

The other examples of having both a render.php and a save.js that I seem to recall are typically along the lines of a "template" block, i.e. something like the Comment Template block that the user can customize in the editor (by rearranging inner blocks etc), which is then "populated" on the frontend with data from the DB that is dynamically fetched by PHP.

The problem you're encountering is indeed somewhat inherent in how Block Hooks work: As @gziolo said, since we want them to function on the frontend even if the user never loads and saves them in the editor, they have to be dynamic, and have to render the desired output from their default state, i.e. with no attributes set or inner blocks present. However, since save() is called by the editor during load time for block validation, it's bound to cause a validation error if the default markup returned from save() is non-empty -- which in the context of your example block is by design.

This kind of block validation happens at a fairly low level, so I'm leaning towards disallowing blocks like that from being used as hooked blocks. Note that we might be able to widen the set of allowed blocks a bit beyond "only has a render_callback but not save.js": It's probably permissible to have a save.js in addition to a render callback, as long as that save.js returns the exact same output as does the render callback, if invoked with no attributes set (i.e. only default values) and no inner blocks present. This might allow for (some) "template-style" blocks such as Comment Template to be used as hooked blocks (but I'll have to verify, and maybe tweak the markup returned from save.js and the render callback a bit).

@bph
Copy link
Contributor

bph commented Oct 6, 2023

@ndiego @ockham @gziolo This seems to be more discussion on what's possible and what's not rather than about something that can be fixed. It's more about defining the boundaries. It made it on to the project board for WordPress 6.4 release, though. What's are your thoughts about taking it off the board?

@ockham
Copy link
Contributor

ockham commented Oct 9, 2023

This seems to be more discussion on what's possible and what's not rather than about something that can be fixed. It's more about defining the boundaries. It made it on to the project board for WordPress 6.4 release, though. What's are your thoughts about taking it off the board?

Sounds fine to me 👍 (@ndiego feel free to veto/overrule 😄 )

@ndiego
Copy link
Member Author

ndiego commented Oct 9, 2023

Sounds good

@ockham
Copy link
Contributor

ockham commented Mar 25, 2024

AFAICT, this is no longer a limitation in WP 6.5 (see e.g. the Copyright Date block being used in this example in the Dev Blog post on Block Hooks).

@ockham ockham closed this as completed Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block API API that allows to express the block paradigm. [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

4 participants