Fix fatal error in Site Editor if a block defines an array of style/editor_style handles

[mikejolley]

Description

We're currently working on making WooCommerce Blocks compatible with FSE, and one issue we've come across is the way in which handles are being merged in gutenberg_extend_block_editor_styles_html here:

gutenberg/lib/client-assets.php

Lines 718 to 728 in da8555d

	foreach ( $block_registry->get_all_registered() as $block_type ) {
	if ( ! empty( $block_type->style ) ) {
	$handles[] = $block_type->style;
	}
	if ( ! empty( $block_type->editor_style ) ) {
	$handles[] = $block_type->editor_style;
	}
	}
	$handles = array_unique( $handles );

Some of our blocks do not have a single editor_script handle, for example here we include the block styles, and the vendor styles, so the handle is an array, not a string.

This results in a white screen/fatal error when loading up the Site Editor (array to string conversion).

To fix this, this PR checks to see if a handle is a string or an array, and merges it appropriately.

How has this been tested?

• Ensure Gutenberg, WooCommerce and WooCommerce Blocks are active, as well as a theme supporting FSE
• In admin, go to the site editor. See white screen. Debug log will show an array to string conversion error
• Apply patch. Go to site editor. Page loads successfully.

Types of changes

This is a Bug fix for the style handle handler.

@nerrad recommended I ping @noisysocks in case this fix needs porting to WP 5.7.

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• I've tested my changes with keyboard and screen readers.
• My code has proper inline documentation.
• I've included developer documentation if appropriate.
• I've updated all React Native files affected by any refactorings/renamings in this PR.

[noisysocks]

Do you know why this used to work? I'm surprised because the doc comments in WP_Block_Type say that $style and $editor_style must be a string or null.

https://github.com/WordPress/wordpress-develop/blob/69032204d511bc9ca71ee6e3f025aefef14693ee/src/wp-includes/class-wp-block-type.php#L166-L180

We'll have to update that documentation in Core and potentially any other code that uses WP_Block_Type.

[TimothyBJacobs]

This used to work because wp_enqueue_registered_block_scripts_and_styles just passes the handles directly onto the WP_Dependencies API which accepts an array of handles in enqueue.

[mikejolley]

@noisysocks Can you let me know if this is going to be enforced (string/null) or will support an array of handles? If it's going to be string only we'll need to make changes on our side. cc @nerrad

[nerrad]

Personally I think it should support whatever the WP_Dependencies API supports, and based on @TimothyBJacobs 's comment, whatever change happened in GB that prompted this need for this fix is a regression. The bug here seems to have been incorrect typing in the jsdocs prior to the regression?

So I'm in favor of merging this pull request and fixing the jsdocs as well.

[TimothyBJacobs]

This is definitely a regression, but I don't think we should adjust this. I think array support was only incidental in that the lower level WP_Dependencies API supports an array of handles.

It's specified as a string in the RFC. In the PHP docs, the REST API endpoints, etc...

If we allow for an array of handles, that means any downstream code has to handle both the array format and a string format. Whereas if we were to have allowed arrays in the beginning, we would have allowed a string, but upgraded it to an array during registration.

I'm also not sure why it is necessary. What are the instances where an array of handles is needed instead of adding the additional handles as dependencies to the original handle?

[nerrad]

Good points Timothy. @mikejolley I think we had a discussion on this and offhand we couldn't recall why we pass two dependency handles in to the registration API. I agree, it seems like it's something that could be addressed in how WooCommerce blocks registers things. Mike, any additional thoughts?

[mikejolley]

I agree, it seems like it's something that could be addressed in how WooCommerce blocks registers things. Mike, any additional thoughts?

Yeah we can update our usage. Closing this, I don't know if GB should do some type checking on these values instead of breaking though, even if arrays are not supported ¯_(ツ)_/¯