Update docs to reflect new preferred convert method on transform objects

[getdave]

A new convert method will shortly become the preferred way to define the way in which one (or more blocks) can be transformed into one (or more) other blocks using the Block Transforms mechanic.

Currently developers define a transform method on the transform object definition in which they defined how to transform the block.

transforms: {
	from: [ {
		type: 'block',
		blocks: ['*'],
		transform: (attributes, innerBlocks) => {
			// do the transforming!
		}),
	} ],
},

The issue is that this function is provided with only 2 arguments

1. Block attributes
2. Block innerBlocks

However, there are many cases where having access to the entire Block object is required (eg: for example see the Grouping mechanic).

After a lengthy discussion in Slack and having reviewed several options, it was decided to create a new convert method which would accept the full block object.

transforms: {
	from: [ {
		type: 'block',
		blocks: ['*'],
		convert: (blocks) => {
			// do the transforming with all the block data!
		}),
	} ],
},

When the transform is applied this convert method is always run in preference over any transform method. However, if no convert method is defined then the transform method will be attempted. As a result it is fully backwards compatible.

transforms: {
	from: [ {
		type: 'block',
		blocks: ['*'],
		convert: (blocks) => {
			// this will run in preference to the transform method below
		}),
		transform: (attributes, innerBlocks) => {
			// this will NOT be run as convert takes precedence
		}),
	} ],
},

We now need to fully document this change. This involves:

• Updating the Transforms API documentation
• Some other tasks...

See also #14908

[getdave]

@youknowriad @aduth Did we decide to stick with this naming?

Either way is this docs task required for the release (which I think is coming up soon right?).

[aduth]

@youknowriad @aduth Did we decide to stick with this naming?

I don't personally take strong issue with it. I know @nerrad had proposed an alternative applyTransform which is clear and respects the act of the application of a transform, but I find to be both verbose and somewhat redundant (with the transforms top-level property, not that the previous transform function wasn't already).

I think I'll be fine to see it go through as-is. We could flag it as "experimental" if we wanted some more time to consider it. But just as well, there is still an option to deprecate before the next WordPress stable release if a viable alternative presents itself.

Either way is this docs task required for the release (which I think is coming up soon right?).

No, I don't think it's required.

[aduth]

After talking a bit more with @nerrad , I think we may want to just mark this as experimental for now. There were a few other approaches we discussed, and in general I don't think we seem all that comfortable to commit to the current API as proposed. Since it's not documented anyways, we can plan to stabilize the API in tandem with introducing the documentation.

A few of the additional considerations:

• We could overload to and from to act as the complement to whichever is defined as the source array of block types.
  • For example, { from: [ 'core/paragraph' ], to( blocks ) { /* ... */ } } or the inverse { to: [ 'core/paragraph' ], from( blocks ) { /* ... */ } }
• To better clarify the expected return value, some more verbose alternative to describe the act of the transformation (e.g. getTransformedBlocks)

Personally, I also sought to look up additional options for words, closer to the intention with the "apply" as describe the act of applying a transform. None of which really strike me more than what's already been considered.

• execute
• run
• transact
• change
• act
• operate
• fulfill

I've also considered completely different options for defining the transform, again not striking me as particularly viable or an improvement.

• A transform as a tuple of configuration and the function to carry it out (anonymous function)
• A transform as itself a function, either defining characteristics as properties of the function or in the return value. Possibly that all transform functions for a block type are run on any transform attempt, and a return value exists only if the block could carry it out.
• Revisiting the idea of the transform generator function idea as originally proposed in the discussion at Enables Block Grouping/UnGrouping using core/group #14908 (comment)

As an immediate action item, I will follow-up with a pull request to mark this function as experimental (Edit: See #16047).

[nerrad]

Thanks for summarizing this @aduth!

[getdave]

@aduth When might be remove __exeperimental? Is there a process for this?

[getdave]

@youknowriad @aduth Sorry to chase, but just wondering if it's ok to make this a first-class citizen now? I noticed this effort to audit experimental APIs so perhaps this will be taken care of without me needing to keep 👁 👁 on it?

[aduth]

@getdave Sorry that I'd missed your October ping, as it happened to fall within a window of my extended leave from the project. Coincidentally, I'd been thinking about this since it had been brought up in Slack as something to stabilize. Much of this prior discussion had slipped my memory, and I don't personally take any issue with stabilizing the function as it exists today.

I can plan to take care of it, unless you had already been thinking about it?

[aduth]

Looking at the documentation:

https://github.com/WordPress/gutenberg/blob/master/docs/designers-developers/developers/block-api/block-registration.md#transforms-optional

There are two things which stand out to me:

• Based on the previously-linked Slack discussion, we might want to consider the impact this has on the shortcode transform API, which is distinct from the block transform process, but adheres more to the API of the transform function (receives attributes as an argument). I suppose it depends whether we'd ever care to support the use-case of converting multiple shortcodes to one or more equivalent blocks (probably not), but in any case, there's now a bit more of a divergence in how the two are used.
• The usability of convert in transforming multiple blocks (always), vs. the previous usage of assuming to operate on a single block. This was obviously an intentional part of adding support for the multiple block conversion, but I consider that it could make the previous usage a bit more cumbersome, if the transform did happen to be a one-to-one transformation. Ultimately, this would likely amount to an Array#map on the return value, but I don't know that this feels quite as nice as the previous behavior.

One idea which I don't know had been considered previously: What if we continue to support transform, and name the new function transformMany? Or some equivalent naming which implies that the intended usage is to convert multiple blocks. In this line of thinking, we don't need to advocate one or the other as being preferred, but each as equally sensible under certain conditions.

There's still a problem in that one form receives the full block objects (transformMany( blocks )) while the other only receives a subset of the block detail (transform( attributes, innerBlocks )). Again, this is where it might have just been preferable to have passed the block object to transform in the first place, but it's not something we can really change at this point in time.

What we could do, though, if we want the behavior to align, is to proceed with the renaming and soft-deprecation of transform, while still keeping the singular vs. many forms as two separate options.

Something like:

• convert( block: WPBlock ): WPBlock|WPBlock[]
• convertMany( blocks: WPBlock[] ): WPBlock|WPBlock[]

The reason I think this matters is that, anecdotally, I suspect that the majority of usage will only care to apply a consistent transformation on singular blocks. The multi-block transforms (like for the Group block) seem to be more of the edge-case, so it doesn't really make sense to me to optimize for this.

[aduth]

Looking again at the implementation, there's already an (undocumented) expectation that multi-block transformations should provide a isMultiBlock: true value as part of the transformation descriptor. Indeed, this is what the group block already provides to ensure that the conversion function receives an array of blocks.

Thus, I don't think we need to have separate functions, we can just name the function convert, and if the transformation also assigns isMultiBlock: true, then it will receive an argument as an array of blocks, otherwise it will receive as a single block.

[aduth]

• Based on the previously-linked Slack discussion, we might want to consider the impact this has on the shortcode transform API, which is distinct from the block transform process, but adheres more to the API of the transform function (receives attributes as an argument). I suppose it depends whether we'd ever care to support the use-case of converting multiple shortcodes to one or more equivalent blocks (probably not), but in any case, there's now a bit more of a divergence in how the two are used.

Related to this, and probably more pressing for consideration: There are special kinds of transformation "types" which are handled outside the standard block transform API. These also use the transform naming, presumably as an overload of the block transformation transform function. I think for consistency's sake, we'd want to update these as well to prefer the consistent convert naming.

See also:

• 'prefix' and 'files' type transforms: https://github.com/WordPress/gutenberg/blob/master/docs/designers-developers/developers/block-api/block-registration.md#transforms-optional
• Handlers:

  • gutenberg/packages/block-editor/src/components/block-drop-zone/index.js

    Lines 60 to 74 in 772a5d5

    	onFilesDrop( files, position ) {
    	if ( ! this.props.hasUploadPermissions ) {
    	return;
    	}
    	const transformation = findTransform(
    	getBlockTransforms( 'from' ),
    	( transform ) => transform.type === 'files' && transform.isMatch( files )
    	);
    	if ( transformation ) {
    	const insertIndex = this.getInsertIndex( position );
    	const blocks = transformation.transform( files, this.props.updateBlockAttributes );
    	this.props.insertBlocks( blocks, insertIndex );
    	}
    	}

  • gutenberg/packages/block-editor/src/components/rich-text/index.js

    Lines 282 to 286 in 772a5d5

    	const prefixTransforms = getBlockTransforms( 'from' )
    	.filter( ( { type } ) => type === 'prefix' );
    	const transformation = findTransform( prefixTransforms, ( { prefix } ) => {
    	return trimmedTextBefore === prefix;
    	} );

This late observation and additional maintenance overhead can serve as good justification that this ought to be refactored to a consolidated approach, which might be worth doing as part of this effort.

[aduth]

I managed to work my way through most of a branch to deprecate transform consistently in favor of convert, but encountered another issue in a potential inconsistency it introduces with the transforms isMatch function. The function receives an attributes object as its argument, much like the transform function. Granted, there's already a bit of inconsistency here in that it does not receive innerBlocks as a second argument like the transform function had. We could decide to be content with this disparity. In retrospect, ideally this would receive a block or blocks object(s) just like an ideal transform function would have.

[aduth]

The pull request at #19401 will stabilize this API. The open questions are still pending on how we address some of the inconsistencies noted in my previous comment.

[oandregal]

Cross-commenting for visibility #19401 (comment)