RTC: Fix inner template multiple insertion

packages/block-editor/src/components/inner-blocks/use-inner-block-template-sync.js

@@ -14,6 +14,7 @@ import { synchronizeBlocksWithTemplate } from '@wordpress/blocks';
  * Internal dependencies
  */
 import { store as blockEditorStore } from '../../store';
+import { flushPendingNestedSettingsUpdates } from './use-nested-settings-update';
 
 /**
  * This hook makes sure that a block's inner blocks stay in sync with the given
@@ -48,71 +49,61 @@ export default function useInnerBlockTemplateSync(
 	const existingTemplateRef = useRef( null );
 
 	useLayoutEffect( () => {
-		let isCancelled = false;
-
 		const {
 			getBlocks,
 			getSelectedBlocksInitialCaretPosition,
 			isBlockSelected,
 		} = registry.select( blockEditorStore );
-		const { replaceInnerBlocks, __unstableMarkNextChangeAsNotPersistent } =
+		const { __unstableMarkNextChangeAsNotPersistent, replaceInnerBlocks } =
 			registry.dispatch( blockEditorStore );
 
-		// There's an implicit dependency between useInnerBlockTemplateSync and useNestedSettingsUpdate
-		// The former needs to happen after the latter and since the latter is using microtasks to batch updates (performance optimization),
-		// we need to schedule this one in a microtask as well.
-		// Example: If you remove queueMicrotask here, ctrl + click to insert quote block won't close the inserter.
-		window.queueMicrotask( () => {
-			if ( isCancelled ) {
-				return;
-			}
+		// Only synchronize innerBlocks with template if innerBlocks are empty
+		// or a locking "all" or "contentOnly" exists directly on the block.
+		const currentInnerBlocks = getBlocks( clientId );
+		const shouldApplyTemplate =
+			currentInnerBlocks.length === 0 ||
+			templateLock === 'all' ||
+			templateLock === 'contentOnly';
 
-			// Only synchronize innerBlocks with template if innerBlocks are empty
-			// or a locking "all" or "contentOnly" exists directly on the block.
-			const currentInnerBlocks = getBlocks( clientId );
-			const shouldApplyTemplate =
-				currentInnerBlocks.length === 0 ||
-				templateLock === 'all' ||
-				templateLock === 'contentOnly';
+		const hasTemplateChanged = ! fastDeepEqual(
+			template,
+			existingTemplateRef.current
+		);
 
-			const hasTemplateChanged = ! fastDeepEqual(
-				template,
-				existingTemplateRef.current
-			);
+		if ( ! shouldApplyTemplate || ! hasTemplateChanged ) {
+			return;
+		}
 
-			if ( ! shouldApplyTemplate || ! hasTemplateChanged ) {
-				return;
-			}
+		existingTemplateRef.current = template;
+		const nextBlocks = synchronizeBlocksWithTemplate(
+			currentInnerBlocks,
+			template
+		);
 
-			existingTemplateRef.current = template;
-			const nextBlocks = synchronizeBlocksWithTemplate(
-				currentInnerBlocks,
-				template
+		if ( ! fastDeepEqual( nextBlocks, currentInnerBlocks ) ) {
+			// This hook depends on useNestedSettingsUpdate having applied
+			// the block-list settings for this InnerBlocks area. Flush pending
+			// settings before the template replacement to preserve that ordering.
+			// Example: If you remove flush here, ctrl + click to insert quote
+			// block won't close the inserter.
+			flushPendingNestedSettingsUpdates( registry );
+			__unstableMarkNextChangeAsNotPersistent( {
+				history: 'ignore',
+			} );
+			replaceInnerBlocks(
+				clientId,
+				nextBlocks,
+				currentInnerBlocks.length === 0 &&
+					templateInsertUpdatesSelection &&
+					nextBlocks.length !== 0 &&
+					isBlockSelected( clientId ),
+				// This ensures the "initialPosition" doesn't change when applying the template
+				// If we're supposed to focus the block, we'll focus the first inner block
+				// otherwise, we won't apply any auto-focus.
+				// This ensures for instance that the focus stays in the inserter when inserting the "buttons" block.
+				getSelectedBlocksInitialCaretPosition()
 			);
-
-			if ( ! fastDeepEqual( nextBlocks, currentInnerBlocks ) ) {
-				__unstableMarkNextChangeAsNotPersistent( {
-					history: 'ignore',
-				} );
-				replaceInnerBlocks(
-					clientId,
-					nextBlocks,
-					currentInnerBlocks.length === 0 &&
-						templateInsertUpdatesSelection &&
-						nextBlocks.length !== 0 &&
-						isBlockSelected( clientId ),
-					// This ensures the "initialPosition" doesn't change when applying the template
-					// If we're supposed to focus the block, we'll focus the first inner block
-					// otherwise, we won't apply any auto-focus.
-					// This ensures for instance that the focus stays in the inserter when inserting the "buttons" block.
-					getSelectedBlocksInitialCaretPosition()
-				);
-			}
-		} );
-
-		return () => {
-			isCancelled = true;
-		};
+		}
 	}, [
 		template,
 		templateLock,

packages/block-editor/src/components/inner-blocks/use-nested-settings-update.js

@@ -16,6 +16,17 @@ import { getLayoutType } from '../../layouts';
 
 const pendingSettingsUpdates = new WeakMap();
 
+export function flushPendingNestedSettingsUpdates( registry ) {
+	const settings = pendingSettingsUpdates.get( registry );
+	if ( ! settings || ! Object.keys( settings ).length ) {
+		return;
+	}
+
+	const { updateBlockListSettings } = registry.dispatch( blockEditorStore );
+	updateBlockListSettings( settings );
+	pendingSettingsUpdates.set( registry, {} );
+}
+
 // Creates a memoizing caching function that remembers the last value and keeps returning it
 // as long as the new values are shallowly equal. Helps keep dependencies stable.
 function createShallowMemo() {
@@ -164,13 +175,7 @@ export default function useNestedSettingsUpdate(
 		}
 		pendingSettingsUpdates.get( registry )[ clientId ] = newSettings;
 		window.queueMicrotask( () => {
-			const settings = pendingSettingsUpdates.get( registry );
-			if ( Object.keys( settings ).length ) {
-				const { updateBlockListSettings } =
-					registry.dispatch( blockEditorStore );
-				updateBlockListSettings( settings );
-				pendingSettingsUpdates.set( registry, {} );
-			}
+			flushPendingNestedSettingsUpdates( registry );
 		} );
 	}, [
 		clientId,

packages/block-editor/src/components/provider/test/use-block-sync.js

@@ -1,12 +1,12 @@
 /**
  * WordPress dependencies
  */
