#47620 closed feature request (fixed)
REST API: Expose blocks registered on the server
Reported by: | gziolo | Owned by: | spacedmonkey |
---|---|---|---|
Milestone: | 5.5 | Priority: | high |
Severity: | normal | Version: | 5.0 |
Component: | REST API | Keywords: | has-patch has-unit-tests dev-feedback has-dev-note |
Focuses: | rest-api | Cc: |
Description (last modified by )
Related post:
https://make.wordpress.org/core/2019/05/27/the-block-registration-api-status-update/
Related GitHub issue:
https://github.com/WordPress/gutenberg/issues/2751
Related Block Registration API technical proposal with all the necessary details is available at:
https://github.com/WordPress/gutenberg/blob/master/docs/rfc/block-registration.md
The long term vision for the block discovery in WordPress includes:
- Fetching the available block types through REST APIs.
- Fetching block objects from posts through REST APIs.
There is no immediate need to start working on new block type related API endpoints. However, it would be great to have it included on the roadmap. Ideally, they should stay as close as possible to the WordPress.org API and the final shape of the endpoint for searching for blocks. This is going to be introduced as part of the Block Directory project.
Attachments (4)
Change History (47)
#4
@
5 years ago
Also related: https://github.com/WordPress/gutenberg/issues/4116
#5
@
5 years ago
- Keywords has-patch has-unit-tests dev-feedback added
- Owner set to spacedmonkey
- Status changed from new to assigned
I have created a first pass of the api in 47620.diff.
I have added the ability to filter name a block's namespace. Not sure if this is required, but seems like a nice feature.
I think this can land in WP 5.4. Thoughts @kadamwhite @TimothyBlynJacobs ?
#6
@
5 years ago
Just a bit of quick feedback, I think we should only have one get_item
route instead of registering a new rest route for each block.
This ticket was mentioned in Slack in #core-restapi by spacedmonkey. View the logs.
5 years ago
This ticket was mentioned in Slack in #core-editor by spacedmonkey. View the logs.
5 years ago
#9
in reply to:
↑ 3
@
5 years ago
- What fields should be exposed in the api?
You can see the full Block API which should be exposed to the client in https://github.com/WordPress/gutenberg/blob/master/docs/rfc/block-registration.md#block-api.
The gist of it is presented in the following example:
{ "name": "my-plugin/notice", "title": "Notice", "category": "common", "parent": [ "core/group" ], "icon": "star", "description": "Shows warning, error or success notices ...", "keywords": [ "alert", "message" ], "textDomain": "my-plugin", "attributes": { "message": { "type": "string", "source": "html", "selector": ".message" } }, "styleVariations": [ { "name": "default", "label": "Default", "isDefault": true }, { "name": "other", "label": "Other" } ], "editorScript": "build/editor.js", "script": "build/main.js", "editorStyle": "build/editor.css", "style": "build/style.css" }
I think some of the fields are missing in your initial diff.
What permissions should be required to access this fields?
That's a great question. I don't know myself. I can share for reference the implementation of the endpoint for the Block Directory:
https://github.com/WordPress/gutenberg/blob/b138818dd4860151eb345606b1396a198572aefa/lib/class-wp-rest-block-directory-controller.php#L76-L85
It doesn't look like it checks the permission for accessing blocks. /cc @tellyworth
#10
follow-up:
↓ 11
@
5 years ago
Just a bit of quick feedback, I think we should only have one get_item route instead of registering a new rest route for each block.
Thanks for the feedback at @TimothyBlynJacobs . I have updated the patch to not do this anymore see 47620.2.diff.
What permissions should be required to access this fields?
Current my patch uses edit_posts
cap. This api should be available to everyone that can edit a post, if it is to be used in the mobile app. install_plugins
cap is very limiting as by default only administrators have this cap.
As for the missing fields @gziolo , I wasn't sure of the state of the RFC, so didn't review it. Can we use this a design for the API?
Currently the API lists off a list of WP_Block_Type
objects, the definition of which can we found here. But this class only has the following properties.
- name
- render_callback
- attributes
- editor_script
- script
- editor_style
- style
The classes doesn't allow for other fields. So these would have to be added to the class. This is not a small change, as it would require the updating of the docs / unit tests / implementation of the register_block_type function. This should likely happen in another ticket, as this is a sizale change.
Reviewing the document, it seems like the missing fields are.
- title
- category
- parent
- icon
- description
- keywords
- textDomain
- styleVariations
But I would also recommend adding
- supports
I would also look at the newly added
- example
Also where is the URL for translations defined?
#11
in reply to:
↑ 10
@
5 years ago
Replying to spacedmonkey:
Current my patch uses
edit_posts
cap. This api should be available to everyone that can edit a post, if it is to be used in the mobile app.install_plugins
cap is very limiting as by default only administrators have this cap.
It's a different use case for the Block Directory, my point was that it doesn't use any other permission check there. This is something that should be addressed in both places.
As for the missing fields @gziolo , I wasn't sure of the state of the RFC, so didn't review it. Can we use this a design for the API?
It's based on the actual implementation in Gutenberg for core blocks and the Block Directory also follows the same API.
The classes doesn't allow for other fields. So these would have to be added to the class. This is not a small change, as it would require the updating of the docs / unit tests / implementation of the register_block_type function. This should likely happen in another ticket, as this is a sizale change.
It's a fair point. You can split work into two parts, whatever other reviewers feel is the best. To make it functional though, we need to have all those additional fields covered because we won't be able to integrate this new endpoint with the block editor on the client-side.
But I would also recommend adding
- supports
We plan to deprecate this field. In the long run, we want to provide a more robust API to achieve the same goal. It would be nice to skip this field from the REST API. We keep this field on the client for all core blocks.
I would also look at the newly added
- example
Yes, this one might be a good fit as well. We should double-check first whether the format provided can be encoded in JSON files.
Also where is the URL for translations defined?
Can you explain what is this about? Maybe @swissspidy will know :)
This ticket was mentioned in Slack in #core-editor by gziolo. View the logs.
5 years ago
#15
@
5 years ago
In the RFC, editor_script, script, editor_style and style are defines as URLs. However PHP registered block use script / style handles. See wp_enqueue_registered_block_scripts_and_styles.
The wp_enqueue_style
and wp_enqueue_script
can not currently handle passing a url to them.
TL:DR If you pass a URL in editor_script, script, editor_style and style fields, is not going to work and may break things.
Also looking into this, I believe it is pointless to just output the url / handle of the script. Without the dependencies, inline scripts, translations and localized, the script is unless. So maybe we either need to add all this data into the API or we need another REST API for this. Thoughts @gziolo ?
This ticket was mentioned in Slack in #core-restapi by spacedmonkey. View the logs.
5 years ago
#17
@
5 years ago
@spacedmonkey, this path to the script or the style isn't very useful on its own, see the full description:
https://github.com/WordPress/gutenberg/blob/master/docs/rfc/block-registration.md#WPDefinedAsset
in addition @youknowriad opened this ticket which perfectly fits to your question:
https://core.trac.wordpress.org/ticket/48654
This asset handling is the most complex part of the block definition but we need it to be able to (lazy) load blocks on demand in the client.
#18
@
5 years ago
As discussed in the REST API meeting, it makes sense to handle script and styles in another REST API. This is different data. Also a scripts and styles API has other uses, such as lazy loading.
Let's keep just the handles in this API for now, and move the discussion of scripts / styles into #48654
#19
@
5 years ago
I missed the chat but I echo what you shared. It makes a lot of sense to split this task into smaller parts to focus on what's already well defined.
#20
@
5 years ago
The WPDefinedAsset
object isn't specially useful. It doesn't allow a user to get a list of dependencies, localisation and translations, meaning it wouldn't work well for mobile app or lazying loading blocks. This why that work has moved to #48654 where the security issues can be discussed in more detail.
#21
@
5 years ago
Have we considered adding block versions to the API? We've already encountered some issues in the past where there was a mismatch between the version of the block shipping with the apps and the one in the server. A version number doesn't solve the whole problem of backwards and forwards compatibility, but at least can help detect and notify the user better if they need to update something.
#22
@
5 years ago
To clarify the mobile perspective, we are currently not loading any code from the server. We have doubts that the App Store Guidelines would allow us to do so. The current approach is that we are shipping the code for the core blocks we support with the apps. Once we have enough to support plugins, the idea is to start doing the same for the most popular/requested plugins and ship their code alongside the apps.
That's why we need a version to know which code matches to a specific block. We have already run into issues with that recently. Whenever a block markup changes, Gutenberg is designed to deal with backwards compatibility and migrate that content automatically. However, it might be the case that you try to open a block created with the newer markup with an older client, leading to "invalid content".
This can happen both ways: people might upgrade their site before their apps, but also we are currently developing gutenberg-mobile against master, so users would get updated on mobile way before they do on their sites, unless they are running the plugin.
#23
@
5 years ago
Decided to move the work for this ticket on github, gutenberg. So I have created a PR at #21065. Let move the discussion there and we can iterate faster in the plugin.
#24
@
5 years ago
- Keywords needs-refresh added
- Milestone changed from Future Release to 5.5
The GB PR has been merged. Milestoning this for 5.5 to handle merging the controllers to core when the time comes.
This ticket was mentioned in Slack in #core-restapi by spacedmonkey. View the logs.
5 years ago
This ticket was mentioned in PR #317 on WordPress/wordpress-develop by spacedmonkey.
5 years ago
#26
- Keywords needs-refresh removed
Ported from https://github.com/WordPress/gutenberg/pull/21065
Related post:
https://make.wordpress.org/core/2019/05/27/the-block-registration-api-status-update/
Related GitHub issue:
https://github.com/WordPress/gutenberg/issues/2751
Related Block Registration API technical proposal with all the necessary details is available at:
https://github.com/WordPress/gutenberg/blob/master/docs/rfc/block-registration.md
Trac ticket: https://core.trac.wordpress.org/ticket/47620
#27
@
5 years ago
- Keywords needs-dev-note added
This is a port from original PR in https://github.com/WordPress/gutenberg/pull/21065
I have updated the patch via a pull request on github. https://github.com/WordPress/wordpress-develop/pull/317
Look in @gziolo
TimothyBJacobs commented on PR #317:
5 years ago
#28
We need to add @ticket
annotations for the tests.
I'm also going to hold off on committing this until the Gutenberg merge happens for 5.5.
This ticket was mentioned in Slack in #core-restapi by timothybjacobs. View the logs.
5 years ago
spacedmonkey commented on PR #317:
5 years ago
#31
We need to add
@ticket
annotations for the tests.
I'm also going to hold off on committing this until the Gutenberg merge happens for 5.5.
Added comment for tickets and version numbers.
#32
@
4 years ago
We still need to include the context
property from Block API:
https://github.com/WordPress/gutenberg/pull/22686
As noted, we also need to merge Gutenberg and wire all block.json
files with the server-side block registration to have block definitions available to expose. It's worth mentioning that definitions for Embed blocks most likely won't be ready for WordPress 5.5 because this block requires major rewrite to use block variations internally.
#33
@
4 years ago
Actually, I discovered one issue with default values for supports
and providesContext
. When empty they get serialized to an array when passed to the client. I think we should default to null
to avoid this issue. At least until we have REST API endpoint integrated.
#34
@
4 years ago
I think adding the context properties in patch 47620-context-2.diff to the block type class should happen as a follow up on #48529
I have updated the code in wordpress-develop#317 after WordPress-gutenberg#22721.
#36
@
4 years ago
https://github.com/WordPress/wordpress-develop/pull/317 – I did another check after the latest update, everything looks good, all tests pass, code is properly backported. @TimothyBlynJacobs can you confirm that it's ready to go?
TimothyBJacobs commented on PR #317:
4 years ago
#37
I added some comments, I'd also like to see a test that covers all the different fields. There is a test that iterates over all the properties, but only a few are defined. I'd like to see something like register_block_type( 'test', [ /* all the args */ ] )
that then checks for each property with an expected value. Not repeating the logic from the controller itself, but asserting against a static string ( or object, array, etc... ). See the themes controller tests for instance.
spacedmonkey commented on PR #317:
4 years ago
#38
I added some comments, I'd also like to see a test that covers all the different fields. There is a test that iterates over all the properties, but only a few are defined. I'd like to see something like
register_block_type( 'test', [ /* all the args */ ] )
that then checks for each property with an expected value. Not repeating the logic from the controller itself, but asserting against a static string ( or object, array, etc... ). See the themes controller tests for instance.
I am not sure where tests for register_block_type
start and the controller tests here. Sounds like you want to repeated logic. Not sure I see the value of this.
TimothyBJacobs commented on PR #317:
4 years ago
#39
I am not sure where tests for register_block_type start and the controller tests here. Sounds like you want to repeated logic. Not sure I see the value of this.
The majority of our controllers have tests that are covered by other areas, yet they are still important so we make sure everything is output as expected. I think the themes controller is a great example of this.
TimothyBJacobs commented on PR #317:
4 years ago
#41
#42
@
4 years ago
Looks like there was an unintentional typo on line 37 in in the last commit?
trunk/src/wp-includes/class-wp-block-styles-registry.php
Shouldn't it be type instead of ty?
Thanks for the ticket @gziolo .
I have some questions.
Can someone loop someone from the mobile team into this ticket to give feedback.
CC @aduth