From aabacd20758af7ba227e8e2dc5a66d7699b45c1f Mon Sep 17 00:00:00 2001 From: Bernie Reiter Date: Mon, 6 Jul 2020 22:18:14 +0200 Subject: [PATCH] Edit Site: Remove templateIds state (#22893) Remove some now-obsolete state, and corresponding initial state passed from the server. We've found that the now-obsolete `gutenberg_find_template_post_and_parts()` calls in `lib/edit-site-page.php` were causing a considerable slowdown of the initial page render (see #23662). --- lib/edit-site-page.php | 25 +------ lib/template-loader.php | 2 +- packages/edit-site/src/store/actions.js | 16 ++--- packages/edit-site/src/store/reducer.js | 34 ---------- packages/edit-site/src/store/selectors.js | 22 ------- packages/edit-site/src/store/test/actions.js | 14 ++-- packages/edit-site/src/store/test/reducer.js | 65 ------------------- .../edit-site/src/store/test/selectors.js | 16 ----- 8 files changed, 14 insertions(+), 180 deletions(-) diff --git a/lib/edit-site-page.php b/lib/edit-site-page.php index 7658fe05a59516..18c1fbe9b00802 100644 --- a/lib/edit-site-page.php +++ b/lib/edit-site-page.php @@ -145,30 +145,11 @@ function gutenberg_edit_site_init( $hook ) { } $settings['styles'] = gutenberg_get_editor_styles(); - $template_ids = array(); - $template_part_ids = array(); - foreach ( get_template_types() as $template_type ) { - // Skip 'embed' for now because it is not a regular template type. - // Skip 'index' because it's a fallback that we handle differently. - if ( in_array( $template_type, array( 'embed', 'index' ), true ) ) { - continue; - } - - $current_template = gutenberg_find_template_post_and_parts( $template_type ); - if ( isset( $current_template ) ) { - $template_ids[ $template_type ] = $current_template['template_post']->ID; - $template_part_ids = $template_part_ids + $current_template['template_part_ids']; - } - } - $settings['editSiteInitialState'] = array(); - $settings['editSiteInitialState']['templateType'] = 'wp_template'; - $settings['editSiteInitialState']['templateIds'] = array_values( $template_ids ); - $settings['editSiteInitialState']['templatePartIds'] = array_values( $template_part_ids ); - - $settings['editSiteInitialState']['showOnFront'] = get_option( 'show_on_front' ); - $settings['editSiteInitialState']['page'] = array( + $settings['editSiteInitialState']['templateType'] = 'wp_template'; + $settings['editSiteInitialState']['showOnFront'] = get_option( 'show_on_front' ); + $settings['editSiteInitialState']['page'] = array( 'path' => '/', 'context' => 'page' === $settings['editSiteInitialState']['showOnFront'] ? array( 'postType' => 'page', diff --git a/lib/template-loader.php b/lib/template-loader.php index cb0993791e1fd4..8e9d34d97ba7c5 100644 --- a/lib/template-loader.php +++ b/lib/template-loader.php @@ -431,7 +431,7 @@ function gutenberg_template_loader_filter_block_editor_settings( $settings ) { } // If this is the Site Editor, auto-drafts for template parts have already been generated - // through `gutenberg_find_template_post_and_parts` in `gutenberg_edit_site_init`. + // through `filter_rest_wp_template_part_query`, when called via the REST API. if ( isset( $settings['editSiteInitialState'] ) ) { return $settings; } diff --git a/packages/edit-site/src/store/actions.js b/packages/edit-site/src/store/actions.js index 83545e1c4fce20..c68baf50a55094 100644 --- a/packages/edit-site/src/store/actions.js +++ b/packages/edit-site/src/store/actions.js @@ -51,11 +51,11 @@ export function setTemplate( templateId ) { } /** - * Returns an action object used to add a template. + * Adds a new template, and sets it as the current template. * * @param {Object} template The template. * - * @return {Object} Action object. + * @return {Object} Action object used to set the current template. */ export function* addTemplate( template ) { const newTemplate = yield dispatch( @@ -66,32 +66,28 @@ export function* addTemplate( template ) { template ); return { - type: 'ADD_TEMPLATE', + type: 'SET_TEMPLATE', templateId: newTemplate.id, }; } /** - * Returns an action object used to remove a template. + * Removes a template, and updates the current page and template. * * @param {number} templateId The template ID. * - * @return {Object} Action object. + * @return {Object} Action object used to set the current page and template. */ export function* removeTemplate( templateId ) { yield apiFetch( { path: `/wp/v2/templates/${ templateId }`, method: 'DELETE', } ); - yield dispatch( + return dispatch( 'core/edit-site', 'setPage', yield select( 'core/edit-site', 'getPage' ) ); - return { - type: 'REMOVE_TEMPLATE', - templateId, - }; } /** diff --git a/packages/edit-site/src/store/reducer.js b/packages/edit-site/src/store/reducer.js index d525425a382409..bd7cfd8740eb14 100644 --- a/packages/edit-site/src/store/reducer.js +++ b/packages/edit-site/src/store/reducer.js @@ -107,7 +107,6 @@ export function homeTemplateId( state ) { export function templateId( state, action ) { switch ( action.type ) { case 'SET_TEMPLATE': - case 'ADD_TEMPLATE': case 'SET_PAGE': return action.templateId; } @@ -143,7 +142,6 @@ export function templatePartId( state, action ) { export function templateType( state, action ) { switch ( action.type ) { case 'SET_TEMPLATE': - case 'ADD_TEMPLATE': case 'SET_PAGE': return 'wp_template'; case 'SET_TEMPLATE_PART': @@ -153,36 +151,6 @@ export function templateType( state, action ) { return state; } -/** - * Reducer returning the list of template IDs. - * - * @param {Object} state Current state. - * @param {Object} action Dispatched action. - * - * @return {Object} Updated state. - */ -export function templateIds( state = [], action ) { - switch ( action.type ) { - case 'ADD_TEMPLATE': - return [ ...state, action.templateId ]; - case 'REMOVE_TEMPLATE': - return state.filter( ( id ) => id !== action.templateId ); - } - - return state; -} - -/** - * Reducer returning the list of template part IDs. - * - * @param {Object} state Current state. - * - * @return {Object} Updated state. - */ -export function templatePartIds( state = [] ) { - return state; -} - /** * Reducer returning the page being edited. * @@ -219,8 +187,6 @@ export default combineReducers( { templateId, templatePartId, templateType, - templateIds, - templatePartIds, page, showOnFront, } ); diff --git a/packages/edit-site/src/store/selectors.js b/packages/edit-site/src/store/selectors.js index 760e6a39b17910..aaaf9e870d619f 100644 --- a/packages/edit-site/src/store/selectors.js +++ b/packages/edit-site/src/store/selectors.js @@ -123,28 +123,6 @@ export function getTemplateType( state ) { return state.templateType; } -/** - * Returns the current template IDs. - * - * @param {Object} state Global application state. - * - * @return {number[]} Template IDs. - */ -export function getTemplateIds( state ) { - return state.templateIds; -} - -/** - * Returns the current template part IDs. - * - * @param {Object} state Global application state. - * - * @return {number[]} Template part IDs. - */ -export function getTemplatePartIds( state ) { - return state.templatePartIds; -} - /** * Returns the current page object. * diff --git a/packages/edit-site/src/store/test/actions.js b/packages/edit-site/src/store/test/actions.js index 199eccb18fceea..07776d9d8bae6d 100644 --- a/packages/edit-site/src/store/test/actions.js +++ b/packages/edit-site/src/store/test/actions.js @@ -32,7 +32,7 @@ describe( 'actions', () => { } ); describe( 'addTemplate', () => { - it( 'should yield the DISPATCH control to create the template and return the ADD_TEMPLATE action', () => { + it( 'should yield the DISPATCH control to create the template and return the SET_TEMPLATE action', () => { const template = { slug: 'index' }; const newTemplate = { id: 1 }; @@ -45,7 +45,7 @@ describe( 'actions', () => { } ); expect( it.next( newTemplate ) ).toEqual( { value: { - type: 'ADD_TEMPLATE', + type: 'SET_TEMPLATE', templateId: newTemplate.id, }, done: true, @@ -54,7 +54,7 @@ describe( 'actions', () => { } ); describe( 'removeTemplate', () => { - it( 'should yield the API_FETCH control, yield the SELECT control to set the page by yielding the DISPATCH control for the SET_PAGE action, and return the REMOVE_TEMPLATE action', () => { + it( 'should yield the API_FETCH control and yield the SELECT control to set the page by yielding the DISPATCH control for the SET_PAGE action', () => { const templateId = 1; const page = { path: '/' }; @@ -72,19 +72,13 @@ describe( 'actions', () => { selectorName: 'getPage', args: [], } ); + // @FIXME: Test for done: true. expect( it.next( page ).value ).toEqual( { type: 'DISPATCH', storeKey: 'core/edit-site', actionName: 'setPage', args: [ page ], } ); - expect( it.next() ).toEqual( { - value: { - type: 'REMOVE_TEMPLATE', - templateId, - }, - done: true, - } ); } ); } ); diff --git a/packages/edit-site/src/store/test/reducer.js b/packages/edit-site/src/store/test/reducer.js index 269af0d67681d3..c220305c6dd705 100644 --- a/packages/edit-site/src/store/test/reducer.js +++ b/packages/edit-site/src/store/test/reducer.js @@ -13,8 +13,6 @@ import { templateId, templatePartId, templateType, - templateIds, - templatePartIds, page, showOnFront, } from '../reducer'; @@ -100,15 +98,6 @@ describe( 'state', () => { ).toEqual( 2 ); } ); - it( 'should update when a template is added', () => { - expect( - templateId( 1, { - type: 'ADD_TEMPLATE', - templateId: 2, - } ) - ).toEqual( 2 ); - } ); - it( 'should update when a page is set', () => { expect( templateId( 1, { @@ -157,14 +146,6 @@ describe( 'state', () => { ).toEqual( 'wp_template' ); } ); - it( 'should update when a template is added', () => { - expect( - templateType( undefined, { - type: 'ADD_TEMPLATE', - } ) - ).toEqual( 'wp_template' ); - } ); - it( 'should update when a page is set', () => { expect( templateType( undefined, { @@ -182,52 +163,6 @@ describe( 'state', () => { } ); } ); - describe( 'templateIds()', () => { - it( 'should apply default state', () => { - expect( templateIds( undefined, {} ) ).toEqual( [] ); - } ); - - it( 'should default to returning the same state', () => { - const state = {}; - expect( templateIds( state, {} ) ).toBe( state ); - } ); - - it( 'should add template IDs', () => { - expect( - templateIds( deepFreeze( [ 1 ] ), { - type: 'ADD_TEMPLATE', - templateId: 2, - } ) - ).toEqual( [ 1, 2 ] ); - } ); - - it( 'should remove template IDs', () => { - expect( - templateIds( deepFreeze( [ 1, 2 ] ), { - type: 'REMOVE_TEMPLATE', - templateId: 2, - } ) - ).toEqual( [ 1 ] ); - expect( - templateIds( deepFreeze( [ 1, 2 ] ), { - type: 'REMOVE_TEMPLATE', - templateId: 1, - } ) - ).toEqual( [ 2 ] ); - } ); - } ); - - describe( 'templatePartIds()', () => { - it( 'should apply default state', () => { - expect( templatePartIds( undefined, {} ) ).toEqual( [] ); - } ); - - it( 'should default to returning the same state', () => { - const state = {}; - expect( templatePartIds( state, {} ) ).toBe( state ); - } ); - } ); - describe( 'page()', () => { it( 'should apply default state', () => { expect( page( undefined, {} ) ).toEqual( {} ); diff --git a/packages/edit-site/src/store/test/selectors.js b/packages/edit-site/src/store/test/selectors.js index 2493cec574f97e..8335492fdf5598 100644 --- a/packages/edit-site/src/store/test/selectors.js +++ b/packages/edit-site/src/store/test/selectors.js @@ -9,8 +9,6 @@ import { getTemplateId, getTemplatePartId, getTemplateType, - getTemplateIds, - getTemplatePartIds, getPage, getShowOnFront, } from '../selectors'; @@ -131,20 +129,6 @@ describe( 'selectors', () => { } ); } ); - describe( 'getTemplateIds', () => { - it( 'returns the template IDs', () => { - const state = { templateIds: {} }; - expect( getTemplateIds( state ) ).toBe( state.templateIds ); - } ); - } ); - - describe( 'getTemplatePartIds', () => { - it( 'returns the template part IDs', () => { - const state = { templatePartIds: {} }; - expect( getTemplatePartIds( state ) ).toBe( state.templatePartIds ); - } ); - } ); - describe( 'getPage', () => { it( 'returns the page object', () => { const state = { page: {} };