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).
WordPress · Jul 9, 2020 · aabacd2 · aabacd2
1 parent 1f57f61
commit aabacd2
Show file tree

Hide file tree

Showing 8 changed files with 14 additions and 180 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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: {} };