WordPress.org

Make WordPress Core

Opened 8 weeks ago

Last modified 6 weeks ago

#47375 new enhancement

Blocks API: Add server-side `serialize_block()`

Reported by: dmsnell Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: trunk
Component: Editor Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

We have the ability to parse_blocks() on the server and transform serialized block content in HTML form into a list of block objects or nodes. We lack the ability to transform in the opposite direction.

serialize_block() takes the output from parse_blocks() and returns an HTML string which would produce the same content as at the beginning of the process. in code terms…

<?php
$post_content === implode( "\n\n", array_map( 'serialize_block', parse_blocks( $post_content ) ) );

An initial use-case I have found is the desire to transform the saved contents of $post_content from a block-structural point of view. I want to "extract" the inner contents of a block from its surrounding block comment delimiters without touching the other parts of $post_content.

Another use-case would be any similar structural changes. Consider the case where we want to update a post and remove all align: left attributes from the comment delimiters…

<?php
$blocks = parse_blocks( $post_content );

foreach( $blocks as $block ) {
        if ( isset( $block['attrs']['align'] ) && 'left' === $block['attrs']['align'] ) {
                unset( $block['attrs']['align'] );
        }
}

return implode( "\n\n", array_map( 'serialize_block', $blocks ) );

Essentially this copies existing logic which exists inside the editor into the backend.

Attachments (4)

47375.patch (7.6 KB) - added by dmsnell 8 weeks ago.
Proposed function and associated tests
46199-2.patch (4.6 KB) - added by dmsnell 7 weeks ago.
Handles non-block blocks (fallback blocks) and includes serialize_blocks() function for handling the full list of blocks returned from parse_blocks()
47375-3.patch (8.7 KB) - added by dmsnell 7 weeks ago.
Fix of 47375-2.patch - I messed up when creating and uploading that one
47375-4.diff (11.0 KB) - added by jorgefilipecosta 6 weeks ago.

Download all attachments as: .zip

Change History (13)

@dmsnell
8 weeks ago

Proposed function and associated tests

This ticket was mentioned in Slack in #core-editor by dmsnell. View the logs.


7 weeks ago

#2 @birgire
7 weeks ago

Hi, I wonder if serialize_block() from 47375.patch should make sure the array block keys exists, to avoid PHP notices if they don't exists in the input? It seems the $block['attrs'] and $block['innerContent'] is taken care of, but not $block['blockName'].

Another thing that came to mind is the serialize part of the function's name. When I read the ticket's title, I first thought of the PHP functions: serialize()/unserialize() and the WordPress core functions: maybe_serialize()/is_serialized(), but that's unrelated to this new function. I'm not sure if this is an issue at all, but I just wanted to mention my momentarily confusion with the name :-)

#3 @dmsnell
7 weeks ago

Thanks for your review and feedback, @birgire!

make sure the array block keys exists

good call - I'll make sure this works and add the tests to verify it

the serialize part of the function's name

this comes with a symmetry to the JavaScript function of the same name. these are each one half of the parser/printer pair that establishes our ability to reliably store the data in HTML form.

I think I'd lean towards keeping it for the sake of consistency across the platforms (JS vs. PHP).

@dmsnell
7 weeks ago

Handles non-block blocks (fallback blocks) and includes serialize_blocks() function for handling the full list of blocks returned from parse_blocks()

@dmsnell
7 weeks ago

Fix of 47375-2.patch - I messed up when creating and uploading that one

#4 @dmsnell
6 weeks ago

As I'll be away from this for a few months is there any chance someone could take over championing this ticket? I'd love to see it move into core. @jorgefilipecosta I remember seeing that you had similar work in another part of the plugin? Could you provide some guidance or help here?

#5 @jorgefilipecosta
6 weeks ago

Hi @dmsnell, sure I will review this patch soon. The plugin has a similar function (not as complete) and it would be good to land this in core so we can remove it from the plugin.
cc: @noisysocks as he was the author of the serialize_blocks function available in the core, and may also have some insights.

#6 @jorgefilipecosta
6 weeks ago

  • Keywords has-patch has-unit-tests added
  • Version set to trunk

I made some tests to the function, and it seems to work as expected. I replaced the Gutenberg function with the one implemented here, and the behavior was not changed. The code seems ready!
I applied some minor changes (type fixes, fix phpcs error, added comments required by phpcs, ...), the changes are available in the patch I just submitted.
I guess unless someone has some concern to raise this should be ready. @noisysocks, @pento any thoughts on this?

#7 @birgire
6 weeks ago

good call - I'll make sure this works and add the tests to verify it

Thanks @dmsnell for the update

this comes with a symmetry to the JavaScript function of the same name. these are each one half of the > parser/printer pair that establishes our ability to reliably store the data in HTML form.
I think I'd lean towards keeping it for the sake of consistency across the platforms (JS vs. PHP).

... and for the link and clarification on the choice of the function's name. It makes sense.

