-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Add block data to REST API #2649
base: trunk
Are you sure you want to change the base?
Add block data to REST API #2649
Conversation
@BE-Webdesign thanks for pointing out #2503 - I searched for a similar PR and couldn't find one. As you pointed out, these PRs are pretty different; from a brief view it looks like #2503 is an API for (reusable) block objects themselves, while this PR is focused on exposing a (gutenberg built) post's blocks (and their data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think rather than creating separate endpoints, we should build this into the existing endpoints instead. In particular .content
on the post type is intentionally an object to allow adding extra properties.
(I'll follow this up in a minute; GH's review comment UI isn't great for leaving long comments.)
In the post object, It also seems like (maybe) Gutenberg distinguishes between rendered and raw block content, based on this PR. (I could be wrong there.) Given that, I'd probably add a new sub-field under Something like this: {
"content": {
"raw": "<!-- gutenberg comments --> interspersed with HTML",
"rendered": "<div class='block-foo'><p>bar</p></div><p>interspersed with HTML</p>",
"blocks": [
{
"type": "foo",
"attributes": {
"value": "bar"
},
"rendered": "<p>bar</p>"
},
{
"type": "html",
"attributes": {
"content": "interspersed with HTML"
},
"rendered": "<p>interspersed with HTML</p>"
}
]
}
} (Disclaimer: I only know the data model in very broad strokes here.) |
While the two endpoints address different use cases, it'd be highly useful to have both endpoints represent a block in a consistent way, e.g. {
"type": "core/paragraph",
"attributes": {
"content": "How are you?",
"dropCap": true
},
"content": "<!-- wp:core/paragraph {\"dropCap\":true} -->\n<p class=\"has-drop-cap\">How are you?</p>\n<!-- /wp:core/paragraph -->",
"rendered": "<p class=\"has-drop-cap\">How are you?</p>"
}
+1 |
Good point @noisysocks, I was simply passing the data back as provided by Thanks for the feedback @rmccue - I like your suggestion of adding this data onto the regular post endpoint content object - my only concern would be the overhead introduced in block parsing the post content, even if it hasn't been edited in gutenberg, seems like we need a performat |
I remapped the response fields to match #2503 and also moved the data to the post object response as content->blocks. Here is what the response from the demo page provided by my install of gutenberg looks like: https://gist.github.com/adamsilverstein/5369160faca6097c5e22e0256e380962 |
@noisysocks & @rmccue can you please take another look after my recent changes where I have addressed your suggestions. |
Won't this point become moot at the point where Gutenberg is the default editor anyway? Also, we could choose to only expose all the block pieces for |
lib/blocks.php
Outdated
|
||
// Set up the item data. | ||
$item_data = array(); | ||
$item_data['type'] = $block['blockName']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be $block_name
instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in 5937d1a
lib/blocks.php
Outdated
$block_name = isset( $block['blockName'] ) ? $block['blockName'] : null; | ||
$attributes = is_array( $block['attrs'] ) ? $block['attrs'] : null; | ||
$raw_content = isset( $block['rawContent'] ) ? $block['rawContent'] : null; | ||
if ( null !== $block_name ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be inverted with a continue instead:
if ( null === $block_name ) {
continue;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in d990979
lib/blocks.php
Outdated
$item_data = array(); | ||
$item_data['type'] = $block['blockName']; | ||
if ( null !== $attributes ) { | ||
$item_data['attributes'] = $block['attrs']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be $attributes
not $block['attrs']
. Also, properties should always be set if they're exposed in a given context to avoid index errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in 862bccc
lib/blocks.php
Outdated
if ( null !== $attributes ) { | ||
$item_data['attributes'] = $block['attrs']; | ||
} | ||
$item_data['content'] = $block['rawContent']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$block['rawContent']
-> $raw_content
Also, should this only be exposed for $context === 'edit'
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is perhaps misnamed when it is returned from the parser - its not really the raw content and doesn't include the html markers for example. This really represents the rendered html and is returned as content
for a block (matching naming from #2503). As far as I can see, rendered
is only used for a couple of block types where their content is dynamic, for example a recent post list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you review the data thats output, it seems public? we could show blocks only in edit context, but that would make my idea of using them for a front end data source less useful.
lib/blocks.php
Outdated
* | ||
* @return array Array of block data. | ||
*/ | ||
function get_block_data_for_api_from_post_content( $content ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe gutenberg_add_blocks_to_post_resource()
? This won't be used apart from the filter, so should probably be a prefixed name rather than a convenient one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in 36f1000
lib/blocks.php
Outdated
$blocks = get_block_data_for_api_from_post_content( $post->post_content ); | ||
if ( $blocks ) { | ||
$response->data['content']['blocks'] = $blocks; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The blocks
field should always be set, even if it's null
.
Also, this should probably use ->set_data()
instead of reaching into the object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for this being a part of the regular content endpoint(s) and glad to see that #1516 was mentioned. Architecturally, we should be thinking about more than just default post content here too since the blocks concept will likely be used in other scenarios (i.e. widgets and menus) in the future. |
…_api_from_post_content
Sourced attributes (like the content of a paragraph) rely on a DOM tree, which is not easily accessed server side. |
Happy to re-open and rebase the PR. Seemed useful to me as well, so happy to see this considered. |
Is there a way to ensure the content is only parsed once with |
@TimothyBJacobs Can you clarify where else |
Could you make the attribute names match the property names in How do you feel about moving |
I can remap the names if that is helpful: can you point me to the full list of names in WP_Block?
It is already there, my screenshot was incomplete, here is a full capture. |
Sure. There's only a few public properties:
https://github.com/WordPress/wordpress-develop/blob/master/src/wp-includes/class-wp-block.php |
As a result of |
@noisysocks One potential issue with trying to transform these on the fly is that the structure is nested, so we would need to recurse through each inner_block section to rename keys there, eg Thinking about this more and your statement that "It's regrettable that parse_block() uses this non-consistent naming" I am wondering why these keys are the way they are? Trying to transform them on the fly seems tricky. |
@TimothyBJacobs - Ah, I see that now, I guess that is required to get the rendered content? The only approach I can imaging here would be to store the parsed block data in a global, then if available the endpoint can use that data directly instead of recalculating it. Any other suggestions for how we might make this work? |
Yeah. I was thinking |
$blocks = array();
foreach ( parse_blocks( $content ) as $parsed_block ) {
$blocks[] = new WP_Block( $parsed_block, array() );
} |
I filed two tickets in WordPress core hoping we can move forward efforts for this POC:
|
Doing some cleanup in the repository. I was wondering if this PR still worth keeping open. Probably needs to be redone entirely if we ever get to it. What do you think? |
I still think this is a good addition to the API surface, would be nice to pick it up |
This pull request adds Gutenberg block data to the post endpoints
This provides a way to get the data and content of each block of a Gutenberg-built page from the front end. This would be very useful for building component based front ends, because the components could map one-to-one with gutenberg blocks. With these endpoints, an App could easily get the data it needs to render each component. This might also provide a patch for a standard component library matching Gutenberg blocks.