Block API: Deprecate block transform function in favor of convert

[aduth]

Closes #15972
Related: #17743
Previously: #14908, #11979

This pull request seeks to stabilize and promote what is currently implemented as the __experimentalConvert function member of a block transformation. In doing so, it deprecates the existing transform method. convert differs from transform in that it will receive the full block object (or an array of block objects if the transform has isMultiBlock: true). By contrast, the transform function would receive an attributes object and innerBlocks array (or an array of attributes objects and an array of innerBlocks arrays if the transform has isMultiBlock: true). The multi-block transform signature was especially cumbersome to use, a consequence of the fact that it was added at a later time (#11979, see also a similar reflection on this at the time at #11979 (review)).

Existing use of transform should be unaffected, but will log with a deprecation warning. All existing core block transforms have been updated. Likewise, the documentation has been updated to reflect the recommended usage. Furthermore, it has been enhanced to detail supported usage of either transforming from or to multiple blocks.

Needs Decision: Throughout the process of putting together this branch, I've rediscovered the many inconsistencies we've carried with us in how we support transforms of varying types and use-cases. Much of this was summarized at #15972 (comment) and subsequent comments. There are also related enhancement requests at #17758 and #14755. Even with these changes, there are still some inconsistencies around how we perform various block operations, where some functions (transform isMatch, deprecation migrate, shortcode transform convert will still receive an "attributes" object). I don't know that all of these would make sense to similarly port to a signature which receives a full block object. In the case of the shortcode transform, "attributes" holds a different meaning, and it and the "files" transform don't operate on blocks as the source. But, as mentioned in #17758, a shortcode does have a similar need for an "inner value" as part of its transformation. It would be worth having a discussion to settle on a clear picture for if and how we consider block operations in how it makes sense to devise a function signature.

Testing Instructions:

Verify there are no regressions in the behavior of block transforms.

[chrisvanpatten]

As an implementer, any progress toward introducing consistency in the transform API would be greatly appreciated. I think the convert naming makes a lot of sense, and standardizing that it always receives a block or blocks, rather than parsed out pieces of blocks, will make all the APIs easier to use and more intuitive. Right now I have to confess even though I've done a fair amount of implementation work around transforms, I still find myself needing to hunt through code and documentation to fully understand what I'm doing; consistent APIs will certainly help to mitigate that!

[kadamwhite]

Regarding Shortcodes, I'd ideally like to see this same type of function signature, with a convert method on the transform that accepts a shortcode object (with attributes, name, and content properties, etc) where the methods in this file receive a block object. This would give us the flexibility to convert a shortcode however we require by returning an array of blocks just as if we were writing a convert method.

(Any one-to-one shortcode-to-block transform will fall down, I believe, because there's sometimes hierarchies of content in shortcodes which would be represented differently in blocks. As an example, I just converted a wrapping [accordion] shortcode with nested [accordion-item] shortcodes into one single AccordionItem block that accepts inner content; so I'd want the freedom to be able to parse any accordion shortcode items detected in content into multiple AccordionItem blocks, rather than having to create and later discard a wrapper block representing the no-longer-needed parent accordion. If this example doesn't make sense, apologies; the tl;dr is that arbitrary block creation given the parsed attributes and string contents of a shortcode would be ideal!)

[oandregal]

Just came across this one via #15972 In #21734 I've documented and extracted the Block Transforms API to a separate page. If we end up doing this we'd need to update also those docs.

[ajitbohra]

Love the simplicity here block(s) in block(s) out

[guarani]

Looks like this needs to be rebased with master and conflicts resolved to be testable on mobile since this PR pre-dates the monorepo changes.

[gziolo]

@mtias or @youknowriad - what’s the plan regarding this API change?

[getdave]

@gziolo Do you want me to pick this up again? I originally wrote convert() and it provides more information transform. Appreciate it exposes a wider API surface area, but it's required to get the Group block to "group" other blocks.

As I see it the two APIs could be complementary with convert() being the more advanced form.

[gziolo]

@getdave, I would double-check with @mtias, @youknowriad, or @mcsf first as they might have a better overview of why this PR never landed. If you think it's still necessary then it would be fantastic if you could take over it.

[getdave]

I'll bring this up in Core Editor chat on Wednesday.

[gwwar]

+1 for stabilizing this, though I'd also recommend adding/testing more multi-select block conversions in the core block library to test the API before it's finalized. For example, a selection of an image and a paragraph selection -> media and text transform.

I do think the multi-selection wildcard transform needs additional iteration but that can potentially wait for follow up. There's a couple of common cases where a container block may want to be able to group arbitrary blocks, but not nest itself.

[Mamaduka]

I also want to recommend adding wildcard support for transforms.to. It can be useful for conversations like Media & Text > Media + Paragraph (#16516), Cover -> Image/Video + Paragraph, etc.

Currently, both APIs can handle this. The only thing that is missing is the BlockTransformationsMenu item for the wildcard.

Group block already does this by adding a custom menu item in the "Options" dropdown.

[Mamaduka]

I think we should also pass more than just an array of attributes to the isMatch method when isMultiBlock is true. It would make, granular transformations as @gwwar suggested, much more manageable.

[mtias]

I'm not very comfortable with the burden of having both transform and convert explained with the friction of telling people to start using a differently named API for the same conceptual operation.

[getdave]

As there's been a few requests in Core Editor chat, I'm going to try and move this forward.

Firstly, a quick summary of what this is all about.

A Quick Summary

What we're discussing is the argument signature of the respective "transformation" methods:

1. transform - the original method (currently the primary API):

gutenberg/packages/blocks/src/api/factory.js

Lines 505 to 508 in 17fd83b

	transformationResults = transformation.transform(
	blocksArray.map( ( currentBlock ) => currentBlock.attributes ),
	blocksArray.map( ( currentBlock ) => currentBlock.innerBlocks )
	);

2. __experimentalConvert - the "new(er)" method (what this PR proposes to rename to convert):

gutenberg/packages/blocks/src/api/factory.js

Lines 501 to 503 in 17fd83b

	transformationResults = transformation.__experimentalConvert(
	blocksArray
	);

The reason convert was required is that transform doesn't provide sufficient detail about the blocks being transformed to allow for the most complex transform use cases (for example - that required by the Group block).

This is because transform only provides the following data about the block(s):

• block.attributes
• block.innerBlocks

In contrast, convert passes the entire block definition. This is much more powerful and provides access to all/any data about the block(s) you might need (including attributes and innerBlocks).

Suggested options to move forward

There doesn't seem to be a clear consensus on the way forward otherwise this PR wouldn't have been around for so long (indeed it's over 2 years since I introduced the convert API in experimental form) 😄 .

Above, @mtias has noted:

I'm not very comfortable with the burden of having both transform and convert explained with the friction of telling people to start using a differently named API for the same conceptual operation.

That seems to suggest we need to land on having a single primary API for this "transformation" operation. That makes sense as it would be unclear for consumers to have two top-level APIs for the same operation.

Therefore I am going to propose we choose from one of the following two options:

Option 1 - deprecate transform

Formally deprecate transform, promote convert to be the primary API, and then allow folks to move over to convert (we'd display a "deprecation" warning in the console to encourage this) in their own time.

For most blocks, updating to convert should require nothing more than the following change within the block transform handling:

- transform: ( attributes, innerBlocks  ) => {
+ transform: ( { attributes, innerBlocks } ) => {
    // do transform
}

Option 2 - augment transform's arg signature

Augment the argument signature of the existing transform method so that it also provides the full block definition as a 3rd argument. Remove all usage of convert in Core (giving time for 3rd party developers to catch up before it's removed) and utilise the 2nd argument of transform instead.

Here's how that might look at an API level:

transformationResults = transformation.transform(
  blocksArray.map((currentBlock) => currentBlock.attributes), // arg #1
  blocksArray.map((currentBlock) => currentBlock.innerBlocks), // arg #2
  blocksArray, // arg #3: here we pass all the blocks just like `convert`
);

...and here's how it would look like in a transform definition:

transforms: {
    from: [
        {
            type: 'block',
            blocks: [ '*' ],
            isMultiBlock: true,
            transform( attributes, innerBlocks, blocks ) { // note the 3rd argument here
                const groupInnerBlocks = blocks.map(
                    ( { name, attributes, innerBlocks } ) => {
                        return wp.blocks.createBlock(
                            name,
                            attributes,
                            innerBlocks
                        );
                    }
                );

                return wp.blocks.createBlock(
                    'test/alternative-group-block',
                    {},
                    groupInnerBlocks
                );
            },
        },
    ],
},

One clear benefit of this approach is that it requires little or no changes to any existing transform definitions. Only if the developer has used the __experimentalConvert API will they need to update their transform.

Other options?

If folks have any other suggestions then please note them here.

Indicate your preference

I'd be interested to hear from folks about which option they'd prefer out of #1 and #2 above.

It would also be really great to hear again from @youknowriad, @mtias and indeed anyone else from the Core Gutenberg team so that we can get an architectural decision.

To be clear, I believe this API change isn't something we can avoid. It's in use today in Core and in the wider ecosystem. Therefore it is incumbent upon the Gutenberg project to come to a decision about if/how we are going to handle it. I'm looking forward to us reaching a consensus.

Many thanks 🙇‍♂️

[talldan]

I'd vote for deprecating transform. Looking at it, the API seems to have some problems. Especially the second parameter, which for a multiple block transform is blocksArray.map( ( currentBlock ) => currentBlock.innerBlocks ).

It seems weird to receive an array of arrays of inner blocks like that, and there's no indication of what the wrapping block is for each array, just some attributes that are in a separate parameter. It is possible to specify multiple block types in a multi-block transform (via the wildcard or by specifying more than one block type in the blocks array), so this seems like a shortcoming.

Plus, i'm assuming the innerBlocks has the full block object, so that's another inconsistency.

If folks have any other suggestions then please note them here.

The only alternative I can think of is to add a property to the transform to opt in to a different transform function signature. Something like isFullBlockTransform: true. But I'd still vote for deprecating transform and moving on with life.

[youknowriad]

@talldan's thoughts make sense to me, I'm also in favor of the more sane "convert" API. I do have some concerns about how widespread this deprecation could be though but maybe it's fine since it's just a single warning.

[getdave]

This was brought up in the Core Editor chat in Slack so I'll wait a couple of days for any further opinions before outlining a plan to move this forward.

[talldan]

I wonder if rather than moving core blocks over to convert and applying the deprecation in one PR, we could migrate a few blocks at a time to __experimentalConvert in batches first. The eventual switch over then becomes a simple rename of the function.

While the changes are fairly simple I personally find fewer changes makes it easier to be more thorough in a review. It'll also result in fewer conflicts.

[mcsf]

Thanks for the recap and your persistence, @getdave.

To the question of which API suits us best, IMO that's certainly convert. It's the transition I'm concerned with.

On one hand, block transforms or transformations have become established terminology in Gutenberg. On the other, it's not easy to communicate a switch from an API (transforms) to a new API that means roughly-but-not-exactly the same (convert). This is because, in reality, we have in our hands a compatibility-breaking version change of an API disguised as a renaming. In that sense, Option 2 is an easier proposition for API consumers.

As hinted in the above paragraph, part of the issue is that we don't have versioning built into these APIs (as with most non-REST APIs of Gutenberg, with the notable exceptions of block.json and theme.json). That is something hard to solve without harming the developer experience of API consumers.

(For a moment, I considered a different proposal that tried to marry Options 1 and 2: what if we use function arity as a signal? A transform function f with f.length === 1 would stand for a convert-type consumer, whereas a function with f.length === 2 would stand for a legacy transform-type consumer. The biggest drawback would be its implicitness. That alone should raise eyebrows, but I think the deal breaker is that there may be consumers out there which only look at attributes, so arity is a poor signal.)

So I'm not sure what the exit is.

I'm not very comfortable with the burden of having both transform and convert explained with the friction of telling people to start using a differently named API for the same conceptual operation.

Maybe we just bite the bullet and effectively deprecate transform with a clear notice on how to switch to convert. We are not having to explain two different but similar APIs: there is only one public API and one deprecated API.

[peterwilsoncc]

This is in the stable product a resolution of #40316 will be needed prior to merge.

[noisysocks]

Augment the argument signature of the existing transform method so that it also provides the full block definition as a 3rd argument.

This idea isn't as bad as it first seems when you consider that render_callback also receives render_callback( $attributes, $content, $block ).