What about adding a note to the new functions here, that they are not related to the serialize() function in PHP?

I wondered about the 'innerHTML' part, as it was not in 47375.patch, but I see it's been introduced in 47375-3.patch.

What about invalid $block input of serialize_block( $block ) ?

e.g. if $block is not an array or $block['innerHTML'] array key is missing, then it looks like we would get a PHP notice there.

Same with the null === $block['blockName'] check in 47375-3.patch, seems to assume that the array key exists. It will give a PHP notice if it doesn't. Played with it here https://3v4l.org/5Vjh3

What do you think about making sure the input has the correct structure?

Thanks for the updated patch @jorgefilipecosta.

Here are few suggestions to 47375-4.diff:

  • Missing end of line (dot) in few places (also in test file).
  • I think it would be beneficial to the docs to add a more detailed description (i.e. the array keys blockName, attrs, innerHTML and innerContent) belonging to the block's input: @param array $block The block being rendered.
  • Same for * @param array $blocks The list of parsed block objects.
  • What about adding @covers ::serialize_block() to the test class description, as all current tests are for the serialize_block() function?
  • If added, then I think one could simplify the description of test methods, e.g.: * Test: test_serializes_and_strips_away_core_namespace() should serialize and strip away core namespace. to * The function should serialize and strip away core namespace.


#8 @dmsnell
6 weeks ago

What about adding a note to the new functions here, that they are not related to the serialize() function in PHP?

Admittedly I have mixed feelings but I'm hesitant to add superfluous comments when I think they are incidentally needed. serialize() is a wildly-generic term and I'm not personally familiar with any common trends that create some kind of specialized version of serialize() for specific data types. I won't try and block an additional comment but in some ways I find the extra explanation a little unwarranted.

I wondered about the 'innerHTML' part

That's a shortcut based on the way the blocks are designed. We could have replaced it with (the arguably more reliable) $block['innerContents'][0] but I left it short for clarity. Maybe that's the wrong call - not sure.

What about invalid $block input of serialize_block( $block )
What do you think about making sure the input has the correct structure?

I've intentionally not validated the inputs and while that may sound bizarre I do it for the sake of improving the quality. First of all, I think if we are worried about invalid input we might gain by addressing this from another angle - make it clearer that this should only get the contents of the block structure provided by parse_blocks() or equivalents.

At this low level I consider it a game of tense alliances between all the different subsystems. If any block data is passed around which doesn't conform to our expectations than multiple systems will fail - to that end I'd rather we fail right out front at the boundary then cover the error - it's going to fail either here or right away somewhere else.

That block structure does bring certain assumptions and you're right to call them out. I'm not sure if I'll get this in but some links to documentation/specifications can hopefully help fill in the contextual gap here.

Otherwise awesome feedback @birgire and thanks so much @jorgefilipecosta for helping this along. You are both wonderful!

#9 @birgire
6 weeks ago

@dmsnell thank you for an informative reply.

Admittedly I have mixed feelings but I'm hesitant to add superfluous comments when I think they are
incidentally needed. serialize() is a wildly-generic term and I'm not personally familiar with any
common trends that create some kind of specialized version of serialize() for specific data types. I
won't try and block an additional comment but in some ways I find the extra explanation a little unwarranted.

My initial thought was that this new PHP function was related to the commonly used serialize() function in PHP, but I might be the only one experiencing that initial thought :-) Maybe a general dev-note introducing this new function, might then be more relevant instead?

That's a shortcut based on the way the blocks are designed. We could have replaced it with (the
arguably more reliable) $blockinnerContents?[0] but I left it short for clarity. Maybe that's the
wrong call - not sure.

That sounds good to me.

I've intentionally not validated the inputs and while that may sound bizarre I do it for the sake of
improving the quality. First of all, I think if we are worried about invalid input we might gain by
addressing this from another angle - make it clearer that this should only get the contents of the
block structure provided by parse_blocks() or equivalents.

At this low level I consider it a game of tense alliances between all the different subsystems. If
any block data is passed around which doesn't conform to our expectations than multiple systems will
fail - to that end I'd rather we fail right out front at the boundary then cover the error - it's
going to fail either here or right away somewhere else.

That block structure does bring certain assumptions and you're right to call them out. I'm not sure
if I'll get this in but some links to documentation/specifications can hopefully help fill in the
contextual gap here.

Yes I think it would be helpful to mention the relation to parse_blocks() in the docs.

As this will become a public API (unlike the underscore _ prefixed functions), I can imagine it will be used in all kinds of situations and all kinds of inputs.

I think it could be helpful to validate the input, in such a way that the function will not output a "block zombie", for my lack of a better word :-)

This kind of output might look valid from a distance, but can scare the hell out of us when we look closer :-)

Here's an example:

serialize_block( [ 'blockName' => [] ] )

with output:

"<!-- wp:Array /-->"

I wonder if these kind of outputs should be allowed to go through?

Note: See TracTickets for help on using tickets.