RTC: fix stale delete propagation issue (symptom: delete causes temporary divergence between clients, which heals to incorrect result with deleted content getting re-added)

packages/core-data/src/actions.js

@@ -462,7 +462,10 @@ export const editEntityRecord =
 				objectId,
 				editsWithMerges,
 				origin,
-				{ isNewUndoLevel }
+				{
+					baseRecord: options.baseRecord,
+					isNewUndoLevel,
+				}
 			);
 		}
 		if ( ! options.undoIgnore ) {

packages/core-data/src/entities.js

@@ -412,10 +412,24 @@ async function loadPostTypeEntities() {
 			 *
 			 * @param {import('@wordpress/sync').CRDTDoc}               crdtDoc
 			 * @param {Partial< import('@wordpress/sync').ObjectData >} changes
+			 * @param {Object}                                          options
 			 * @return {void}
 			 */
-			applyChangesToCRDTDoc: ( crdtDoc, changes ) =>
-				applyPostChangesToCRDTDoc( crdtDoc, changes, syncedProperties ),
+			applyChangesToCRDTDoc: ( crdtDoc, changes, options ) => {
+				if ( options ) {
+					return applyPostChangesToCRDTDoc(
+						crdtDoc,
+						changes,
+						syncedProperties,
+						options
+					);
+				}
+				return applyPostChangesToCRDTDoc(
+					crdtDoc,
+					changes,
+					syncedProperties
+				);
+			},
 
 			/**
 			 * Create the awareness instance for the entity's CRDT document.

packages/core-data/src/hooks/use-entity-block-editor.js

@@ -1,7 +1,12 @@
 /**
  * WordPress dependencies
  */
-import { useCallback, useMemo } from '@wordpress/element';
+import {
+	useCallback,
+	useLayoutEffect,
+	useMemo,
+	useRef,
+} from '@wordpress/element';
 import { useDispatch, useSelect } from '@wordpress/data';
 import { parse, __unstableSerializeAndClean } from '@wordpress/blocks';
 
@@ -82,6 +87,10 @@ export default function useEntityBlockEditor( kind, name, { id: _id } = {} ) {
 
 		return _blocks;
 	}, [ kind, name, id, editedBlocks, content ] );
+	const blocksRef = useRef( blocks );
+	useLayoutEffect( () => {
+		blocksRef.current = blocks;
+	}, [ blocks ] );
 
 	const onChange = useCallback(
 		( newBlocks, options ) => {
@@ -102,6 +111,7 @@ export default function useEntityBlockEditor( kind, name, { id: _id } = {} ) {
 			};
 
 			editEntityRecord( kind, name, id, edits, {
+				baseRecord: { blocks },
 				isCached: false,
 				...rest,
 			} );
@@ -126,11 +136,12 @@ export default function useEntityBlockEditor( kind, name, { id: _id } = {} ) {
 			};
 
 			editEntityRecord( kind, name, id, edits, {
+				baseRecord: { blocks: blocksRef.current },
 				isCached: true,
 				...rest,
 			} );
 		},
-		[ kind, name, id, meta, editEntityRecord ]
+		[ kind, name, id, meta, editEntityRecord, blocksRef ]
 	);
 
 	return [ blocks, onInput, onChange ];

packages/core-data/src/utils/crdt-blocks.ts

@@ -77,6 +77,10 @@ export type YBlockAttributes = Y.Map< Y.Text | unknown >;
  */
 export type MergeCursorPosition = WPBlockSelection | null;
 
+export interface MergeCrdtBlocksOptions {
+	baseBlocks?: Block[];
+}
+
 const serializableBlocksCache = new WeakMap< WeakKey, Block[] >();
 
 /**
@@ -157,6 +161,185 @@ function makeBlocksSerializable( blocks: Block[] ): Block[] {
 	} );
 }
 
+function getBlockClientId( block: Block ): string | undefined {
+	return typeof block.clientId === 'string' && block.clientId
+		? block.clientId
+		: undefined;
+}
+
+function getClientIdsIfEveryBlockHasUniqueId(
+	blocks: Block[]
+): string[] | undefined {
+	const clientIds = blocks.map( getBlockClientId );
+
+	if ( clientIds.some( ( clientId ) => ! clientId ) ) {
+		return undefined;
+	}
+
+	const uniqueClientIds = new Set( clientIds );
+
+	return uniqueClientIds.size === clientIds.length
+		? ( clientIds as string[] )
+		: undefined;
+}
+
+function getBlocksByClientId( blocks: Block[] ): Map< string, Block > {
+	return new Map(
+		blocks.map( ( block ) => [
+			getBlockClientId( block ) as string,
+			block,
+		] )
+	);
+}
+
+function haveSharedClientId(
+	firstClientIds: string[],
+	secondClientIds: string[]
+): boolean {
+	const secondClientIdSet = new Set( secondClientIds );
+	return firstClientIds.some( ( clientId ) =>
+		secondClientIdSet.has( clientId )
+	);
+}
+
+function findBlockIndexByClientId( blocks: Block[], clientId: string ): number {
+	return blocks.findIndex(
+		( block ) => getBlockClientId( block ) === clientId
+	);
+}
+
+function getRemoteBlockInsertIndex(
+	currentBlocks: Block[],
+	currentIndex: number,
+	blocksToSync: Block[],
+	blockIdsToSync: Set< string >
+): number {
+	for ( let index = currentIndex - 1; index >= 0; index-- ) {
+		const previousClientId = getBlockClientId( currentBlocks[ index ] );
+
+		if ( previousClientId && blockIdsToSync.has( previousClientId ) ) {
+			const previousIndex = findBlockIndexByClientId(
+				blocksToSync,
+				previousClientId
+			);
+
+			return previousIndex === -1
+				? blocksToSync.length
+				: previousIndex + 1;
+		}
+	}
+
+	for (
+		let index = currentIndex + 1;
+		index < currentBlocks.length;
+		index++
+	) {
+		const nextClientId = getBlockClientId( currentBlocks[ index ] );
+
+		if ( nextClientId && blockIdsToSync.has( nextClientId ) ) {
+			const nextIndex = findBlockIndexByClientId(
+				blocksToSync,
+				nextClientId
+			);
+
+			return nextIndex === -1 ? blocksToSync.length : nextIndex;
+		}
+	}
+
+	return blocksToSync.length;
+}
+
+function reconcileBlockWithBase(
+	localBlock: Block,
+	currentBlock: Block,
+	baseBlock: Block
+): Block {
+	const innerBlocks = reconcileBlocksWithBase(
+		localBlock.innerBlocks ?? [],
+		currentBlock.innerBlocks ?? [],
+		baseBlock.innerBlocks ?? []
+	);
+
+	return innerBlocks === localBlock.innerBlocks
+		? localBlock
+		: { ...localBlock, innerBlocks };
+}
+
+function reconcileBlocksWithBase(
+	localBlocksToSync: Block[],
+	currentBlocks: Block[],
+	baseBlocks: Block[]
+): Block[] {
+	const localClientIds =
+		getClientIdsIfEveryBlockHasUniqueId( localBlocksToSync );
+	const currentClientIds =
+		getClientIdsIfEveryBlockHasUniqueId( currentBlocks );
+	const baseClientIds = getClientIdsIfEveryBlockHasUniqueId( baseBlocks );
+
+	if ( ! localClientIds || ! currentClientIds || ! baseClientIds ) {
+		return localBlocksToSync;
+	}
+
+	// Save/reload can regenerate editor client IDs while the CRDT document
+	// still has the old IDs. In that state, ID-based stale-delete rebasing
+	// would treat every local block as remotely deleted.
+	if (
+		baseClientIds.length > 0 &&
+		currentClientIds.length > 0 &&
+		! haveSharedClientId( baseClientIds, currentClientIds )
+	) {
+		return localBlocksToSync;
+	}
+
+	const currentBlocksByClientId = getBlocksByClientId( currentBlocks );
+	const baseBlocksByClientId = getBlocksByClientId( baseBlocks );
+	const localClientIdSet = new Set( localClientIds );
+	const baseClientIdSet = new Set( baseClientIds );
+	const blocksToSync: Block[] = [];
+	const blockIdsToSync = new Set< string >();
+
+	localBlocksToSync.forEach( ( localBlock ) => {
+		const clientId = getBlockClientId( localBlock ) as string;
+		const currentBlock = currentBlocksByClientId.get( clientId );
+		const baseBlock = baseBlocksByClientId.get( clientId );
+
+		if ( baseBlock && ! currentBlock ) {
+			return;
+		}
+
+		const blockToSync =
+			baseBlock && currentBlock
+				? reconcileBlockWithBase( localBlock, currentBlock, baseBlock )
+				: localBlock;
+
+		blocksToSync.push( blockToSync );
+		blockIdsToSync.add( clientId );
+	} );
+
+	currentBlocks.forEach( ( currentBlock, currentIndex ) => {
+		const clientId = getBlockClientId( currentBlock ) as string;
+
+		if (
+			baseClientIdSet.has( clientId ) ||
+			localClientIdSet.has( clientId ) ||
+			blockIdsToSync.has( clientId )
+		) {
+			return;
+		}
+
+		const insertIndex = getRemoteBlockInsertIndex(
+			currentBlocks,
+			currentIndex,
+			blocksToSync,
+			blockIdsToSync
+		);
+		blocksToSync.splice( insertIndex, 0, currentBlock );
+		blockIdsToSync.add( clientId );
+	} );
+
+	return blocksToSync;
+}
+
 /**
  * Recursively walk an attribute value and convert any strings that correspond
  * to rich-text schema nodes into RichTextData instances. This is the inverse
@@ -415,16 +598,19 @@ function createNewYBlock( block: Block ): YBlock {
  * Merge incoming block data into the local Y.Doc.
  * This function is called to sync local block changes to a shared Y.Doc.
  *
- * @param yblocks         The blocks in the local Y.Doc.
- * @param incomingBlocks  Gutenberg blocks being synced.
- * @param attributeCursor When provided, describes a selection cursor falling within a
- *                        RichText field associated with a specific block and attribute.
- *                        Derived from the changes that produced the blocks.
+ * @param yblocks            The blocks in the local Y.Doc.
+ * @param incomingBlocks     Gutenberg blocks being synced.
+ * @param attributeCursor    When provided, describes a selection cursor falling within a
+ *                           RichText field associated with a specific block and attribute.
+ *                           Derived from the changes that produced the blocks.
+ * @param options            Merge options.
+ * @param options.baseBlocks Pre-change block snapshot to rebase against.
  */
 export function mergeCrdtBlocks(
 	yblocks: YBlocks,
 	incomingBlocks: Block[],
-	attributeCursor: MergeCursorPosition
+	attributeCursor: MergeCursorPosition,
+	options: MergeCrdtBlocksOptions = {}
 ): void {
 	// Ensure we are working with serializable block data.
 	if ( ! serializableBlocksCache.has( incomingBlocks ) ) {
@@ -434,8 +620,18 @@ export function mergeCrdtBlocks(
 		);
 	}
 
-	const incomingBlocksToSync =
+	const localBlocksToSync =
 		serializableBlocksCache.get( incomingBlocks ) ?? [];
+	const baseBlocksToSync = options.baseBlocks
+		? makeBlocksSerializable( options.baseBlocks )
+		: undefined;
+	const incomingBlocksToSync = baseBlocksToSync
+		? reconcileBlocksWithBase(
+				localBlocksToSync,
+				yblocks.toJSON() as Block[],
+				baseBlocksToSync
+		  )
+		: localBlocksToSync;
 
 	// This is a rudimentary diff implementation similar to the y-prosemirror diffing
 	// approach.

packages/core-data/src/utils/crdt.ts

@@ -24,6 +24,7 @@ import {
 	type Block,
 	deserializeBlockAttributes,
 	mergeCrdtBlocks,
+	type MergeCrdtBlocksOptions,
 	type MergeCursorPosition,
 	mergeRichTextUpdate,
 	type YBlock,
@@ -55,6 +56,10 @@ export type PostChanges = Partial< Post > & {
 	title?: Post[ 'title' ] | string;
 };
 
+interface ApplyChangesToCRDTDocOptions {
+	baseRecord?: Partial< ObjectData >;
+}
+
 // A post record as represented in the CRDT document (Y.Map).
 export interface YPostRecord extends YMapRecord {
 	author: number;
@@ -122,12 +127,15 @@ function defaultApplyChangesToCRDTDoc(
  * @param {CRDTDoc}     ydoc
  * @param {PostChanges} changes
  * @param {Set<string>} syncedProperties
+ * @param {Object}      options
+ * @param {ObjectData}  options.baseRecord
  * @return {void}
  */
 export function applyPostChangesToCRDTDoc(
 	ydoc: CRDTDoc,
 	changes: PostChanges,
-	syncedProperties: Set< string >
+	syncedProperties: Set< string >,
+	options: ApplyChangesToCRDTDocOptions = {}
 ): void {
 	const ymap = getRootMap< YPostRecord >( ydoc, CRDT_RECORD_MAP_KEY );
 
@@ -169,7 +177,21 @@ export function applyPostChangesToCRDTDoc(
 
 				// Merge blocks does not need `setValue` because it is operating on a
 				// Yjs type that is already in the Y.Doc.
-				mergeCrdtBlocks( currentBlocks, newValue, newCursorPosition );
+				const mergeOptions: MergeCrdtBlocksOptions = {};
+				const baseBlocks = (
+					options.baseRecord as PostChanges | undefined
+				 )?.blocks;
+
+				if ( Array.isArray( baseBlocks ) ) {
+					mergeOptions.baseBlocks = baseBlocks;
+				}
+
+				mergeCrdtBlocks(
+					currentBlocks,
+					newValue,
+					newCursorPosition,
+					mergeOptions
+				);
 				break;
 			}