Transforms: "isMatch" does not work for shortcode transformation

[sayontan]

Describe the bug
The Block API documentation talks about the use of isMatch to filter or specifically target certain blocks or nodes. This however does not seem to work if the transform from is of type: "shortcode".

To Reproduce
Steps to reproduce the behavior:

1. Create a plugin with the following transform:

transforms: {
	from: [
		{
			type: 'shortcode',
			tag: 'gallery',
			isMatch: function(attributes) {
				return !empty(attributes.named.type);
			},
			attributes: {
				shortcode: {
					type: 'string',
					shortcode: function(named) {
						return JSON.stringify(named.named);
					}
				}
			}
		}
	]
},

2. Create some gallery shortcodes in a post in the "Code Editor" mode:

[gallery ids="139,140,141,142,97"]
[gallery photoset_id="12345" type='flickr']

3. In the Visual editor click on "Convert to Blocks"
4. Both, the first and the second shortcode will be transformed to block you are registering

Expected behavior
I would expect the first shortcode to continue to be treated as a regular WordPress gallery, and the second to be treated as a block defined by my plugin. If isMatch is not the right way for this to happen, how would such targeting be done?

Use Case
There are multiple plugins today that use the same shortcode, with the gallery shortcode being one of the more commonly used ones. With the shortcodes there is a way to make them coexist by having specific attributes. Vendors typically use this approach to avoid shortcodes showing up in the front-end. E.g. in the two examples I have provided above the first gallery will render as per the inputs, while the second one will not be visible at all if you don't have the plugin.

When the plugin developer wants to use the Block API they would typically target their specific instances by applying certain match criteria for transformation. Without this capability they will end up inadvertently changing all shortcodes with the same tag to the same block type when the user clicks on "Convert to Blocks" for the first time.

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

• OS: Windows 10
• Browser Chrome
• Version 69.0.3497.100

Additional context

• Gutenberg version 3.90

[sayontan]

@designsimply,
Not sure why this has been labeled "Help Request". This is a bug. If you look at the code to transform shortcodes to blocks it does not honor the isMatch attribute at all. The documentation on the other hand says that isMatch "provides an opportunity to perform additional checks on whether a transform should be possible".

[designsimply]

@sayontan my apologies for not seeing your message sooner. I have updated the labels for you. I think I initially labeled this issue as a help request because you asked, "if isMatch is not the right way for this to happen, how would such targeting be done?" However, sometimes I don't get the labels right on the first try and I'm sorry for the trouble.

[simison]

Just confirmed this bug/missing feature and it's kinda blocker for using conditional transforms, especially for the poor [gallery] shortcode (https://github.com/Automattic/wp-calypso/issues/30723) that gets extended by various different plugins.

Currently, a block with the highest priority attribute wins the transform from all the other plugins.

Suppose these shortcodes and if every plugin implements isMatch, each plugin would only transform its own shortcode:

[gallery ids="1,2"] → core gallery
[gallery ids="1,2" layout="mosaic"] → Jetpack Tiled gallery
[gallery ids="1,2" layout="slideshow"] → Jetpack Slideshow gallery
[gallery] → shortcode block

An alternative approach could be to define specific attributes as required with a possible list of accepted values.

[ryelle]

We're running into the same problem with the WooCommerce Blocks– we have a few different blocks that correspond to a single shortcode with different attributes. For example:

• [products on_sale="true"] should transform to the "On Sale Products" block
• [products best_selling="true"] should transform to the "Best Selling Products" block
• [products ids="1,2,3"] should transform to the "Hand-picked Products" block

And so on (we have 7 blocks that all match different configurations of the products shortcode).

Having an isMatch option with the content/attributes from the shortcode would work & match the other transforms. The "required attributes" idea could work for most scenarios, but we might also want to say "don't transform if the shortcode has X attribute", because our block doesn't support it. (happy to give a few more examples & test things out, our shortcode is complex)

[mcsf]

This is a useful issue to raise and I agree that there should be some sort of equivalent to other transform types' isMatch functionality. However, as far as the specification goes, there are a few unknowns:

• Signature of isMatch, from lowest to highest level:

  • isMatch( htmlFragment: string ) -> Boolean
  • isMatch( shortcodeMatch: object ) -> Boolean
    • or specifically shortcodeMatch.shortcode.attrs
  • isMatch( createdBlock: object ) -> Boolean
    • or specifically createBlock.attributes

• Behaviour in case of a failed isMatch test:

  • Fall back to whichever next block transform matches the shortcode
  • Skip fragment (seems doubtful this one would be preferred, but I thought I'd mention it)

After a quick stab at this, my personal preference goes for the mid-level API, i.e. the result of the shortcode attributes match per the Shortcode API, which also mirrors how attributes are transformed:

{
	type: 'shortcode',
	tag: [ 'my-broccoli' ],
	attributes: {
		id: {
			type: 'number',
			shortcode: ( { named: { id } } ) => parseInt( id, 10 ),
		},
	},
	isMatch( { named: { id } } ) {
		return id < 1000;
	},
},

[mcsf]

See #18459 for a fix.