REST API: Sync /themes endpoint with Core's by ockham · Pull Request #23321 · WordPress/gutenberg

ockham · 2020-06-19T16:31:32Z

Description

In #21578, I added a few fields to Core's /themes endpoint, for use by the Site Editor's Template Switcher (see both #21578 and #21768). I then submitted those changes as a PR against Core. That PR underwent a number of modifications and was eventually merged; the new fields will be part of the /themes endpoint exposed by Core starting from the next WP release.

This PR updates the fields added by Gutenberg to follow the same semantics, as well as the callsites that use that endpoint.

Fixes #23054.

How has this been tested?

Test the template switcher's theme preview as described in #21768. You need to do this both with WP 5.4, and WordPress' current master branch. Verify that the theme preview works in both cases.

Alternatively:

Check out Gutenberg's master branch, and make sure you're using it with WordPress' latest master branch.
Apply the following patches to disable preloading of the /themes endpoint, and to suppress Gutenberg's addition of fields to the /themes endpoint (so that we get the /themes endpoint to reply with the fields that are defined in Core):

diff --git a/lib/edit-site-page.php b/lib/edit-site-page.php
index 8371486663..e5a43ae479 100644
--- a/lib/edit-site-page.php
+++ b/lib/edit-site-page.php
@@ -187,7 +187,7 @@ function gutenberg_edit_site_init( $hook ) {
                '/wp/v2/types?context=edit',
                '/wp/v2/taxonomies?per_page=100&context=edit',
                '/wp/v2/pages?per_page=100&context=edit',
-               '/wp/v2/themes?status=active',
+               //'/wp/v2/themes?status=active',
                array( '/wp/v2/media', 'OPTIONS' ),
        );
        $preload_data  = array_reduce(

diff --git a/lib/rest-api.php b/lib/rest-api.php
index c0f1c2aa31..9d11fa23ce 100644
--- a/lib/rest-api.php
+++ b/lib/rest-api.php
@@ -126,7 +126,7 @@ function gutenberg_register_rest_block_directory() {
        $block_directory_controller = new WP_REST_Block_Directory_Controller();
        $block_directory_controller->register_routes();
 }
-add_filter( 'rest_api_init', 'gutenberg_register_rest_block_directory' );
+//add_filter( 'rest_api_init', 'gutenberg_register_rest_block_directory' );
 
 /**
  * Registers the Block types REST API routes.

Go to the Site Editor.
Check the Network tab for the /themes endpoint's response, and copy it (in JSON format) to a text editor.
Revert the lib/rest-api.php patch, but keep the lib/edit-site-page.php patch.
Switch to this branch.
Apply the following patch to remove all of the fields that Core's endpoint provides (except for theme_supports, which predates REST API: Themes: Expose some additional basic fields wordpress-develop#222) in order to rely on Gutenberg to add those fields:

diff --git a/lib/rest-api.php b/lib/rest-api.php
index 5eccb8c3f9..a1e9409ab7 100644
--- a/lib/rest-api.php
+++ b/lib/rest-api.php
@@ -64,8 +64,9 @@ add_filter( 'rest_request_after_callbacks', 'gutenberg_filter_oembed_result', 10
  * @param WP_REST_Request  $request  Request object.
  */
 function gutenberg_filter_rest_prepare_theme( $response, $theme, $request ) {
-       $data   = $response->get_data();
-       $fields = array_keys( $data );
+       $theme_supports = $response->get_data()['theme_supports'];
+       $data           = array();
+       $fields         = array_keys( $data );
 
        /**
         * The following is basically copied from Core's WP_REST_Themes_Controller::prepare_item_for_response()
@@ -122,6 +123,8 @@ function gutenberg_filter_rest_prepare_theme( $response, $theme, $request ) {
                }
        }
 
+       $data['theme_supports'] = $theme_supports;
+
        $response->set_data( $data );
        return $response;
 }

Repeat step 4.
Compare the network responses -- they should be identical.

Types of changes

Update to sync with Core.

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.
I've included developer documentation if appropriate.
I've updated all React Native files affected by any refactorings/renamings in this PR.

github-actions · 2020-06-19T16:48:57Z

Size Change: +35 B (0%)

Total Size: 1.13 MB

Filename	Size	Change
`build/edit-site/index.js`	16.7 kB	+35 B (0%)

ℹ️ View Unchanged

Filename	Size	Change
`build/a11y/index.js`	1.14 kB	0 B
`build/annotations/index.js`	3.62 kB	0 B
`build/api-fetch/index.js`	3.4 kB	0 B
`build/autop/index.js`	2.82 kB	0 B
`build/blob/index.js`	620 B	0 B
`build/block-directory/index.js`	7.38 kB	0 B
`build/block-directory/style-rtl.css`	941 B	0 B
`build/block-directory/style.css`	942 B	0 B
`build/block-editor/index.js`	109 kB	0 B
`build/block-editor/style-rtl.css`	10.7 kB	0 B
`build/block-editor/style.css`	10.7 kB	0 B
`build/block-library/editor-rtl.css`	7.6 kB	0 B
`build/block-library/editor.css`	7.6 kB	0 B
`build/block-library/index.js`	130 kB	0 B
`build/block-library/style-rtl.css`	8.04 kB	0 B
`build/block-library/style.css`	8.04 kB	0 B
`build/block-library/theme-rtl.css`	730 B	0 B
`build/block-library/theme.css`	732 B	0 B
`build/block-serialization-default-parser/index.js`	1.88 kB	0 B
`build/block-serialization-spec-parser/index.js`	3.1 kB	0 B
`build/blocks/index.js`	48.2 kB	0 B
`build/components/index.js`	198 kB	0 B
`build/components/style-rtl.css`	15.9 kB	0 B
`build/components/style.css`	15.9 kB	0 B
`build/compose/index.js`	9.65 kB	0 B
`build/core-data/index.js`	11.4 kB	0 B
`build/data-controls/index.js`	1.29 kB	0 B
`build/data/index.js`	8.44 kB	0 B
`build/date/index.js`	5.47 kB	0 B
`build/deprecated/index.js`	772 B	0 B
`build/dom-ready/index.js`	569 B	0 B
`build/dom/index.js`	3.19 kB	0 B
`build/edit-navigation/index.js`	9.87 kB	0 B
`build/edit-navigation/style-rtl.css`	1.02 kB	0 B
`build/edit-navigation/style.css`	1.02 kB	0 B
`build/edit-post/index.js`	303 kB	0 B
`build/edit-post/style-rtl.css`	5.51 kB	0 B
`build/edit-post/style.css`	5.5 kB	0 B
`build/edit-site/style-rtl.css`	3.03 kB	0 B
`build/edit-site/style.css`	3.03 kB	0 B
`build/edit-widgets/index.js`	9.32 kB	0 B
`build/edit-widgets/style-rtl.css`	2.42 kB	0 B
`build/edit-widgets/style.css`	2.42 kB	0 B
`build/editor/editor-styles-rtl.css`	537 B	0 B
`build/editor/editor-styles.css`	539 B	0 B
`build/editor/index.js`	44.8 kB	0 B
`build/editor/style-rtl.css`	3.85 kB	0 B
`build/editor/style.css`	3.85 kB	0 B
`build/element/index.js`	4.65 kB	0 B
`build/escape-html/index.js`	733 B	0 B
`build/format-library/index.js`	7.72 kB	0 B
`build/format-library/style-rtl.css`	547 B	0 B
`build/format-library/style.css`	548 B	0 B
`build/hooks/index.js`	2.13 kB	0 B
`build/html-entities/index.js`	622 B	0 B
`build/i18n/index.js`	3.56 kB	0 B
`build/is-shallow-equal/index.js`	711 B	0 B
`build/keyboard-shortcuts/index.js`	2.51 kB	0 B
`build/keycodes/index.js`	1.94 kB	0 B
`build/list-reusable-blocks/index.js`	3.12 kB	0 B
`build/list-reusable-blocks/style-rtl.css`	450 B	0 B
`build/list-reusable-blocks/style.css`	451 B	0 B
`build/media-utils/index.js`	5.29 kB	0 B
`build/notices/index.js`	1.79 kB	0 B
`build/nux/index.js`	3.4 kB	0 B
`build/nux/style-rtl.css`	663 B	0 B
`build/nux/style.css`	660 B	0 B
`build/plugins/index.js`	2.56 kB	0 B
`build/primitives/index.js`	1.5 kB	0 B
`build/priority-queue/index.js`	788 B	0 B
`build/redux-routine/index.js`	2.85 kB	0 B
`build/rich-text/index.js`	14 kB	0 B
`build/server-side-render/index.js`	2.68 kB	0 B
`build/shortcode/index.js`	1.7 kB	0 B
`build/token-list/index.js`	1.28 kB	0 B
`build/url/index.js`	4.06 kB	0 B
`build/viewport/index.js`	1.85 kB	0 B
`build/warning/index.js`	1.14 kB	0 B
`build/wordcount/index.js`	1.17 kB	0 B

_{compressed-size-action}

Addison-Stavlo

I tested this on 5.4 and master, the theme preview works as expected. Although I noticed this still works on current Gutenberg master branch, so this isn't fixing the theme preview? That's more of a smoke test to ensure the endpoint is still working?

ockham · 2020-06-19T21:02:35Z

I tested this on 5.4 and master, the theme preview works as expected. Although I noticed this still works on current Gutenberg master branch, so this isn't fixing the theme preview? That's more of a smoke test to ensure the endpoint is still working?

Uh, you're right, there was a mistake in my instructions 😅 I've updated them now, I hope they make more sense now. They're still a bit complex -- we're basically comparing the output

from Core's master, with Gutenberg's added fields suppressed
to Core's fields suppressed, with Gutenberg's added fields updated by this branch.

spacedmonkey · 2020-06-22T09:57:39Z

Related: WordPress/wordpress-develop#264

TimothyBJacobs · 2020-06-22T16:09:29Z

 			/>
 			<div className="edit-site-template-switcher__theme-preview-description">
-				{ truncate( description, {
+				{ truncate( description.raw, {


Why isn't this the rendered version? The rendered version has wptexturize applied which handles things like curly quotes.

My thinking was that the rendered version might contain HTML markup (stuff like <a /> or <em />) and would thus require us to __dangerouslySetInnerHtml to render it in React, which I wanted to avoid.

It could, but it has wp_kses applied to be safe.

Hmm, okay. I can see that for name, but with description here, there's an extra complication -- I'm truncate()ing, so we might end up with an opening HTML tag that messes up the rest of the markup 😕

It looks like this could be possible with https://github.com/arendjr/text-clipper.

Thanks, I wasn't aware of that! I think it's worth considering, but since we'd require an extra dependency for truncating the theme description down to ~120 chars, it might be controversial, so I'd rather not block this PR on it. We can open a separate issue/PR tho!

TimothyBJacobs · 2020-06-22T16:10:14Z

 		<div className="edit-site-template-switcher__theme-preview">
 			<span className="edit-site-template-switcher__theme-preview-name">
-				{ name }
+				{ name.raw }


Why not rendered here as well? We get a safe fallback if the theme doesn't have a proper name listed.

I think we should, that matches core's behavior see wp_prepare_themes_for_js.

TimothyBJacobs · 2020-06-22T16:12:35Z

I left some comments. I think we also need to consider the value of _fields when checking that a field has not been included by Core already.

I am somewhat worried about making sure these stay in sync with Core. Have we ruled out requiring trunk for experimental features like this?

ockham · 2020-06-22T20:55:11Z

I left some comments. I think we also need to consider the value of _fields when checking that a field has not been included by Core already.

Ah yeah, good point. I'll modify it to that effect 👍

I am somewhat worried about making sure these stay in sync with Core. Have we ruled out requiring trunk for experimental features like this?

I haven't really been in any conversation regarding that. TBH I'd rather avoid that -- I think it's fair to require a rock-solid implementation of e.g. a REST API endpoint in Core, but I was happy that it wasn't a blocker for developing some code on the client side that depended on it.

Getting a preliminary version of the endpoint additions into Gutenberg allowed me (and others) to iterate quickly on the client-side code, while I was at the same time refining the Core implementation of the endpoint (in accordance with feedback). Overall, it took about 8 weeks until the latter was ready to be merged; that's the amount of time I would've had to wait if we had required those changes to be in Core's trunk for Gutenberg to use it.

TimothyBJacobs · 2020-06-22T21:29:12Z

Getting a preliminary version of the endpoint additions into Gutenberg allowed me (and others) to iterate quickly on the client-side code, while I was at the same time refining the Core implementation of the endpoint (in accordance with feedback). Overall, it took about 8 weeks until the latter was ready to be merged; that's the amount of time I would've had to wait if we had required those changes to be in Core's trunk for Gutenberg to use it.

I think I did a bad job of explaining what I meant. The idea would still be that code lives in Gutenberg first, but once it is merged to trunk, trunk would be required. That way we don't have to worry about any future changes to trunk making it back to the Gutenberg plugin or the syncing back to Gutenberg from Core was incorrect.

So the development flow would be...

Develop API in Gutenberg.
Stabilize API and include in trunk.
Drop development API in Gutenberg and use the trunk version instead.

ockham · 2020-06-23T09:26:48Z

Getting a preliminary version of the endpoint additions into Gutenberg allowed me (and others) to iterate quickly on the client-side code, while I was at the same time refining the Core implementation of the endpoint (in accordance with feedback). Overall, it took about 8 weeks until the latter was ready to be merged; that's the amount of time I would've had to wait if we had required those changes to be in Core's trunk for Gutenberg to use it.

I think I did a bad job of explaining what I meant. The idea would still be that code lives in Gutenberg first, but once it is merged to trunk, trunk would be required. That way we don't have to worry about any future changes to trunk making it back to the Gutenberg plugin or the syncing back to Gutenberg from Core was incorrect.

So the development flow would be...

Develop API in Gutenberg.

Stabilize API and include in trunk.

Drop development API in Gutenberg and use the trunk version instead.

Right, thanks for clarifying! But wouldn't that mean that once a new version of the Gutenberg plugin is released after 3., it could no longer be tested on a stable WordPress Core install?

youknowriad · 2020-06-23T09:55:17Z

we still need to maintain compatibility in the Gutenberg plugin with 5.4 even after 5.5 lands.

ockham · 2020-06-23T10:04:09Z

I left some comments. I think we also need to consider the value of _fields when checking that a field has not been included by Core already.

Ah yeah, good point. I'll modify it to that effect

I just tested adding some _fields query args to the endpoint, and it seems to work already -- probably because the rest_filter_response_fields filter is run after gutenberg_filter_rest_prepare_theme. I'll leave as-is, since I don't think the little extra-optimization for this back-compat patch is worth copy-pasting a bunch of parsing logic.

ockham · 2020-06-25T11:36:55Z

@TimothyBJacobs I think I addressed your feedback, would you mind giving this another look? 😊 🙏

TimothyBJacobs · 2020-06-26T04:13:01Z

we still need to maintain compatibility in the Gutenberg plugin with 5.4 even after 5.5 lands.

I'll move this to a separate issue, but I think doing this for some things will be completely impossible, like the navigation screen due to how batch requests need to be implemented in Core. I also think its going to prove difficult to keep the larger REST API changes in sync.

I think I addressed your feedback, would you mind giving this another look?

Looks good. I still wish we'd use the same data ( rendered description ) that we do in wp-admin, but I can see how that complicates things.

ockham · 2020-06-28T09:35:19Z

we still need to maintain compatibility in the Gutenberg plugin with 5.4 even after 5.5 lands.

I'll move this to a separate issue, but I think doing this for some things will be completely impossible, like the navigation screen due to how batch requests need to be implemented in Core. I also think its going to prove difficult to keep the larger REST API changes in sync.

I might be missing something here, but is this related to the changes in this PR? The way we're adding fields to the /themes endpoint should work with WP 5.4 and 5.5 regardless, shouldn't it? 🤔 (Retaining that sort of compatibility was kind of the point of this PR 😅 )

I think I addressed your feedback, would you mind giving this another look?

Looks good. I still wish we'd use the same data ( rendered description ) that we do in wp-admin, but I can see how that complicates things.

Thank you! In #23321 (comment), I'm suggesting a follow-up issue/PR to further look into that 🙂

ockham · 2020-06-28T09:52:33Z

The test setup has changed and tests seem to be stuck. I'll rebase.

ockham added the [Feature] Full Site Editing label Jun 19, 2020

ockham requested review from Addison-Stavlo, TimothyBJacobs and vindl June 19, 2020 16:31

ockham requested review from spacedmonkey and youknowriad as code owners June 19, 2020 16:31

ockham self-assigned this Jun 19, 2020

Addison-Stavlo reviewed Jun 19, 2020

View reviewed changes

ockham mentioned this pull request Jun 22, 2020

WP 5.5: merge all PHP changes #23210

Closed

12 tasks

TimothyBJacobs reviewed Jun 22, 2020

View reviewed changes

TimothyBJacobs self-requested a review June 26, 2020 04:15

TimothyBJacobs approved these changes Jun 26, 2020

View reviewed changes

ockham added 4 commits June 28, 2020 11:52

Template Switcher: Get raw theme name

81fa574

ThemePreview: Update to match new /themes endpoint format

fbc82c0

REST API: Sync /themes endpoint with Core's

ac9f927

phpcbf

5e2a1e6

Use dangerouslySetInnerHTML and name.rendered, but keep description.raw

870324e

ockham force-pushed the update/rest-api-themes-endpoint-to-match-core branch from ec6f16d to 870324e Compare June 28, 2020 09:53

ockham merged commit 332397e into master Jun 28, 2020

ockham deleted the update/rest-api-themes-endpoint-to-match-core branch June 28, 2020 10:17

github-actions Bot added this to the Gutenberg 8.5 milestone Jun 28, 2020

Uh oh!

Conversation

ockham commented Jun 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How has this been tested?

Types of changes

Checklist:

Uh oh!

github-actions Bot commented Jun 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Addison-Stavlo left a comment

Choose a reason for hiding this comment

Uh oh!

ockham commented Jun 19, 2020

Uh oh!

spacedmonkey commented Jun 22, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TimothyBJacobs commented Jun 22, 2020

Uh oh!

ockham commented Jun 22, 2020

Uh oh!

TimothyBJacobs commented Jun 22, 2020

Uh oh!

ockham commented Jun 23, 2020

Uh oh!

youknowriad commented Jun 23, 2020

Uh oh!

ockham commented Jun 23, 2020

Uh oh!

ockham commented Jun 25, 2020

Uh oh!

TimothyBJacobs commented Jun 26, 2020

Uh oh!

ockham commented Jun 28, 2020

Uh oh!

ockham commented Jun 28, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ockham commented Jun 19, 2020 •

edited

Loading

github-actions Bot commented Jun 19, 2020 •

edited

Loading