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

FSE: Move initial template fetch to client #23186

Merged
merged 2 commits into from
Jun 16, 2020

Conversation

noahtallen
Copy link
Member

@noahtallen noahtallen commented Jun 15, 2020

Description

This PR solves in issue with initial homepage template resolution in the site editor.

If a theme specifies three block templates (home, singular, index), the following behavior should be true when visiting the root of your site:

  1. When front page is set to show a static page, the singular template is resolved.
  2. When front page is set to show posts, the home template is resolved.

This behavior is correct on the front end, but in the editor, it loaded index in each case. This is because we initialize the editor with the front-page template in PHP. Unfortunately, template resolution does not work like this: each template type does not resolve to its closest match. Rather, in most cases, each template type resolves to either itself or to index.

As a result, since the theme didn't have the front-page template, it resolved to index. Unfortunately, I did not see a clear way to do server-side template resolution because WordPress does not actually provide a function to give you the closest matching template. Instead, as it renders an individual page or post, it will use the first template which both exists and matches the criteria of the individual post. The emphasis here is that this happens during page/post render, and not via an easily consumable function.

So I figured it'd be easiest for now to just use the existing front-end findTemplate helper by setting the page up front. It definitely solves the issue, but it does add some overhead to loading the editor. I've solved the overhead problem by displaying the editor even before the template loads. This should make the first meaningful paint more snappy in general, even without the client loading stuff.

How has this been tested?

Locally in edit site

Screenshots

Types of changes

Bug fix

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.

@noahtallen noahtallen added [Type] Bug An existing feature does not function as intended [Feature] Full Site Editing labels Jun 15, 2020
@noahtallen noahtallen self-assigned this Jun 15, 2020
@github-actions
Copy link

github-actions bot commented Jun 15, 2020

Size Change: +7 B (0%)

Total Size: 1.13 MB

Filename Size Change
build/edit-site/index.js 16.6 kB +7 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.83 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 7.27 kB 0 B
build/block-directory/style-rtl.css 892 B 0 B
build/block-directory/style.css 892 B 0 B
build/block-editor/index.js 106 kB 0 B
build/block-editor/style-rtl.css 12.1 kB 0 B
build/block-editor/style.css 12.1 kB 0 B
build/block-library/editor-rtl.css 7.88 kB 0 B
build/block-library/editor.css 7.89 kB 0 B
build/block-library/index.js 129 kB 0 B
build/block-library/style-rtl.css 7.96 kB 0 B
build/block-library/style.css 7.96 kB 0 B
build/block-library/theme-rtl.css 684 B 0 B
build/block-library/theme.css 686 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.1 kB 0 B
build/components/index.js 195 kB 0 B
build/components/style-rtl.css 19.5 kB 0 B
build/components/style.css 19.5 kB 0 B
build/compose/index.js 9.32 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 568 B 0 B
build/dom/index.js 3.17 kB 0 B
build/edit-navigation/index.js 8.26 kB 0 B
build/edit-navigation/style-rtl.css 975 B 0 B
build/edit-navigation/style.css 974 B 0 B
build/edit-post/index.js 302 kB 0 B
build/edit-post/style-rtl.css 5.6 kB 0 B
build/edit-post/style.css 5.6 kB 0 B
build/edit-site/style-rtl.css 2.96 kB 0 B
build/edit-site/style.css 2.96 kB 0 B
build/edit-widgets/index.js 9.34 kB 0 B
build/edit-widgets/style-rtl.css 2.4 kB 0 B
build/edit-widgets/style.css 2.4 kB 0 B
build/editor/editor-styles-rtl.css 423 B 0 B
build/editor/editor-styles.css 423 B 0 B
build/editor/index.js 44.8 kB 0 B
build/editor/style-rtl.css 4.26 kB 0 B
build/editor/style.css 4.27 kB 0 B
build/element/index.js 4.64 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 502 B 0 B
build/format-library/style.css 502 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 710 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.13 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 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 616 B 0 B
build/nux/style.css 613 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 789 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

@noahtallen noahtallen force-pushed the try/move-init-template-fetch-to-server branch from c2df2c3 to eb4fe79 Compare June 16, 2020 00:55
$settings['editSiteInitialState'] = array();

$settings['editSiteInitialState']['homeTemplateId'] = $current_template_id;
$settings['editSiteInitialState']['templateId'] = $current_template_id;
$settings['editSiteInitialState']['templateType'] = 'wp_template';
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we can also get rid of all of these except for showOnFront unless that is already in the site entity.

Copy link
Member Author

Choose a reason for hiding this comment

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

Cool. I'll look into removing these in a follow-up :)

@ockham
Copy link
Contributor

ockham commented Jun 16, 2020

Unfortunately, I did not see a clear way to do server-side template resolution because WordPress does not actually provide a function to give you the closest matching template. Instead, as it renders an individual page or post, it will use the first template which both exists and matches the criteria of the individual post. The emphasis here is that this happens during page/post render, and not via an easily consumable function.

Yep, that's the crux here. We basically use the ?_wp-find-template query arg as a resort -- we can append it to the route and rely on WordPress' rendering of that route to give us the correct template.

A nice alternative would be to have the wp_template entities endpoint accept a route arg and returns the template that's in charge of rendering that route. However, that's not easy to pull off, as WordPress' relevant template hierarchy functions evaluate context that's set implicitly during rendering (such as entity IDs and the like), see this discussion -- we would likely have to make that explicit (i.e. change a bunch of get_{$type}_template functions in core to accept arguments relevant for context explicitly, or duplicate all that in Gutenberg (which doesn't seem great in terms of maintainability).

So I figured it'd be easiest for now to just use the existing front-end findTemplate helper by setting the page up front.

I think that's a perfectly legit fix 👍

@@ -59,7 +59,7 @@ function gutenberg_add_template_loader_filters() {
* @param string $template_type A template type.
* @return string[] A list of template candidates, in descending order of priority.
*/
function get_template_hierachy( $template_type ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

😅

Copy link
Member

@vindl vindl left a comment

Choose a reason for hiding this comment

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

Works great for me! 🚀

@noahtallen noahtallen merged commit bd46c6f into master Jun 16, 2020
@noahtallen noahtallen deleted the try/move-init-template-fetch-to-server branch June 16, 2020 16:49
@ellatrix ellatrix mentioned this pull request Jun 17, 2020
12 tasks
This was referenced Jun 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants