Try: Render variation alias blocks

[tjcafferkey]

Trac ticket:

---

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

[ockham]

As @tjcafferkey mentioned in a DM, variations are still missing a class name on the frontend. E.g. replacing wp:social-link with wp:core/social-link/wordpress in the markup, we get this on the frontend:

The reason is that the block is missing the wp-block-social-link class.

[ockham]

Per https://github.com/WordPress/gutenberg/issues/43743#issuecomment-2114811994:

The next layer where we need to decide how to represent aliases is WP_Block instances. This layer is a lot more semantic that the parsed block format; among other things, it validates the block name based on what blocks are registered. For this reason (and others), I think that the WP_Block instance's $name field should reflect the canonical block name, and that we need to introduce additional fields and/or methods to reflect the current variation.

In the vein of that comment, I tried to apply the following patch to this PR:

diff --git a/src/wp-includes/class-wp-block.php b/src/wp-includes/class-wp-block.php
index 960bee9260..9f3a3a14ce 100644
--- a/src/wp-includes/class-wp-block.php
+++ b/src/wp-includes/class-wp-block.php
@@ -128,7 +128,7 @@ class WP_Block {
         */
        public function __construct( $block, $available_context = array(), $registry = null ) {
                $this->parsed_block = $block;
-               $this->name         = $block['blockName'];
+               $this->name         = $this->get_canonical_block_name( $block['blockName'] );
 
                if ( is_null( $registry ) ) {
                        $registry = WP_Block_Type_Registry::get_instance();
@@ -136,8 +136,7 @@ class WP_Block {
 
                $this->registry = $registry;
 
-               $block_name       = $this->get_canonical_block_name( $this->name );
-               $this->block_type = $registry->get_registered( $block_name );
+               $this->block_type = $registry->get_registered( $this->name );
 
                $this->available_context = $available_context;
 

Since this would set $this->name to the canonical block name pretty much at the start of the lifecycle of any WP_Block instance, I thought it would fix the missing classname issue. However, to my surprise, that issue still persists.

This is quite puzzling to me. One potential explanation I could see was that the class name was generated based on the parsed block's blockName field rather than the WP_Block instance's name field, but this code seems to indicate otherwise.

We'll need to investigate this a bit more.

[tjcafferkey]

We'll need to investigate this a bit more.

This is happening because when we apply block support functionality we pass in the $parsed_block which fetches the matching WP_Block_Type and we haven't yet registered a WP_Block_Type for the variation block (only a WP_Block).

Fetching the WP_Block_Type of the canonical block fixes this issue (https://github.com/WordPress/wordpress-develop/pull/6555/commits/480b3544ca2345b2bafd2b812a56adc83e489d3c), however the WP_Block_Type isn't aware of whether it is a variation of itself or not (because we've not registered it).

We may need to either a) register variations as a WP_Block_Type b) Provide the ability to pass in the variation name to generate class names for.

At the moment I'm leaning towards option B because I don't think we should register variations as WP_Block_Types but instead use the canonical blocks instance since that's what variations are.

[tjcafferkey]

~@ockham I don't necessarily think these are the correct solutions but here are two approaches I experimented with to demonstrate the problem~

• ~https://github.com/WordPress/wordpress-develop/pull/6555/commits/9741349d511ede63a737fd014d7ca55c509c887e~
• ~https://github.com/WordPress/wordpress-develop/commit/2c5ac25a2ab38c01958bd85ae4bb0b43fac78b63~

Edit: Keep scrolling

[tjcafferkey]

In this PR currently when we retrieve the WP_Block_Type we do so by inferring the canonical block name, looking that up in the registry and then setting ->name as the alias block name as seen here.

Whereas in the WP_Block instance we again infer the canonical block name of an alias, and set that its ->name as the canonical name, as seen here.

What this means is that we will have a WP_Block instance and a WP_Block_Type instance of the same alias block. The former with a name property of namespace/block-name and the latter with a name property of namespace/block-name/alias-name

This is a mismatch I believe we should fix.

[tjcafferkey]

Following on from my previous comment, it feels as though we need to always return the instance of the canonical block. Here are two other approaches for this same problem.

• https://github.com/WordPress/wordpress-develop/pull/6580
• https://github.com/WordPress/wordpress-develop/pull/6582