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.

Sign up for GitHub

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

Block editor settings endpoint - Remove experimental namespace #33128

Merged
merged 2 commits into from
Jul 2, 2021
Merged

Block editor settings endpoint - Remove experimental namespace #33128

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
397c4d6
Block editor settings endpoint - Remove experimental namespace
Jul 1, 2021
29dae13
Merge branch 'trunk' into update/block-editor-namespace
Jul 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/class-wp-rest-block-editor-settings-controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class WP_REST_Block_Editor_Settings_Controller extends WP_REST_Controller {
* Constructs the controller.
*/
public function __construct() {
$this->namespace = '__experimental/wp-block-editor/v1';
$this->namespace = 'wp-block-editor/v1';
Copy link
Member

Choose a reason for hiding this comment

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

I've checked some stable endpoints in the gutenberg and wordpress-develop codebase and this is what I see: wp/v2/pattern-directory, wp/v2/widget-types, wp/v2/widgets, wp/v2/block-directory, wp/v2/block-types, wp/v2/comments.

Following the convention of existing endpoints, I wonder if this should be wp/v2/block-editor-settings instead, wp/v2 as namespace and block-editor-settings as rest_base.

I'd welcome thoughts from people more experienced with the REST API naming schema, cc @TimothyBJacobs

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the suggestions @nosolosw!

From this comment it was suggested to use this naming instead of following the wp/v2/ convention, although it'd make things easier if it was using the same structure as the existing ones 😅

Copy link
Member

Choose a reason for hiding this comment

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

Yeah the intention to use the different namespace is that wp/v2 is for modeling data, whereas this is purely a block editor API returning configuration values.

There really is no plan to introduce a wp/v3 for numerous reasons, but putting this in a block editor namespace would allow for more freedom in introducing newer versions for breaking changes.

This would match what we did for Site Health in having a wp-site-health/v1 namespace. I think the only inconsistency here would be that ideally the block-directory and pattern-directory endpoints would've been part of this namespace as well.

Copy link
Member

Choose a reason for hiding this comment

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

I see, thanks for the context. Let's do this then.

As a data point for the future, I'd like to provide the thought that it's difficult to reason about why/when something is data or a setting from the perspective of a consumer of the API. By looking at the reference documentation, I see endpoints that are arguably config: post types, block types, site settings to name a few.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good! Let's keep the wp-site-health/v1 namespace 👍

Can we then proceed with removing the __experimental part? Thanks!

$this->rest_base = 'settings';
}

Expand Down