Respect non-null values from the `pre_render_block` filter

[danieliser]

Currently WP core hooks into pre_render_block in 2 places.

In both cases they hook in at priority 10, and in both cases they explicitly return null.

This means anybody following the example usage of that filter could end up with their return not respected unless they change priority to higher than 10, which is not documented.

Either those filters should be moved to an earlier priority, such as 0, or they should expect & respect non-nullish values by not returning null and instead returning $pre_render.

Further if these are conditional loads based on block type, they should probably check if ! is_null( $pre_render ) and if so, act accordingly and not enqueue assets or bail early etc.

• ​https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/block-supports/settings.php#L77-L149

• ​https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/block-supports/elements.php#L135-L222

[danieliser]

Ugg, no edit, forgot to add, this is what Documentation says about that filter:

Allows render_block() to be short-circuited, by returning a non-null value.

Thus these return null; don't respect the documented pattern.

[pbiron]

Maybe I'm misunderstanding, but core's use of that filter seems perfectly fine to me.

1. core does hook into the filter at the default priority (10). but it does that during the bootstrap process before plugins (or themes) are loaded. So, any plugin/theme that hooks into the filter at the default priority will have their callback fired after core's callback...which is what they'd want.

2. core's callbacks for that filter return null precisely because core does not want it's callbacks to short-circuit the rendering of the block. Thus, in effect, core is treating the filter more like an action. If a plugin/theme hooks into the filter (at any priority of 10 or more) and does return a non-null value (assuming their callback is the last one called), then their non-null return value will be honored (and the rendering of the block will be short-circuited...which is what is supposed to happen.

Can you please explain more about why you think there is a problem?

[dmsnell]

@danieliser for more context here, and thank you @pbiron for sharing above, this filter performs a kind of inverted goal to most filters. it's only wanting to know if the rendering process should be circumvented, and so there's an ambiguity if it returns content. with null it's clear that some filter took care of it and so the process can abort.

the original block is passed as a second argument to the filter, so it's still possible to inspect what was there before, but if some plugin removes a block from the rendering stage, there's nothing left to filter after that point.

are you trying to avoid some enqueued styles?

[jeremyfelt]

IMO, the general promise with pre_ filters in core is that I can short-circuit something entirely.

In pre_render_block's current form, if I add_filter( 'pre_render_block', '__return_empty_string', 1 );, I would expect that all blocks are rendered as an empty string.

Because core's own use of pre_render_block does not respect the value of $pre_render set earlier than priority 10, the filter doesn't work as promised. Instead, I need to set my priority so that it runs after core.

If I shift my priority to 11, and acknowledge that core will always process this block before I decide alter its output, then core will always prepare styles that I may not need via wp_render_elements_support_styles() and _wp_add_block_level_preset_styles(). That's somewhat annoying, but not as annoying as the block continuing to render. :)

It seems that if these two functions are treating pre_render_block as an action rather than a filter, they should pass through the value of $pre_render untouched rather than force it to always be null. That's the patch I would propose here.

I agree with @danieliser that the style processing should ideally not run at all if ! is_null( $pre_render ), but I also understand the complication here. I'm not sure what a way forward to solve this looks like other than removing those hooks whenever my pre_render filters something.

[dmsnell]

@jeremyfelt that seems quite reasonable, with one additional caveat that maybe those functions abort early if $pre_render is already null.

[pbiron]

Replying to jeremyfelt:

IMO, the general promise with pre_ filters in core is that I can short-circuit something entirely.

Agreed...and the last callback hooked to such filters is what determines whether something is short-circuited.

In pre_render_block's current form, if I add_filter( 'pre_render_block', '__return_empty_string', 1 );, I would expect that all blocks are rendered as an empty string.

I would expect all blocks to be rendered as empty strings if no other callbacks are hooked after the above, and it is documented that core hooks into the filter at priority 10. So, if you really want to ensure that __return_empty_string wins, then hook at PHP_INT_MAX (and make sure that you hook in at that prio after every other plugin/theme has done so ;-).

Because core's own use of pre_render_block does not respect the value of $pre_render set earlier than priority 10, the filter doesn't work as promised. Instead, I need to set my priority so that it runs after core.

It seems that if these two functions are treating pre_render_block as an action rather than a filter, they should pass through the value of $pre_render untouched rather than force it to always be null. That's the patch I would propose here.

Agreed again...since core's callbacks to this _filter_ is acting like an _action_, core's callbacks should return $pre_render rather than null. Jeremy, would you like to do the honors of submitting a PR/patch?

With Dennis' comment (he commented while I was composing this ;-), I think a patch to return $pre_render instead of null will get a lot of support (including from me). But I'm weary of Dennis' "additional caveat", because I can imagine a plugin rendering a block almost like it would "normally" but maybe adding a data-xyz attribute to some HTML element...and expecting core to have output the styles it would normally do.

[danieliser]

I concur that there is a possible side effect with script loading.

That likely requires changing how those things are handled from a hacked filter to a proper action after render is decidedly going to happen.

That action likely should also include a filter for the scripts to be loaded if I were wishing out loud.

Then you could not only overload the output of a block, but also filter which scripts/styles would be loaded for it.

---

For the record we did just change our implementation to be higher priority than 10, but this is decidedly so a break from WP norms and the current documented usage, which should be simple enough to address.

[dmsnell]

maybe these core filters should be registered as add_action() instead of add_filter()?

But I'm weary of Dennis' "additional caveat", because I can imagine a plugin rendering a block almost like it would "normally" but maybe adding a data-xyz attribute to some HTML element...and expecting core to have output the styles it would normally do.

I'm not following this, but I guess I misunderstood this whole thing myself in assuming null meant "no more block" - feels like double negatives here 😅

so yeah, ignore that additional thing or maybe instead of null === $pre_render it should be '' === $pre_render - all I meant to do was have Core skip any enqueued styles if the block has been removed.