REST Block Renderer: Fails when using the Advanced > Additional CSS Class functionality

[akirk]

When creating a block with a ServerSideRender component and using the "Additional CSS Class" block option, the rendering breaks with this HTTP response:

{"code":"rest_invalid_param","message":"Invalid parameter(s): attributes","data":{"status":400,"params":{"attributes":"className is not a valid property of Object."}}}

The reason for this is that the Block Renderer just takes the list of default attributes and className is not one of them.

I came across this when actually introducing another attribute via a editor.BlockEdit filter which adds an attribute to all blocks but there is no chance to tell the REST renderer about it.

I first submitted this as ​https://github.com/WordPress/gutenberg/pull/13249

[kadamwhite]

After discussion in today's meeting we have questions for the block editor team around the desired functionality here. It was suggested that this is a documentation issue, not a code issue; but I think it's more complex than that, per the (verbose -- sorry!) description below.

To re-state the problem: In JS-rendered blocks, we implicitly support Additional CSS. If I create a block like this,

registerBlockType( 'trac/ticket45882', {
        title: 'Advanced CSS SSR Demonstration 1',
        description: 'Show that Advanced > Additional CSS requires no client-side opt-in.',
        icon: 'calendar',
        category: 'common',
        edit() {
                return (<p>Today's number is 5!</p>);
        },
        save() {
                return (<p>Today's number is 5!</p>);
        },
} );

and I specify "custom-css-class" for Additional CSS, then those classes are reflected in the rendered output:

<p class="wp-block-trac-ticket45882 custom-css-class">Today’s number is 5!</p>

and className will appear if I access and echo "attributes" within edit() or save().

Now, I update my block to this:

        save() {
                return null;
        },

and implement a dynamic block on the server side:

<?php
function ticket_45882_render_callback() {
        return sprintf( '<p>Today\'s number is %d!</p>', mt_rand( 1, 100 ) );
}
register_block_type( 'trac/ticket45882', [
        'render_callback' => 'ticket_45882_render_callback',
] );

then set "custom-css-class" for Additional CSS, I will not see that class reflected in the rendered output. To see Additional CSS classes in the output, I must manually output $attributes['className']; this is a documentation issue. However I do _not_ need to declare className as an attribute anywhere in order to support Additional CSS; I just need to render it if it is present. This is enough:

function render_callback( $attributes ) {
        return sprintf(
                '<p%s>Today\'s number is %d!</p>',
                isset( $attributes['className'] ) ? ' class="' . esc_attr( $attributes['className'] ) . '"' : '',
                mt_rand( 1, 100 )
        );
}

I won't get the wp-block- class, but I will successfully get <p class="more-custom-css">Today’s number is 47!</p>. If this is a bug, the bug is that we show the Advanced CSS input field at all when the block is server-rendered and we do not know if className is handled.

The issue described in this ticket is separate: the fact that the ServerSideRender handler will fail unexpectedly if className is present in the attributes passed to the SSR callback.

My question for the Block Editor team (cc @youknowriad) is:

Does this seem like a good solution to add className to all server-rendered blocks, knowing that the class name will not do anything if it is not explicitly handled? If so, we should move forward with this patch or a patch like it. (I could also see us solving this by explicitly permitting className in the SSR handler, rather than requiring it to be in attributes, for better parity with how we handle it in JS.)

Or, instead, should we say that if you are using className in your PHP render callback, you _must_ specify className in a PHP-side attributes array in your server-side block registration? In this case, we would make no code change here and treat it as a documentation issue; possibly we could then open a new issue to hide the Additional CSS input for blocks to which it will not apply.

I lean towards the former but I'm not confident that I understand the repercussions of this patch on all types of dynamic blocks.

[mcsf]

In terms of what should be correct, I suspect we need a filter for get_attributes like the one in the patch.

Further, since client-side hooks can be applied to blocks indiscriminately based on specific supports rules, it should be possible on the server to hook into blocks based on the same rules. That way, a plugin can make sure that its client-side extensions don't harm server-side expectations of blocks. However, AFAIK, there is no current server-side knowledge of a block type's supports.

Regardless of what we decide for SSR documentation, it seems that a patch for this should cover supports as well.

[phpbits]

Custom attributes added via blocks.registerBlockType JS filter are having the same error. JS filters should communicate to PHP displays or add similar filters for server side rendering. Thanks!