Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explore storing a JSON representation of menus in the CPT #35868

Closed
Tracked by #35521
draganescu opened this issue Oct 22, 2021 · 13 comments
Closed
Tracked by #35521

Explore storing a JSON representation of menus in the CPT #35868

draganescu opened this issue Oct 22, 2021 · 13 comments
Labels
[Block] Navigation Affects the Navigation Block [Type] Enhancement A suggestion for improvement.

Comments

@draganescu
Copy link
Contributor

draganescu commented Oct 22, 2021

Following up from the exploration on making the navigation block rely on reusable data (#35746), compatibility with classic menus and data interoperability seem like obvious next steps to explore.

Storing navigation as blocks in the wp_navigation post type may be challenging for:

  • using menus to create site maps
  • serving menus via API endpoints
  • easily migrating from classic menus to block menus

... and this is mostly b/c serveside conversion of blocks to JSON may be unreliable at the moment (based on heresay). But even if it is 100% reliable, to convert a classic menu we'd need to create a JSON representation of it, then convert it to blocks, then store it in the CPT. If our common storage format is JSON we remove some surface in this process.

The JSON storage might also help with theme switching mechanisms to be explored in #35750.

So let's find out if:

  • storing blocks as JSON is something which makes migration easier and also makes menus more interoperable as data
  • converting classic menus to JSON is safer than converting to blocks
  • JSON storage allows us to always create/destroy blocks in the browser thus avoiding the need for PHP to do block serialising and hitting its limitations.
@draganescu draganescu added [Block] Navigation Affects the Navigation Block [Type] Enhancement A suggestion for improvement. labels Oct 22, 2021
@adamziel
Copy link
Contributor

adamziel commented Oct 25, 2021

My biggest question here is this: Is it possible to write two functions menu_json_to_blocks( $json ) and menu_blocks_to_json( $blocks ) that will always work regardless of the theme or anything else?

If yes, then the two formats are interchangeable and using JSON for storage would be just a matter of convenience.

If not, then things are getting interesting. I'm not sure about the implications yet, I'll start exploring and post my findings here.

@adamziel
Copy link
Contributor

adamziel commented Oct 25, 2021

Blocks can already be represented equally well as either HTML or JSON.

The following HTML:

$block_html = '<!-- wp:archives {"displayAsDropdown":false,"showPostCounts":false} /-->';

Describes the same block as the following JSON:

$block_json = json_decode('{
    "blockName": "core/archives",
    "attrs": {
        "displayAsDropdown": false,
        "showPostCounts": false
    },
    "innerHTML": "",
    "innerContent": []
}', true);

In this way, JSON and HTML are just different, completely interchangeable expressions of the same data. Like JSON and XML. There are even existing functions to go from one to the other: parse_blocks goes from HTML string to a JSON-serializable array, and serialize_block goes the other way. These identities should, more-or-less, hold:

serialize_block( $block_json ) === $block_html;
parse_blocks( $block_html ) === $block_json;

parse_blocks( serialize_block( $block_json ) ) === $block_json;
serialize_blocks( parse_blocks( $block_html )[0] ) === $block_html;

As for the specific questions in this issue:

JSON storage allows us to always create/destroy blocks in the browser thus avoiding the need for PHP to do block serialising and hitting its limitations.

Javascript can handle both equally well:

  • JSON: Blocks are already stored in a JSON-serializable format in Redux state
  • HTML: In Javascript we have parse() that goes from HTML to JSON, and parseRawBlock() that goes from JSON (WPRawBlock) to block (WPBlock). There is also a serialize() function that goes from JSON to HTML.

converting classic menus to JSON is safer than converting to blocks

Based on the above, I think it's equally safe because that's just a different syntax for expressing the same data.

storing blocks as JSON is something which makes migration easier and also makes menus more interoperable as data

It could be more interoperable for folks querying the database directly. For plugin authors, JSON is just a single parse_blocks() call away in PHP and parse() call in JS.

But even if it is 100% reliable, to convert a classic menu we'd need to create a JSON representation of it, then convert it to blocks, then store it in the CPT. If our common storage format is JSON we remove some surface in this process.

I think of it like this: We start with a classic menu stored in a database, magic happens, and then we have a Gutenberg instance displaying a block representation of the menu. This tells me that converting the data to blocks can't be avoided. It can only be moved from the server to the browser.

cc @kevin940726 @draganescu @mtias @talldan @noisysocks

@adamziel
Copy link
Contributor

adamziel commented Oct 25, 2021

At the risk of stating the obvious: I was briefly thinking about a block-less JSON representation like this:

[
   { "title": "Homepage", "url": "/" },
   // ...
]

But the obvious problem is that it focuses on the URLs and lacks blocks. Moving away from the blocks JSON means moving away from search fields, configurable attributes, and inner blocks. If we want to have blocks, we must embrace the "blockful" data format.

@talldan
Copy link
Contributor

talldan commented Oct 26, 2021

I think the two strongly related issues to this are #30674 and #35750, so worth always keeping those in mind

In this way, JSON and HTML are just different, completely interchangeable expressions of the same dat. Like JSON and XML. There are even existing functions to go from one to the other: parse_blocks goes from HTML string to a JSON-serializable array, and render_block goes the other way. These identities should, more-or-less, hold

I think this is the important part.

An advantage to JSON is that it has equal standing on both client (JavaScript) and server (PHP), both can read or write to it. But I don't think the same assertation about block HTML is true, PHP code can't properly write blocks (particularly static blocks because it'd need to be able to execute the JS save function). My understanding is that serialize_block and render_block rely on the HTML written by the client-side code. I could be wrong though, it wouldn't be the first time!

For data migrations when theme switching, not being able to write data could be problematic. There's a chance the theme switch from classic to block-based could require a PHP migration (after_switch_theme), and so such code should perhaps be able to write to a wp_navigation post type. The user might never visit the site editor after switching theme so a client-side JS migration might not be realistic.

For basic links, it's probably not a huge stretch to write blocks, because they don't actually output any HTML, but I don't know that it'd be the most adaptable system for the future.

@talldan
Copy link
Contributor

talldan commented Oct 26, 2021

The good aspect at least is that PHP can read block HTML and convert it to JSON, so potentially there's little risk in starting with block HTML. It can be migrated to JSON later if needed.

@adamziel
Copy link
Contributor

adamziel commented Oct 26, 2021

@talldan brought a good point that there are two different HTML representation of block data. The first one looks like this:

<!-- wp:paragraph {"content":"Hello", "align":"right"} /-->

I think of it as of an excerpt from the Redux state, only encoded as HTML. In the JSON form it would look like this: { "blockName": "core/paragraph", "attrs": {"content":"Hello", "align":"right"}, "innerBlocks": [] }.

This snippet can't be rendered by PHP. Why? The code that transforms redux state into HTML markup lives in the block's JS save() function, at least for the static blocks. The output of the JS save() function looks like this:

<!-- wp:paragraph {"align":"right"} -->
<p class="has-text-align-right">Hello</p>
<!-- /wp:paragraph -->

I'll call it Rendered Markup. PHP can easily, well, render it. How do I know? Because block data is stored like that in wp_posts. Rendered Markup can be converted into JSON using parse_blocks:

{
	"blockName": "core/paragraph",
	"attrs": {
		"align": "right"
	},
	"innerBlocks": [],
	"innerHTML": "\n<p class=\"has-text-align-right\">Hello</p>\n",
	"innerContent": [
		"\n<p class=\"has-text-align-right\">Hello</p>\n"
	]
}

This brings me to a conclusion: For JS/PHP interoperability, Gutenberg needs to store this fully rendered data. It can be represented as Rendered Markup, JSON, XML, YAML, or anything else. Braces or tags, ultimately these are just different ways of expressing the exact same information. Just as both JS and PHP can parse JSON, they can parse Rendered Markup. I think JSON will be more convenient for folks consuming the navigation data from e.g. python, but I don't think it would make implementing the navigation block any easier.

As @talldan noticed:

Migrating classic menu data will probably only require writing navigation submenu and navigation link blocks (initially), which PHP can already write as block markup.

If JSON ends up being the format of choice here, the implementation could potentially be as easy as implementing a "data transformer" pattern, which essentially means adding a few extra parse_blocks(), serialize_block(), parse(), and serialize() calls in the API endpoints and JS files. cc @draganescu @mtias @spacedmonkey

@noisysocks
Copy link
Member

The good aspect at least is that PHP can read block HTML and convert it to JSON, so potentially there's little risk in starting with block HTML. It can be migrated to JSON later if needed.

Agree with this. It's safest I think to have wp_navigation align very closely to how the post, wp_block and wp_template CPTs work. They all store block-enhanced HTML. We can change this later if our requirements change by writing a database migration.

@talldan
Copy link
Contributor

talldan commented Oct 27, 2021

I think the other point is that Navigation Links and Submenus are fully dynamic, they're represented only as a HTML comment, so it seems like we can parse and serialize that from PHP ok. Is that right? For migrating classic menu data (#35750), there don't seem to be any blockers that would prevent us from continuing to use using block html in a wp_navigation. And it has been established that a migration can happen later if a requirement arises that can't be achieved with Block HTML.

Given that, I think it's fine to park this issue and not worry about it for 5.9

@noisysocks
Copy link
Member

I think the other point is that Navigation Links and Submenus are fully dynamic, they're represented only as a HTML comment, so it seems like we can parse and serialize that from PHP ok. Is that right?

Yes that's right. If a block attribute doesn't have a source then it will be saved as JSON into the HTML comment and PHP can parse and serialise it just fine. If a block attribute has a source then we need save() in order to turn the attribute into HTML and hpq to turn the HTML into an attribute.

Given that, I think it's fine to park this issue and not worry about it for 5.9

I agree.

@getdave
Copy link
Contributor

getdave commented Jan 27, 2022

Are we still interested in exploring this or does the CPT work well as it is? I'm curious as to whether we need to decouple the data any further than we have now?

@draganescu
Copy link
Contributor Author

@getdave so far so good. We can even close this issue IMO. Now that this is in 5.9 and the release is out there we'll see what limitations (if any) we hear of through feedback. Also we may come back to this in the future if the improvements to block navigation hit some limitation.

@getdave
Copy link
Contributor

getdave commented Jan 31, 2022

Thanks Andrei. I do wonder whether we will end up revisiting this if/when we need a representation of the menu data that is decoupled from blocks. However, it does feel difficult to anticipate the need for this at this point.

I'll close this out and we can always reopen if/when we receive new user-driven requirements.

@getdave getdave closed this as completed Jan 31, 2022
@noisysocks
Copy link
Member

If an idea is truly worth doing then it will come up again 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Affects the Navigation Block [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

5 participants