Add register_block_extension method#80
Conversation
Per PR review, if it works for all blocks, the core_ prefix is unneeded and could be misleading
… array If we call this one at a time, we can immediately try to apply the override when called if wp_loaded has run It's also conceptually simpler to have all the methods work on the level of a single block rather than an array, and the impact of N hook callbacks for N custom block overrides is minimal
This MOST-reliably works on Init, still, but depending on usage, after the prior change to run the overrides one blocok at a time, it is now possible to call this up until enqueue_block_assets and still get the desired behavior.
goldenapples
left a comment
There was a problem hiding this comment.
I like this a lot and seeing how it's used in a theme makes the conventions in the API a lot more clear to me.
Approved as is, with a couple suggestions for documentation and additional functionality.
The one piece of functionality I don't see here that I think would be especially helpful is allowing extending the "render" attribute. Is there any reason that's not included here? I know you can do that with a "render_block' filter, but since we're allowing overriding everything else about the block, I would expect to be able to override the PHP render callback in the same place.
| return; | ||
| } | ||
|
|
||
| $asset_fields = [ 'editorScript', 'script', 'viewScript', 'editorStyle', 'style', 'viewStyle' ]; |
There was a problem hiding this comment.
Is it worth trying to support script modules, ie "viewScriptModule" here as well? I'm not sure if that adds complexity, but since it's supported in block.json I could see wanting to use it here.
There was a problem hiding this comment.
Good suggestion, I've opened #81 to track adding that!
| if ( did_action( 'wp_loaded' ) ) { | ||
| Utilities\apply_block_extension( $target_block, $block_config ); | ||
| } else { | ||
| add_action( 'wp_loaded', function () use ( $target_block, $block_config ) { |
There was a problem hiding this comment.
Just out of curiosity, why are you calling this after the blocks are registered rather than registering a "register_block_type_args" filter on the block itself? Is it just to allow these block extensions to be late-registered?
A filter hook that fires when registering the block would give your extension access to the core block properties, which might be useful in some contexts.
There was a problem hiding this comment.
It was to allow flexibility about when we register our extensions, yes. Since it also could be used with third-party blocks, I was also thinking this gives us more leeway versus requiring it to be run on init and before core registers its own blocks, without worrying about priority.
I'd welcome a counterproposal / follow up PR if you have a better idea!
There was a problem hiding this comment.
I guess the counterproposal is to register a filter on "register_block_type_args" so that you can handle all the properties in block.json.
What if you wanted to filter the core/paragraph block to add "ancestor": [ "core/column" ], or override the "title" text for a core block?
I guess as I'm talking that through, that it goes beyond the functionality I'd expect in an asset loader plugin, so maybe I've talked myself out of it.
|
@goldenapples on extending the render property the render_block filter is a little different to what the render PHP file does so it could be confusing. You make it so the override receives the same arguments but would you expect to filter the block or replace the render function completely? I would consider having render there to wholesale replace the default render, and a custom render_filter property could be added that receives the fully formed block html like you get with the filter. |
Yeah, what I had in mind was being able to override the core render function altogether by doing something like |
Note
Recreated from #79 after I made a mistake with branch naming.
This implements the concept behind a piece of wp-scripts-asset-loader without being as prescriptive about file organization as that plugin.
If you pass a block.json specifying a
namehandle (e.g. core/paragraph) and one or more script or style references, like"style": "file:./style.css"to the newregister_core_block_extensionmethod, it will merge this into the core paragraph's dependencies and enqueue that style on output.Open question: Looking for feedback on the point at which we recommend calling this method, and the hook on which we step in and modify the core scripts. It works most-reliably on init (when core registers its blocks), but can technically be run any time before core processes and outputs its own block scripts.