Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Blocks: Add support for block name aliases for variations #43743

Open
Tracked by #60252
gziolo opened this issue Aug 31, 2022 · 13 comments
Open
Tracked by #60252

Blocks: Add support for block name aliases for variations #43743

gziolo opened this issue Aug 31, 2022 · 13 comments
Assignees
Labels
[Feature] Block API API that allows to express the block paradigm. [Feature] Block Variations Block variations, including introducing new variations and variations as a feature [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.

Comments

@gziolo
Copy link
Member

gziolo commented Aug 31, 2022

What problem does this address?

Part of Block API roadmap #41236.

Let's explore whether some block variations can be read as a block type. For example, instead of core/template-part/header be able to just have core/header to still get resolved by the template part functions. We used to have a similar granular naming schema for Embed and Social Link blocks before we converted them to block variations:

// Convert derivative blocks such as 'core/social-link-wordpress' to the
// canonical form 'core/social-link'.
if ( name && name.indexOf( 'core/social-link-' ) === 0 ) {
// Capture `social-link-wordpress` into `{"service":"wordpress"}`
newAttributes.service = name.substring( 17 );
name = 'core/social-link';
}

// Convert derivative blocks such as 'core-embed/instagram' to the
// canonical form 'core/embed'.
if ( name && name.indexOf( 'core-embed/' ) === 0 ) {
// Capture `core-embed/instagram` into `{"providerNameSlug":"instagram"}`
const providerSlug = name.substring( 11 );
const deprecated = {
speaker: 'speaker-deck',
polldaddy: 'crowdsignal',
};
newAttributes.providerNameSlug =
providerSlug in deprecated
? deprecated[ providerSlug ]
: providerSlug;
// This is needed as the `responsive` attribute was passed
// in a different way before the refactoring to block variations.
if ( ! [ 'amazon-kindle', 'wordpress' ].includes( providerSlug ) ) {
newAttributes.responsive = true;
}
name = 'core/embed';
}

Benefits of using aliases

  • Aliased block names would get converted into unique class names assigned to block variations, giving more options to style their sites.
  • Manipulate in theme.json granularly (like attach custom CSS that only is used when the block is used, etc.).
  • Aliases used would give easier access for site owners to the content analysis of which blocks and variations are used in posts and templates.
  • Benefit from all the block API tools in existence (like multiple block support).

What is your proposed solution?

Add handling for block type name aliases used with block variations. They should fall back if necessary to the original block type name, e.g. woocommerce/product-list that can get resolved to the product-list variation added to the core/query block.

Some examples to consider:

<!-- wp:template-part {"slug":"header","theme":"twentytwenty"} /-->
<!-- wp:meta { "name": "mykey", "type": "heading" } /-->

becomes

<!-- wp:template-part/header {"theme":"twentytwenty"} /-->
<!-- wp:meta/mykey { "type": "heading" } /-->

This could also remove the need for complex logic used with block variations when detecting the variation based on the result of the isActive callback executed.

@gziolo gziolo added [Type] Enhancement A suggestion for improvement. [Feature] Block API API that allows to express the block paradigm. [Feature] Block Variations Block variations, including introducing new variations and variations as a feature labels Aug 31, 2022
@gziolo gziolo mentioned this issue Aug 31, 2022
58 tasks
@gziolo gziolo changed the title Blocks: Add support for block name aliases used with variations Blocks: Add support for block name aliases for variations Aug 31, 2022
@gziolo gziolo added the Needs Dev Ready for, and needs developer efforts label Feb 27, 2023
@louwie17
Copy link

Hey @gziolo has there been any more progress around this idea/issue?
I think this would be a great feature to have and may really benefit us in WooCommerce when laying out the template for the new product editor.
We currently make active use of attributes and re-use blocks quite often, I think something like this would offer two big benefits for us:

  • Clean up the actual template by being able to remove a lot of our attribute values
  • Make better use of block hooks, as this will allow 3rd party plugins to hook into variations*

*This will depend if the above proposed solution will also work with block hooks.

A current example would be:
We have a section block, which renders a unique title/description and renders nested blocks. We have several sections across the product editor page. Like a general details section, a pricing section, an attributes section.
It would currently not be possible to extend after a specific section ( like pricing ) by a 3rd party plugin, but with the above they could, as they could hook into the wc:section/pricing for example.

@gaambo
Copy link
Contributor

gaambo commented Apr 25, 2024

I guess this would also allow setting allowedBlocks/template on innerBlocks to allow only some variations inside it? For Example we're using a variation of core-media/text to show a team member with some data - the innerblocks of the core-media/text block are locked. We then have a group block as section in which editors can insert these blocks. At the moment we've locked the group block to allow only core/media-text blocks, but of course the editors could also insert a non-team-member variation of the media-text blocks.
So this would be great for such use cases as well - if I understand it correctly?

@gziolo
Copy link
Member Author

gziolo commented Apr 25, 2024

So this would be great for such use cases as well - if I understand it correctly?

Potentially. It didn't occur to me initially, but now that you said it, it's worth considering as that would be a very interesting use case. It would also play nicely with block variations that define block bindings, so potential Site Title and Post Title could be replicated with the Heading block but maybe people still would want to have a control where to insert it.

@tjcafferkey
Copy link
Contributor

tjcafferkey commented May 3, 2024

I'm still in the process of understanding the complexities in this since this code is unchartered territory for me, but having given this a little bit of thought and there are a few things I think we need to consider, unless I have over complicated it:

  • Currently each variation can be registered using the registerBlockVariation function which only registers it client side for the purpose of the new variation being discoverable in the inserter (ie they're not registered as a new block type). If we want to create an alias registered on the server too we'd need to provide a PHP counterpart method or other means (block.json?) of allowing users to register their variation/alias block.
  • We need to update the block validation process to ensure when this alias'd block is inserted or read, it isn't reported as corrupt/errored.
  • Unless I'm misunderstanding how things currently work but I don't think the server understands the concept of variations the same way the client does. In the eyes of the server an original block and a variation block are one block with the same markup but different attributes/inner blocks. We'd need to think of a way to introduce this concept, I think?

@ntsekouras
Copy link
Contributor

Currently each variation can be registered using the registerBlockVariation function which only registers it client side for the purpose of the new variation being discoverable in the inserter (ie they're not registered as a new block type). If we want to create an alias registered on the server too we'd need to provide a PHP counterpart method or other means (block.json?) of allowing users to register their variation/alias block.

Variations can be registered client side with registerBlockVariation API and also in a block's block.json file either statically or dynamically(ex here).

Unless I'm misunderstanding how things currently work but I don't think the server understands the concept of variations the same way the client does.

I'm not sure if I misunderstood something here, but I don't think there is a difference in handling between client and server. In both cases we treat all variations of a block as the same single block. The difference is that with variations we have some selected attributes or inner blocks, only when we first insert them. After insertion we can update attributes and can end up not 'having' the original variation we inserted. This could be done either from UI with the block variation transforms or by simple updating the specific attributes in the markup.

I think this proposal is for adding aliases where we could still have one single block for editing etc.., but also having some more explicit info about the variation that we could use for other purposes. This would include changes in APIs to add this new info and always point to the shared code of the single block. So in places where have actions about blocks, this new API would also support to treat variations as standalone blocks, where in fact they are not.

Some examples that we need that is global styles and theme.json to add specific styles for specific block variations or in block manager to be able to disable/enable specific variations of the block and not holistically do that for the main block(allow only youtube variation).

It's a complex issue and I think some code explorations will start showing the way forward. I think we can do this iteratively and not update all the UI we would need it eventually. For start we should explore how we can update the registration of blocks and block variations to work with an alias and what an alias would be technically. For example what do we do with the isActive API? Will it be removed or actually preserved and used to add some meta in the block or dynamically generate some info to help us solve the above use cases.

@gziolo gziolo added [Status] In Progress Tracking issues with work in progress and removed Needs Dev Ready for, and needs developer efforts labels May 8, 2024
@ockham
Copy link
Contributor

ockham commented May 16, 2024

Requirements

One of the major questions is where we would use block variation aliases instead of the underlying, „canonical“ block names, and where we would use them in addition. This has been partly answered by @gziolo who mentioned the following goals:

  1. Use the aliases in persisted block markup in the DB (instead of canonical block names), e.g. <!-- wp:myplugin/my-cool-block-variation {"someAttr":"someValue"} /--> instead of <!-- myplugin/some-block {"someAttr":"someValue"} /-->). This will make it easier for tooling to e.g. find out what are the most frequently used block variations across a WordPress installation.
  2. Provide variation-specific class names, allowing 3rd parties to style a block variation differently from others.

OTOH, we're planning to retain the canonical block name in the editor, which means that there will have to be some logic to convert the alias (as found in the persisted markup in the DB) to the canonical name upon loading in the editor, and back upon saving. I've shared some thoughts on whether we need to include the alias in the markup in the editor (e.g. in block metadata) in order for the latter part to work (especially here and here); the tl;dr is that I think that we can avoid this, as we can infer the variation from the block's attributes.

In conclusion, we'll store the block alias in the persisted markup in the DB, but will convert it to its canonical name in the editor. However, we still need to answer the question of whether we'll use the alias or the canonical names for all of the in-between steps (e.g. in the parsed block format or in WP_Block instances on the server), and/or if we'll introduce an additional variation field in any of those. We'll get back to this question later.

Server-side API

A @ntsekouras mentioned, there’s an existing API on the server side to register block variations (variation_callback, get_block_type_variations). For simplicity’s sake, I’d like to avoid introducing any new functions or filters to register a variation alias; instead, it should be doable through the existing API.

Furthermore, variations already have a name field to distinguish them from each other. For example, the core/social-link block has variations named wordpress, x, tumblr, etc.

IMO, there’s a risk of confusion if we introduce aliases (which would be typically prefixed — think myplugin/cool-block-variation) as a separate concept that the user has to assign themselves. I’m thus leaning towards auto-generating alias names from the block type and the variation by concatenating them, separated by a slash.

(Note that this would require us to change the parser, and would pose another challenge described in more detail here.)

Auto-generating aliases from block type names and variation names has the drawback that 3rd-party plugins cannot use their own namespace for variations they add to existing blocks; e.g. #43743 had the example of a woocommerce/product-list alias for the product-list variation of core/query. This would have to become core/query/product-list instead, which isn't ideal, but probably not a dealbreaker. (It'd be nice to include some sort of vendor prefix/namespace in the part after the slash to avoid collisions, i.e. something like core/query/woo-product-list. This might be a bit trickier to implement, though -- especially for existing blocks.)

On the plus side, we already seem to be creating class names for block variations that closely match the pattern we're considering for the aliases. For example, the wordpress variant of the Social Link block already gets wp-social-link wp-social-link-wordpress assigned as its class names. This means that the second requirement mentioned at the top of this comment would be taken care of already.

Finally, note that using "auto-generated" aliases is also partly motivated by YAGNI -- it doesn't preclude allowing manual setting of aliases later. To keep the API simple, I'd add an alias (or blockAlias, or even createBlockAlias) field to the variation schema.

Backwards-compatibility and consistency of layers

One of the trickiest problems with using block variation aliases is the following dilemma: On the one hand, we want to expose them to extenders so that they can write code that targets them much like they would otherwise target canonical block names -- e.g. for Block Hooks to be able to inject a block variation. On the other hand, if we start exposing the variation name instead of the canonical name in certain places, this is likely going to break existing code.

I'll give an example. For Block Hooks' hooked_block_types filter to work with variation aliases, we'd basically need the parsed block's blockName field to contain the alias, rather than the canonical block name; i.e. <!-- core/social-link/wordpress /--> will result in

array(
    'blockName' => 'core/social-link/wordpress'
)

(rather than setting the blockName field to core/social-link).

IMO, this is fine: The parsed block format is a fairly low-level representation of the markup, generated by the block parser. (Note that we also likely don't want to introduce a dedicated blockVariation field, much like there's no dedicated blockNamespace field. Those are higher-level, semantic concepts that aren't reflected in the block parser.)

However, what about existing code that relied on the canonical block name -- e.g. a hooked_block_types filter that used core/social-link as its anchor block? That code would break if we started to persist aliases instead of canonical block names to the DB.

As a direct consequence, with the constraints above, we cannot automatically opt in all blocks that have variations to using auto-generated aliases in the persisted markup; instead, we'll have to make it a per-block user choice, likely through an alias field that for starters could be a boolean, telling WP whether to generate and use the alias name.

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.


Anyway, that's the current state of my research and thinking. Curious to hear your thoughts, especially from the folks who've worked with variations before @gziolo @ntsekouras. LMK if you agree with my assumptions and conclusions, and if I'm missing something 😄

@tjcafferkey
Copy link
Contributor

Thank you @ockham for documenting this so thoroughly! Something I want to pull out for discussion which I've brought up in DM with you is the following statement:

I've shared some thoughts on whether we need to include the alias in the markup in the editor (e.g. in block metadata) in order for the latter part to work (especially #61481 (comment) and #61481 (comment)); the tl;dr is that I think that we can avoid this, as we can infer the variation from the block's attributes.

I'd still be interested to see how this might work, I am currently under the assumption that this means that we can compare the current blocks attributes against all registered variation blocks default attributes to determine whether it's a variation or not (reminder; one of the main motivators for variations existing is allowing a user to create multiple versions of the same block with different attribute values). With that in mind, if the user inserts a variation and changes an attribute it would fall back to the canonical block type and wouldn't get converted upon serialization and persisted to the database.

I'm imaging scenarios where we want to insert the same variation with slightly different attributes, imagine the scenario where we create a variation of WooCommerce's Product Collection block (e.g. woocommerce/product-collection/best-sellers) and we want to use this variation across different patterns or templates but change the layout attribute to display products in a grid/list format depending on its context. The purpose of the variation remains the same, display best sellers but only the ones that match the default attribute get saved as the 'correct' variation, the others saved as the canonical block. A consequence of this would be that we could not reliably hook blocks against this alias.

Obviously this may only be an issue if I've understood what you mean correctly, or perhaps there's a way around this 😄

@ockham
Copy link
Contributor

ockham commented May 21, 2024

@tjcafferkey So I'd definitely wouldn't want the variation to become unrecognized if the user changes only some circumstantial attribute(s). But I thought we'd be safe from that happening as we'd only evaluate "distinguishing"/"defining" attributes?

There aren't a lot of examples of server-side registered variations in the WP and/or GB codebase, but here's one: The Post Terms block registers a variation for each known taxonomy (so one for Categories, one for Tags, etc?). The defining attribute is the block's term attribute:

'attributes' => array(
'term' => $taxonomy->name,
),

This means that if we're given a Post Term block (no matter if as markup, parsed, or WP_Block instance), we can tell what variant it is simply by looking at its term attribute -- and that's not going to be affected by changing any of its other attributes.

So the algorithm I'm envisioning would do something like this:

foreach ( known_variations_for_a_given( $block as $block_variation ) ) {
  foreach ( $block_variation->attributes as $attribute => $value ) {
    if ( $block->$attribute !== $value ) {
      continue 2;
    }
  }
  return $block_variation->name;
}

Or so. Maybe I'll give that a try somewhere, e.g. over at WordPress/wordpress-develop#6594 😬

@ockham
Copy link
Contributor

ockham commented May 21, 2024

Or so. Maybe I'll give that a try somewhere, e.g. over at WordPress/wordpress-develop#6594 😬

WordPress/wordpress-develop#6594 (comment)

@tjcafferkey
Copy link
Contributor

But I thought we'd be safe from that happening as we'd only evaluate "distinguishing"/"defining" attributes?

Ah, I see now the way in which you're thinking about this and I like the idea! One thing I just want to mention is that you could have a block variation whose attributes remain the same as the canonical block but its inner blocks are different. Just pulling out this sentence from the API documentation for reference.

The Block Variations API allows you to create additional versions of existing blocks that differ by a set of initial attributes or inner blocks.

From my understanding your approach should still work but we'd also need to run a comparison against $variation['innerBlocks'] as well as $variation['attributes']

@ockham
Copy link
Contributor

ockham commented May 21, 2024

But I thought we'd be safe from that happening as we'd only evaluate "distinguishing"/"defining" attributes?

Ah, I see now the way in which you're thinking about this and I like the idea! One thing I just want to mention is that you could have a block variation whose attributes remain the same as the canonical block but its inner blocks are different. Just pulling out this sentence from the API documentation for reference.

The Block Variations API allows you to create additional versions of existing blocks that differ by a set of initial attributes or inner blocks.

From my understanding your approach should still work but we'd also need to run a comparison against $variation['innerBlocks'] as well as $variation['attributes']

Oh, that's a very good point, thank you!

I just had another look at the docs myself, with a focus on the isActive part. When used on the client side, that's typically a function that helps WP determine which block variation is currently active, based on information such as attributes -- in other words, pretty much what we're doing in infer_block_variation.

isActive: ( blockAttributes, variationAttributes ) =>
   blockAttributes.providerNameSlug === variationAttributes.providerNameSlug,

Of course we cannot use a JS function like isActive on the server side, but there's a second format that isActive accepts, which is an array of (distinguishing) attributes.

isActive: [ 'providerNameSlug' ]

That's a format that we could actually use (and evaluate) on the server side! We might want to give precedence to that isActive field, and fall back to our current behavior (which is to compare all variation-defining attributes).

@tjcafferkey
Copy link
Contributor

That's a format that we could actually use (and evaluate) on the server side! We might want to give precedence to that isActive field, and fall back to our current behavior (which is to compare all variation-defining attributes).

Makes sense. Although it is my understand that callbacks assigned to the isActive property can become complex which is why @gziolo would like to see this removed (see quote below from the original issue desc). I don't know a tonne about how it's used currently though but it's worth considering how we can still remove that logic and infer a variation if it's possible at all.

This could also remove the need for complex logic used with block variations when detecting the variation based on the result of the isActive callback executed.

@ockham
Copy link
Contributor

ockham commented May 27, 2024

Update:

Based on our research and experiments, we're planning to generate block aliases by concatenating block name and variation (i.e. the wordpress variation of the core/social-link block gets the core/social-link/wordpress alias). The alias will then be used in the persisted markup, and at the lowest levels of block processing on the server side; it will however be converted back to the canonical block name at higher levels, and on the client side.

Block authors will need to opt in to have aliases generated for their blocks, for reasons given here.

We have a prototype that seems to be working reasonably well in WordPress/wordpress-develop#6594 (which is based on prior art by @tjcafferkey). We'll work to polish that PR and get it ready for inclusion in 6.6.

Since this will affect the way that block markup is stored and processed, there's a risk that this will meet some opposition, or that it might impact existing code (both Core and 3rd party) in a way that we're not yet aware of. To deal with that risk, we'll break the problem into multiple parts, and will frontload the less risky parts, as they're required for the riskier parts anyway, and have a higher chance to make it into 6.6:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block API API that allows to express the block paradigm. [Feature] Block Variations Block variations, including introducing new variations and variations as a feature [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

6 participants