Fix extra undo/redo step when removing or replacing all blocks (#54457)

* Fix extra undo/redo step when removing or replacing all blocks

* add more comments

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

@@ -375,7 +375,7 @@ function getBlocksWithDefaultStylesApplied( blocks, blockEditorSettings ) {
  */
 export const replaceBlocks =
 	( clientIds, blocks, indexToSelect, initialPosition = 0, meta ) =>
-	( { select, dispatch } ) => {
+	( { select, dispatch, registry } ) => {
 		/* eslint-enable jsdoc/valid-types */
 		clientIds = castArray( clientIds );
 		blocks = getBlocksWithDefaultStylesApplied(
@@ -394,16 +394,22 @@ export const replaceBlocks =
 				return;
 			}
 		}
-		dispatch( {
-			type: 'REPLACE_BLOCKS',
-			clientIds,
-			blocks,
-			time: Date.now(),
-			indexToSelect,
-			initialPosition,
-			meta,
+		// We're batching these two actions because an extra `undo/redo` step can
+		// be created, based on whether we insert a default block or not.
+		registry.batch( () => {
+			dispatch( {
+				type: 'REPLACE_BLOCKS',
+				clientIds,
+				blocks,
+				time: Date.now(),
+				indexToSelect,
+				initialPosition,
+				meta,
+			} );
+			// To avoid a focus loss when removing the last block, assure there is
+			// always a default block if the last of the blocks have been removed.
+			dispatch.ensureDefaultBlock();
 		} );
-		dispatch.ensureDefaultBlock();
 	};
 
 /**

packages/block-editor/src/store/private-actions.js

@@ -92,7 +92,7 @@ export function showBlockInterface() {
  */
 export const privateRemoveBlocks =
 	( clientIds, selectPrevious = true, forceRemove = false ) =>
-	( { select, dispatch } ) => {
+	( { select, dispatch, registry } ) => {
 		if ( ! clientIds || ! clientIds.length ) {
 			return;
 		}
@@ -154,11 +154,14 @@ export const privateRemoveBlocks =
 			dispatch.selectPreviousBlock( clientIds[ 0 ], selectPrevious );
 		}
 
-		dispatch( { type: 'REMOVE_BLOCKS', clientIds } );
-
-		// To avoid a focus loss when removing the last block, assure there is
-		// always a default block if the last of the blocks have been removed.
-		dispatch( ensureDefaultBlock() );
+		// We're batching these two actions because an extra `undo/redo` step can
+		// be created, based on whether we insert a default block or not.
+		registry.batch( () => {
+			dispatch( { type: 'REMOVE_BLOCKS', clientIds } );
+			// To avoid a focus loss when removing the last block, assure there is
+			// always a default block if the last of the blocks have been removed.
+			dispatch( ensureDefaultBlock() );
+		} );
 	};
 
 /**

packages/block-editor/src/store/test/actions.js

@@ -219,8 +219,9 @@ describe( 'actions', () => {
 			};
 			const dispatch = jest.fn();
 			dispatch.ensureDefaultBlock = jest.fn();
+			const registry = createRegistry();
 
-			replaceBlock( 'chicken', block )( { select, dispatch } );
+			replaceBlock( 'chicken', block )( { select, dispatch, registry } );
 
 			expect( dispatch ).toHaveBeenCalledWith( {
 				type: 'REPLACE_BLOCKS',
@@ -285,8 +286,12 @@ describe( 'actions', () => {
 			};
 			const dispatch = jest.fn();
 			dispatch.ensureDefaultBlock = jest.fn();
+			const registry = createRegistry();
 
-			replaceBlocks( [ 'chicken' ], blocks )( { select, dispatch } );
+			replaceBlocks(
+				[ 'chicken' ],
+				blocks
+			)( { select, dispatch, registry } );
 
 			expect( dispatch ).toHaveBeenCalledWith( {
 				type: 'REPLACE_BLOCKS',
@@ -319,14 +324,15 @@ describe( 'actions', () => {
 			};
 			const dispatch = jest.fn();
 			dispatch.ensureDefaultBlock = jest.fn();
+			const registry = createRegistry();
 
 			replaceBlocks(
 				[ 'chicken' ],
 				blocks,
 				null,
 				null,
 				meta
-			)( { select, dispatch } );
+			)( { select, dispatch, registry } );
 
 			expect( dispatch ).toHaveBeenCalledWith( {
 				type: 'REPLACE_BLOCKS',
@@ -628,8 +634,9 @@ describe( 'actions', () => {
 			const dispatch = Object.assign( jest.fn(), {
 				selectPreviousBlock: jest.fn(),
 			} );
+			const registry = createRegistry();
 
-			removeBlocks( clientIds )( { select, dispatch } );
+			removeBlocks( clientIds )( { select, dispatch, registry } );
 
 			expect( dispatch.selectPreviousBlock ).toHaveBeenCalledWith(
 				clientId,
@@ -739,8 +746,8 @@ describe( 'actions', () => {
 			const dispatch = Object.assign( jest.fn(), {
 				selectPreviousBlock: jest.fn(),
 			} );
-
-			removeBlock( clientId )( { select, dispatch } );
+			const registry = createRegistry();
+			removeBlock( clientId )( { select, dispatch, registry } );
 
 			expect( dispatch.selectPreviousBlock ).toHaveBeenCalledWith(
 				clientId,
@@ -753,7 +760,7 @@ describe( 'actions', () => {
 			} );
 		} );
 
-		it( 'should dispatch REMOVE_BLOCKS action, opting out of select previous', () => {
+		it( 'should dispatch REMOVE_BLOCKS action, opting out of select previous', async () => {
 			const clientId = 'myclientid';
 
 			const select = {
@@ -765,7 +772,11 @@ describe( 'actions', () => {
 				selectPreviousBlock: jest.fn(),
 			} );
 
-			removeBlocks( [ clientId ], false )( { select, dispatch } );
+			const registry = createRegistry();
+			removeBlocks(
+				[ clientId ],
+				false
+			)( { select, dispatch, registry } );
 
 			expect( dispatch.selectPreviousBlock ).not.toHaveBeenCalled();