Enable filter hooks for convertLegacyBlockNameAndAttributes

[ddryo]

What? & Why?

Enable filter hooks for convertLegacyBlockNameAndAttributes .

While convertLegacyBlocks only has the core blocks available.

This PR allows plugin creators to change their block names.

For example, the following code would change my-plugin/example-block to my-addon/example-block.

addFilter(
	'editor.convertLegacyBlockNameAndAttributes',
	'my-plugin/filter-convertLegacyBlockNameAndAttributes',
	( blockData ) => {
		const name = blockData[ 0 ];
		const attrs = blockData[ 1 ];

		if ( 'my-plugin/example-block' === name ) {
			return [ 'my-addon/example-block', attrs ];
		}
		return [ name, attrs ];
	}
);

This can solve problems such as #17372.

[Mamaduka]

Thanks for contributing, @ddryo.

We're trying to avoid introducing new JS hooks in the Gutenberg plugin whenever possible. See #35757 (comment).

You can also find a general discussion for extensibility here - #37448.

[ddryo]

So what if I want to do the same thing with custom blocks as with convertLegacyBlocks?

Just having hooks here can greatly reduce maintenance costs for many developers.

I think the necessary hooks should be implemented.

[ddryo]

One of the serious problems with publishing custom blocks as plugins, etc., is the high development costs involved in changing functionality.

It is impossible to design a perfect design from the start so that there are no changes to the specification at all.

This is because the Gutenberg core is constantly evolving and changing.

Important components are changed, deprecated, or new and useful components and APIs are created.

This hook can be useful when trying to keep up with changes in the core and adjust blocks developed in the past to make them even better.

Of course, in many cases, the deprecated option will suffice.
However, I think this hook can be a lifesaver when that is not possible.

I also expect that there will be many cases where it will be easier to update the attributes with this hook than to write deprecated, and the adjustments can be made with shorter code.

Translated with www.DeepL.com/Translator (free version)

[dmsnell]

@Mamaduka is that project policy? I'm definitely in favor of adding new hooks to make things more extensible even if they are implicit. WordPress itself is fairly implicit and while ugly that has been one of its strength IMO.

This seems like a valuable solution solving a known problem; did you have an alternate idea for implementation?

[Mamaduka]

@dmsnell, this PR mentions some parts of the comments I linked above - #30963, but it was never finalized.

Per my experience, filters become problematic when they change how UI is rendered. However, I don't think this proposal falls into that category.

[dmsnell]

Hm. I'm not seeing what you are seeing in either of the linked issues/PRs. This proposal is so far away from the kind of thing I think @nerrad was talking about too that I don't see it applying (correct me if I'm wrong please). We've made intentional choices to extend flows over location, but there's a lot of data-side extensibility we rely on which is very technical in nature. Renaming a block seems like one of them since that's not possible through the block deprecation process.

We might argue that we should make block deprecation support renaming, but that seems like opening a can of worms to avoid something that's otherwise normal and reasonable.

[Mamaduka]

I just want to be clear that I've nothing against this solution.

@dmsnell, I was under the impression that introducing new hooks were discouraged in the project, but I probably miss interpreted @nerrad's and @youknowriad's comments 😅

Side note: I think it would be nice to have the project's extensibility policy/approach documented somewhere.

[gziolo]

The challenge with introducing a new filter is that the internal function, like convertLegacyBlocks here, becomes part of the public API.

We might argue that we should make block deprecation support renaming, but that seems like opening a can of worms to avoid something that's otherwise normal and reasonable.

We discussed that in the past, and the outcome was that it isn't a common use case to rename the block so we rather keep it limited for core blocks to ensure backward compatibility is kept. However, the proposal to make it filterable sounds reasonable if that would be useful also for other blocks.

[gziolo]

Can we document the filter in the code with JSDoc? For reference, this is how filters get documented in WordPress core:

https://github.com/WordPress/wordpress-develop/blob/e94cd298c6e0208a5feb888099601feadf460eda/src/wp-includes/blocks.php#L262-L269

We should also include a new section in this document:

https://github.com/WordPress/gutenberg/blob/trunk/docs/reference-guides/filters/block-filters.md#block-editor

[Mamaduka]

That would be great. Cc @ryanwelcher.

[retrofox]

hi @ddryo 👋 I'm looking forward to seeing these changes merged, too <3 Are you going to continue with the remaining issues? could I help here?

[retrofox]

hi @ddryo it's me again 😅
I've created a new PR with the same changes you suggest here. Even more, I've cherry-picked up your commits.

I'd do want to merge your PR rather than mine. But also, I'd like to move on with other stuff that is based on this hook. So let's wait a little bit more until asking dev folks to start to consider using this alternative PR.
BTW, I've pushed some commits that document the hook. Feel free to cherry-pick them.

[ddryo]

Sorry, the negative comments came first, so I gave up thinking it was closed and didn't check for a long time.

Is this PR still alive?
Should I continue working on something?

[retrofox]

Sorry, the negative comments came first, so I gave up thinking it was closed and didn't check for a long time.

Is this PR still alive? Should I continue working on something?

I'm not sure, to be honest. I think it's worth it but we should as to core folks.

[mcsf]

I understand the motivation for this change (and thank you for the diff), but I'm apprehensive about it. Indeed, blocks evolve over time, and the Block API accounts for this via validation, attribute migration, etc. However, all this takes place within the relative safety of each block type, which acts both as a namespace and as an error boundary.

The underlying assumption here is that block names are stable. Yes, there have been times over the years when Core needed to rename some its own blocks, but I guess we've lived and learned over five and a half years. The fact that convertLegacyBlockNameAndAttributes is a single-pass function with no hooks reflects the fragility of renaming blocks. I'm concerned that, in opening that up, race conditions and other undefined behaviours would arise when third parties hook into it.

I'd encourage contributors to:

• make the case for block name switches
• explore alternative switching mechanisms in safer environments, e.g.
  • in the editor, block A asks the user to be upgraded to block B by the press of a button
  • in the front end, block A's render_callback adapts to produce a result similar to block B

[priethor]

Let's close this PR, as it needs more info to be actionable. Please feel free to reopen again if there is more information. 🙏