-import { registerBlockType } from '@wordpress/blocks';
+import { createBlock, registerBlockType } from '@wordpress/blocks';
 
 /**
  * External dependencies
  */
-import { render } from '@testing-library/react';
+import { render, waitFor } from '@testing-library/react';
 
 /**
  * Internal dependencies
@@ -45,6 +45,11 @@ describe( 'useBlockSync hook', () => {
 				foo: { type: 'number' },
 			},
 		} );
+		registerBlockType( 'test/container-block', {
+			apiVersion: 3,
+			title: 'Test container block',
+			allowedBlocks: [ 'test/test-block' ],
+		} );
 	} );
 
 	afterEach( () => {
@@ -351,6 +356,59 @@ describe( 'useBlockSync hook', () => {
 		expect( onInput ).not.toHaveBeenCalled();
 	} );
 
+	it( 'merges history-ignored block changes into a deferred block addition', async () => {
+		const onChange = jest.fn();
+		const onInput = jest.fn();
+		let registry;
+		const setRegistry = ( reg ) => {
+			registry = reg;
+		};
+		render(
+			<TestWrapper
+				setRegistry={ setRegistry }
+				value={ [] }
+				onChange={ onChange }
+				onInput={ onInput }
+			/>
+		);
+		onChange.mockClear();
+		onInput.mockClear();
+
+		const parentBlock = createBlock( 'test/container-block' );
+		const templateBlock = createBlock( 'test/test-block', { foo: 2 } );
+
+		registry.dispatch( blockEditorStore ).insertBlock( parentBlock );
+		expect( onChange ).not.toHaveBeenCalled();
+		registry
+			.dispatch( blockEditorStore )
+			.__unstableMarkNextChangeAsNotPersistent( {
+				history: 'ignore',
+			} );
+		registry
+			.dispatch( blockEditorStore )
+			.replaceInnerBlocks( parentBlock.clientId, [ templateBlock ] );
+
+		await waitFor( () => expect( onChange ).toHaveBeenCalledTimes( 1 ) );
+
+		expect( onChange ).toHaveBeenCalledWith(
+			[
+				expect.objectContaining( {
+					clientId: parentBlock.clientId,
+					innerBlocks: [
+						expect.objectContaining( {
+							clientId: templateBlock.clientId,
+						} ),
+					],
+				} ),
+			],
+			expect.objectContaining( { selection: expect.any( Object ) } )
+		);
+		expect( onChange.mock.calls[ 0 ][ 1 ] ).not.toHaveProperty(
+			'undoIgnore'
+		);
+		expect( onInput ).not.toHaveBeenCalled();
+	} );
+
 	it( 'avoids updating the parent if there is a pending incoming change', async () => {
 		const replaceInnerBlocks = jest.spyOn(
 			blockEditorActions,

packages/block-editor/src/components/provider/use-block-sync.js

@@ -1,7 +1,7 @@
 /**
  * WordPress dependencies
  */
-import { useContext, useEffect, useRef } from '@wordpress/element';
+import { useContext, useEffect, useRef, useState } from '@wordpress/element';
 import { useRegistry } from '@wordpress/data';
 import { cloneBlock } from '@wordpress/blocks';
 
