Usage of pre_render_block filter without returning value

[danieliser]

Describe the bug

Seems that somebody used add_filter to handle action like code, but didn't return anything, so filtering before WooCommerce results in corrupted values.

Add filter & callback. This is done in 2 different files in that directory.

The other file is RelatedProducts.php

Impact on pre_render_block in this way means that if you returned '' (to remove a block), at priority 9 or 10 , WooCommerce's filters will void that out by not returning anything.

Hook in at 11 and the issue is gone (for us).

But I know we aren't the only plugin using that filter, so this is likely mucking up other things without any real indication its happening.

I should add this only occurs on WooCommerce pages.

To reproduce

Steps to reproduce the behavior:

1. Create woocommerce my account page.
2. Either use our plugin Content Control to add rules to a block (Show to logged out users only makes for easy test), or manually filter pre_render_block at priority 9 and return '' for any block to remove it from the page.
3. The block will still appear on woocommerce pages, but not on others.

Expected behavior

These 2 callbacks should at minimum return $pre_render;, and should ideally be optimized to bail early in cases where $pre_render is set but to empty-ish values.

If the item was removed from page, no need to run additional checks for loading assets etc.

Environment

WordPress (please complete the following information):

• WordPress version: 6.3.1
• Gutenberg version (if installed): Latest
• WooCommerce version: 8.1.1
• WooCommerce Blocks version: bundled with WooCommerce 8.1.1
• Site language:
• Any other plugins installed: Content Control

[gigitux]

Hey @danieliser,
Thanks for reporting the issue!
I'm able to replicate it.

With the current snippet:

function update_render_block() {
 return "";
}		

add_filter('pre_render_block','update_render_block', 9);

Blocks are still visible on the page.

With

function update_render_block() {
 return "";
}		

add_filter('pre_render_block','update_render_block', 11);

the blocks are not visible.

I'm unsure how adding return $pre_render_block could fix the issue. Could you expand this idea, please?

To ensure that your filter is applied, you have to ensure it has the highest priority. Am I missing something?

[danieliser]

@gigitux

I'm unsure how adding return $pre_render_block could fix the issue. Could you expand this idea, please?

To ensure that your filter is applied, you have to ensure it has the highest priority. Am I missing something?

In general A filter is used to manipulate a value, as such apply_filters expects every callback to return a value that will then be passed to the next hooked callback.

In this case though WooCommerce is not touching the data being filtered, but instead treating it like an action, and doing side-effects based on the block attributes/type. But by not returning the value passed in, it effectively breaks the functionality.

As such yes, priority of higher does solve our problem, but its not the right solution to this issue, our fix is kinda irrelevant (see examples at bottom).

Consider the following example using the_content instead of pre_render_block

add_filter( 'the_content', function ( $content ) {
    // Log that content was viewed.
    some_function_call(); // irrelevant to the $content variable.
}, 10 );

When I call the_content() it will now come back blank, it won't even contain the original post content.

However if I do the following, it works as expected.

add_filter( 'the_content', function ( $content ) {
    // Log that content was viewed.
    some_function_call(); // irrelevant to the $content variable.
    
    return $content;
}, 10 );

The callbacks in question here should look like this, in effort to always return a valid value.

	public function update_query( $pre_render, $parsed_block ) {
		if ( 'core/query' !== $parsed_block['blockName'] ) {
			return $pre_render;
		}

		$this->parsed_block = $parsed_block;

		if ( self::is_woocommerce_variation( $parsed_block ) ) {
			// Set this so that our product filters can detect if it's a PHP template.
			$this->asset_data_registry->add( 'hasFilterableProducts', true, true );
			$this->asset_data_registry->add( 'isRenderingPhpTemplate', true, true );
			add_filter(
				'query_loop_block_query_vars',
				array( $this, 'build_query' ),
				10,
				1
			);
		}

		return  $pre_render;
	}

The reason this seems like a non-issue on the face of it, is that pre_render_block is null by default. It's purpose is to allow short-curcuiting block render with custom output, or in our case none at all.

Not returning anything essentially sets it back to null. So this flaw in WooCommerce filter callback only presents if something is actively changing outputs of a block.

Some examples of why this needs to be fixed, and likely optimized further than the fix:

A site might overload the image block with custom output, they might do so on priority 1 in effort to short-curcuit as early in the process as possible. Every filter after should be aware that the value of $pre_render_block might not be null, and if so, should act accordingly.

Further optimizing might bail (return) early if $pre_render_block is not null, and is empty. This would prevent overloaded blocks from loading assets they don't need.

	public function update_query( $pre_render, $parsed_block ) {
		// Bail early if pre_render isset & empty or if this isn't core/query block.
		if ( 'core/query' !== $parsed_block['blockName'] || '' === $pre_render ) {
			return $pre_render;
		}

		$this->parsed_block = $parsed_block;

		if ( self::is_woocommerce_variation( $parsed_block ) ) {
			// Set this so that our product filters can detect if it's a PHP template.
			$this->asset_data_registry->add( 'hasFilterableProducts', true, true );
			$this->asset_data_registry->add( 'isRenderingPhpTemplate', true, true );
			add_filter(
				'query_loop_block_query_vars',
				array( $this, 'build_query' ),
				10,
				1
			);
		}

		return  $pre_render;
	}

[danieliser]

All of this said, might be worth using phpstan or similar to find any uses of apply_filters that has callbacks that don't return a value. These should all be addressed as they fundamentally break how WP is intended to function.

[gigitux]

Hey @danieliser,
Thanks for your explanation! I added the return value, but the filter doesn't work (with priority 9). I discovered that in WordPress Core, the pre_render filter is used without returning any value (https://github.com/WordPress/wordpress-develop/blob/d8206d2167f2c082258f31db163289bae3682e06/src/wp-includes/block-supports/elements.php/#L135-L225)

I will open a PR for WooCommerce Blocks, but this will not fix the issue.

[danieliser]

@gigitux - Thanks for the patch. I see you caught an additional case I didn't mention above, thanks.

As for your findings about core.. Good catch, and after reading over their code there, I will be filing issues with WP Core as well.

During my test this is what I found, when I hook in at 10 ( not 9 like my explicit example ), then its much more hit/miss.

That is because my filter might be coming after core & before WooCommerce, since all were set to the same priority of 10.

When that oocurs, and only on WooCommerce pages, this fix does work.

So since the docs on that filter show a priority of 10 as the default/sample code, this fix should actually be beneficial overall just the same.

---

Per the docs

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

IE all filters hooked in there should respect an existing non-nullish value, even those in core, especially since they are not hooked in at priority 0.