Changes per review

Author: youknowriad

packages/block-editor/src/store/reducer.js

@@ -218,6 +218,19 @@ const withPostMetaUpdateCacheReset = ( reducer ) => ( state, action ) => {
 	return newState;
 };
 
+/**
+ * Utility returning an object with an empty object value for each key.
+ *
+ * @param {Array} objectKeys Keys to fill.
+ * @return {Object} Object filled with empty object as values for each clientId.
+ */
+const fillKeysWithEmptyObject = ( objectKeys ) => {
+	return objectKeys.reduce( ( result, key ) => {
+		result[ key ] = {};
+		return result;
+	}, {} );
+};
+
 /**
  * Higher-order reducer intended to compute a cache key for each block in the post.
  * A new instance of the cache key (empty object) is created each time the block object
@@ -235,7 +248,7 @@ const withBlockCache = ( reducer ) => ( state = {}, action ) => {
 	}
 	newState.cache = state.cache ? state.cache : {};
 
-	const addParentBlocks = ( clientIds ) => {
+	const getBlocksWithParentsClientIds = ( clientIds ) => {
 		return clientIds.reduce( ( result, clientId ) => {
 			let current = clientId;
 			do {
@@ -246,13 +259,6 @@ const withBlockCache = ( reducer ) => ( state = {}, action ) => {
 		}, [] );
 	};
 
-	const fillKeysWithEmptyObject = ( clientIds ) => {
-		return clientIds.reduce( ( result, key ) => {
-			result[ key ] = {};
-			return result;
-		}, {} );
-	};
-
 	switch ( action.type ) {
 		case 'RESET_BLOCKS':
 			newState.cache = mapValues( flattenBlocks( action.blocks ), () => ( {} ) );
@@ -266,7 +272,7 @@ const withBlockCache = ( reducer ) => ( state = {}, action ) => {
 			newState.cache = {
 				...newState.cache,
 				...fillKeysWithEmptyObject(
-					addParentBlocks( updatedBlockUids ),
+					getBlocksWithParentsClientIds( updatedBlockUids ),
 				),
 			};
 			break;
@@ -276,23 +282,23 @@ const withBlockCache = ( reducer ) => ( state = {}, action ) => {
 			newState.cache = {
 				...newState.cache,
 				...fillKeysWithEmptyObject(
-					addParentBlocks( [ action.clientId ] ),
+					getBlocksWithParentsClientIds( [ action.clientId ] ),
 				),
 			};
 			break;
 		case 'REPLACE_BLOCKS':
 			newState.cache = {
 				...omit( newState.cache, action.replacedClientIds ),
 				...fillKeysWithEmptyObject(
-					addParentBlocks( keys( flattenBlocks( action.blocks ) ) ),
+					getBlocksWithParentsClientIds( keys( flattenBlocks( action.blocks ) ) ),
 				),
 			};
 			break;
 		case 'REMOVE_BLOCKS':
 			newState.cache = {
 				...omit( newState.cache, action.removedClientIds ),
 				...fillKeysWithEmptyObject(
-					difference( addParentBlocks( action.clientIds ), action.clientIds ),
+					difference( getBlocksWithParentsClientIds( action.clientIds ), action.clientIds ),
 				),
 			};
 			break;
@@ -307,7 +313,7 @@ const withBlockCache = ( reducer ) => ( state = {}, action ) => {
 			newState.cache = {
 				...newState.cache,
 				...fillKeysWithEmptyObject(
-					addParentBlocks( updatedBlockUids )
+					getBlocksWithParentsClientIds( updatedBlockUids )
 				),
 			};
 			break;
@@ -321,7 +327,7 @@ const withBlockCache = ( reducer ) => ( state = {}, action ) => {
 			newState.cache = {
 				...newState.cache,
 				...fillKeysWithEmptyObject(
-					addParentBlocks( updatedBlockUids )
+					getBlocksWithParentsClientIds( updatedBlockUids )
 				),
 			};
 			break;
@@ -334,7 +340,7 @@ const withBlockCache = ( reducer ) => ( state = {}, action ) => {
 			newState.cache = {
 				...newState.cache,
 				...fillKeysWithEmptyObject(
-					addParentBlocks( updatedBlockUids )
+					getBlocksWithParentsClientIds( updatedBlockUids )
 				),
 			};
 		}

packages/block-editor/src/store/selectors.js

@@ -170,6 +170,11 @@ export const getBlock = createSelector(
 		};
 	},
 	( state, clientId ) => [
+		// Normally, we'd have both  `getBlockAttributes` dependancies and
+		// `getBlocks` (children) dependancies here but for performance reasons
+		// we use a denormalized cache key computed in the reducer that takes both
+		// the attributes and inner blocks into account. The value of the cache key
+		// is being changed whenever one of these dependencies is out of date.
 		state.blocks.cache[ clientId ],
 	]
 );