@@ -164,6 +164,12 @@ export default function useBlockSync( {
 	const pendingChangesRef = useRef( { incoming: null, outgoing: [] } );
 	const subscribedRef = useRef( false );
 
+	// Used to track deferred block syncs. The deferred payload is flushed from
+	// an effect so template insertion layout effects can update it first.
+	const deferredBlockSyncRef = useRef( null );
+	const [ deferredBlockSyncFlushSignal, setDeferredBlockSyncFlushSignal ] =
+		useState( 0 );
+
 	// Mapping between external (original) and internal (cloned) client IDs.
 	// This allows stable external IDs while using unique internal IDs.
 	const idMappingRef = useRef( {
@@ -301,6 +307,35 @@ export default function useBlockSync( {
 		onChangeRef.current = onChange;
 	}, [ onInput, onChange ] );
 
+	const flushDeferredBlockSync = () => {
+		if ( deferredBlockSyncRef.current ) {
+			const deferredBlockSync = deferredBlockSyncRef.current;
+
+			// Deferred block syncs are always persistent block changes, so use
+			// onChangeRef to persist.
+			deferredBlockSyncRef.current = null;
+			pendingChangesRef.current.outgoing.push( deferredBlockSync.blocks );
+			onChangeRef.current(
+				deferredBlockSync.blocks,
+				deferredBlockSync.options
+			);
+		}
+	};
+
+	const deferBlockSync = ( blocksToSync, options ) => {
+		deferredBlockSyncRef.current = {
+			blocks: blocksToSync,
+			options,
+		};
+		setDeferredBlockSyncFlushSignal( ( signal ) => signal + 1 );
+	};
+
+	useEffect( () => {
+		if ( deferredBlockSyncFlushSignal ) {
+			flushDeferredBlockSync();
+		}
+	}, [ deferredBlockSyncFlushSignal ] );
+
 	// Determine if blocks need to be reset when they change.
 	// Also restores selection from context after blocks are set.
 	useEffect( () => {
@@ -345,6 +380,7 @@ export default function useBlockSync( {
 			getSelectedBlocksInitialCaretPosition,
 			isLastBlockChangePersistent,
 			__unstableGetLastBlockChangeHistoryMode,
+			__unstableShouldLastBlockChangeDeferBlockSync,
 			__unstableIsLastBlockChangeIgnored,
 			areInnerBlocksControlled,
 			getBlockParents,
@@ -415,6 +451,9 @@ export default function useBlockSync( {
 				// receives both changes atomically.
 				registry.batch( () => {
 					if ( blocksChanged ) {
+						const shouldLastBlockChangeDeferBlockSync =
+							__unstableShouldLastBlockChangeDeferBlockSync();
+
 						isPersistent = newIsPersistent;
 						blockHistoryMode = newBlockHistoryMode;
 
@@ -439,20 +478,62 @@ export default function useBlockSync( {
 							  )
 							: selectionInfo;
 
-						pendingChangesRef.current.outgoing.push(
-							blocksForParent
-						);
-
-						const updateParent = isPersistent
-							? onChangeRef.current
-							: onInputRef.current;
 						const updateOptions = {
 							selection: selectionForParent,
 						};
 						if ( blockHistoryMode === 'ignore' ) {
 							updateOptions.undoIgnore = true;
 						}
-						updateParent( blocksForParent, updateOptions );
+
+						if ( deferredBlockSyncRef.current ) {
+							if (
+								! isPersistent &&
+								blockHistoryMode === 'ignore'
+							) {
+								// Non-persistent, history-ignored block changes
+								// can follow a deferred block addition before it flushes,
+								// such as inner template insertion. Update the
+								// deferred payload and return early. It will be
+								// flushed by a React effect after layout effects
+								// have run.
+								//
+								// This is used with template insertion in RTC to ensure
+								// operations like outer block and inner template insertions
+								// are merged into the same change for peers.
+								deferredBlockSyncRef.current = {
+									...deferredBlockSyncRef.current,
+									blocks: blocksForParent,
+									options: {
+										...deferredBlockSyncRef.current.options,
+										selection: selectionForParent,
+									},
+								};
+								return;
+							}
+
+							// A new unrelated block change is starting, so flush
+							// the deferred block sync before handling it.
+							flushDeferredBlockSync();
+						}
+
+						const shouldDeferBlockSync =
+							isPersistent && shouldLastBlockChangeDeferBlockSync;
+
+						if ( shouldDeferBlockSync ) {
+							// Block additions can trigger an inner template
+							// insertion during the same React commit.
+							// Defer this block so that block modifications
+							// can be merged before syncing to the parent.
+							deferBlockSync( blocksForParent, updateOptions );
+						} else {
+							pendingChangesRef.current.outgoing.push(
+								blocksForParent
+							);
+							const updateParent = isPersistent
+								? onChangeRef.current
+								: onInputRef.current;
+							updateParent( blocksForParent, updateOptions );
+						}
 					}
 
 					if (
@@ -508,6 +589,9 @@ export default function useBlockSync( {
 
 	useEffect( () => {
 		return () => {
+			if ( deferredBlockSyncRef.current ) {
+				flushDeferredBlockSync();
+			}
 			unsetControlledBlocks();
 		};
 	}, [] );