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

Add block data to REST API #2649

Open
wants to merge 69 commits into
base: trunk
Choose a base branch
from

Conversation

adamsilverstein
Copy link
Member

@adamsilverstein adamsilverstein commented Sep 2, 2017

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.

@BE-Webdesign
Copy link
Contributor

#2503 is similar. They are addressing two different problems but, it would be good to have coordination between this and #2503 .

@adamsilverstein
Copy link
Member Author

@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)

Copy link
Contributor

@rmccue rmccue left a 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.)

lib/class-wp-rest-gutenberg-blocks.php Outdated Show resolved Hide resolved
lib/class-wp-rest-gutenberg-blocks.php Outdated Show resolved Hide resolved
lib/class-wp-rest-gutenberg-blocks.php Outdated Show resolved Hide resolved
lib/class-wp-rest-gutenberg-blocks.php Outdated Show resolved Hide resolved
@rmccue
Copy link
Contributor

rmccue commented Sep 4, 2017

In the post object, content contains the various representations of the same piece of data: the post content. As I understand it, with Gutenberg, the blocks are simply another representation of that same piece of data. (We also intentionally designed content for this purpose; although Gutenberg wasn't in sight at the time, other page builders were, and making content an object gave us more extensibility there.)

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 content called blocks, which could also contain its own raw and rendered fields (based on the provided context).

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.)

@noisysocks
Copy link
Member

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)

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>"
}

In the post object, content contains the various representations of the same piece of data: the post content. As I understand it, with Gutenberg, the blocks are simply another representation of that same piece of data.

+1

@adamsilverstein
Copy link
Member Author

While the two endpoints address different use cases, it'd be highly useful to have both endpoints represent a block in a consistent way.

Good point @noisysocks, I was simply passing the data back as provided by gutenberg_parse_blocks(). looks like I need to transform the naming anyway so I'll try to match the block object format from #2503.

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 has_been_edited_in_gutenberg() helper, if there is one I couldn't find it.

@adamsilverstein
Copy link
Member Author

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

@adamsilverstein
Copy link
Member Author

@noisysocks & @rmccue can you please take another look after my recent changes where I have addressed your suggestions.

@rmccue
Copy link
Contributor

rmccue commented Sep 5, 2017

my only concern would be the overhead introduced in block parsing the post content, even if it hasn't been edited in gutenberg

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 context=edit if it's a frontend concern.

rmccue
rmccue previously requested changes Sep 5, 2017
lib/blocks.php Outdated

// Set up the item data.
$item_data = array();
$item_data['type'] = $block['blockName'];
Copy link
Contributor

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

Copy link
Member Author

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 ) {
Copy link
Contributor

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;
}

Copy link
Member Author

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'];
Copy link
Contributor

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.

Copy link
Member Author

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'];
Copy link
Contributor

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'?

Copy link
Member Author

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.

Copy link
Member Author

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 ) {
Copy link
Contributor

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.

Copy link
Member Author

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 Show resolved Hide resolved
lib/blocks.php Outdated
$blocks = get_block_data_for_api_from_post_content( $post->post_content );
if ( $blocks ) {
$response->data['content']['blocks'] = $blocks;
}
Copy link
Contributor

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.

Copy link
Member Author

@adamsilverstein adamsilverstein Sep 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 957dc7e & bc5f31e

@joemcgill
Copy link
Member

+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.

@mtias
Copy link
Member

mtias commented Oct 9, 2020

Sourced attributes (like the content of a paragraph) rely on a DOM tree, which is not easily accessed server side.

@adamsilverstein
Copy link
Member Author

@adamsilverstein I think there's still value in an endpoint returning the WP_Block_Parser response, even if it's not as granular as wp.blocks.parse!

Happy to re-open and rebase the PR. Seemed useful to me as well, so happy to see this considered.

@TimothyBJacobs
Copy link
Member

Is there a way to ensure the content is only parsed once with parse_blocks() instead of separately for when calling do_blocks() on the rendered content and then again when building this representation.

@adamsilverstein
Copy link
Member Author

Removed unrelated changed and cleaned up return logic. Rest API data looks like this (from the default Gutenberg post):

image

@adamsilverstein
Copy link
Member Author

Is there a way to ensure the content is only parsed once with parse_blocks() instead of separately for when calling do_blocks() on the rendered content and then again when building this representation.

@TimothyBJacobs Can you clarify where else parse_blocks would normally already be called for a REST API request? Is it already called by core in the post route for the rendered content?

@noisysocks
Copy link
Member

Could you make the attribute names match the property names in WP_Block? In other words, rename blockNamename and attrsattributes. It's regrettable that parse_block() uses this non-consistent naming 🙂

How do you feel about moving blocks to content.blocks?

@adamsilverstein
Copy link
Member Author

Could you make the attribute names match the property names in WP_Block? In other words, rename blockName → name and attrs → attributes. It's regrettable that parse_block() uses this non-consistent naming 🙂

I can remap the names if that is helpful: can you point me to the full list of names in WP_Block?

How do you feel about moving blocks to content.blocks?

It is already there, my screenshot was incomplete, here is a full capture.

@noisysocks
Copy link
Member

I can remap the names if that is helpful: can you point me to the full list of names in WP_Block?

Sure. There's only a few public properties:

  • attributes (computed dynamically using __get)
  • parsed_block (not relevant)
  • name
  • block_type (not relevant)
  • context (not relevant)
  • inner_blocks
  • inner_html
  • inner_content

https://github.com/WordPress/wordpress-develop/blob/master/src/wp-includes/class-wp-block.php

@TimothyBJacobs
Copy link
Member

Is there a way to ensure the content is only parsed once with parse_blocks() instead of separately for when calling do_blocks() on the rendered content and then again when building this representation.

@TimothyBJacobs Can you clarify where else parse_blocks would normally already be called for a REST API request? Is it already called by core in the post route for the rendered content?

As a result of do_blocks as a result of apply_filters( 'the_content' )

@adamsilverstein
Copy link
Member Author

adamsilverstein commented Nov 6, 2020

@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

image

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.

@adamsilverstein
Copy link
Member Author

As a result of do_blocks as a result of apply_filters( 'the_content' )

@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?

@TimothyBJacobs
Copy link
Member

As a result of do_blocks as a result of apply_filters( 'the_content' )

@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 parse_blocks() would essentially memoize. Maybe we better leave that for a separate ticket.

@noisysocks
Copy link
Member

@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

WP_Block already does something like this in its constructor. You might be able to reuse that.

$blocks = array();
foreach ( parse_blocks( $content ) as $parsed_block ) {
	$blocks[] = new WP_Block( $parsed_block, array() );
}

@gziolo gziolo removed the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label Nov 28, 2020
@skorasaurus skorasaurus mentioned this pull request Dec 14, 2020
@mtias mtias mentioned this pull request Dec 24, 2020
16 tasks
Base automatically changed from master to trunk March 1, 2021 15:42
@gziolo
Copy link
Member

gziolo commented Jul 6, 2021

I filed two tickets in WordPress core hoping we can move forward efforts for this POC:

@gziolo gziolo added the [Feature] Block API API that allows to express the block paradigm. label Feb 11, 2023
@gziolo gziolo mentioned this pull request Feb 13, 2023
58 tasks
@youknowriad
Copy link
Contributor

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?

@mtias
Copy link
Member

mtias commented Aug 3, 2024

I still think this is a good addition to the API surface, would be nice to pick it up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core REST API Task Task for Core REST API efforts [Feature] Block API API that allows to express the block paradigm. REST API Interaction Related to REST API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet