RTC: Fix divergence in duplicate table rows when one collaborator edits one duplicate while another collaborator deletes the other duplicate

packages/block-library/src/table/state.js

@@ -94,6 +94,7 @@ export function updateSelectedCell( state, selection, updateCell ) {
 					}
 
 					return {
+						...row,
 						cells: row.cells.map(
 							( cellAttributes, columnIndex ) => {
 								const cellLocation = {
@@ -248,6 +249,7 @@ export function insertColumn( state, { columnIndex } ) {
 					}
 
 					return {
+						...row,
 						cells: [
 							...row.cells.slice( 0, columnIndex ),
 							{
@@ -290,6 +292,7 @@ export function deleteColumn( state, { columnIndex } ) {
 				sectionName,
 				section
 					.map( ( row ) => ( {
+						...row,
 						cells:
 							row.cells.length >= columnIndex
 								? row.cells.filter(

packages/block-library/src/table/test/state.js

@@ -466,6 +466,30 @@ describe( 'insertColumn', () => {
 		expect( state ).toEqual( expected );
 	} );
 
+	it( 'preserves symbol row and cell properties when inserting a column', () => {
+		const syncId = Symbol( 'syncId' );
+		const tableWithSymbolIdentity = deepFreeze( {
+			body: [
+				{
+					[ syncId ]: 'row-1',
+					cells: [
+						{
+							[ syncId ]: 'cell-1',
+							content: 'test',
+							tag: 'td',
+						},
+					],
+				},
+			],
+		} );
+		const state = insertColumn( tableWithSymbolIdentity, {
+			columnIndex: 0,
+		} );
+
+		expect( state.body[ 0 ][ syncId ] ).toBe( 'row-1' );
+		expect( state.body[ 0 ].cells[ 1 ][ syncId ] ).toBe( 'cell-1' );
+	} );
+
 	it( 'adds `th` cells to the head', () => {
 		const state = insertColumn( tableWithHead, {
 			columnIndex: 1,
@@ -774,6 +798,34 @@ describe( 'deleteColumn', () => {
 		expect( state ).toEqual( expected );
 	} );
 
+	it( 'preserves symbol row and cell properties when deleting a column', () => {
+		const syncId = Symbol( 'syncId' );
+		const tableWithSymbolIdentity = deepFreeze( {
+			body: [
+				{
+					[ syncId ]: 'row-1',
+					cells: [
+						{
+							content: 'remove',
+							tag: 'td',
+						},
+						{
+							[ syncId ]: 'cell-2',
+							content: 'keep',
+							tag: 'td',
+						},
+					],
+				},
+			],
+		} );
+		const state = deleteColumn( tableWithSymbolIdentity, {
+			columnIndex: 0,
+		} );
+
+		expect( state.body[ 0 ][ syncId ] ).toBe( 'row-1' );
+		expect( state.body[ 0 ].cells[ 0 ][ syncId ] ).toBe( 'cell-2' );
+	} );
+
 	it( 'should delete all rows when only one column present', () => {
 		const tableWithOneColumn = {
 			body: [
@@ -1306,6 +1358,73 @@ describe( 'updateSelectedCell', () => {
 		} );
 	} );
 
+	it( 'preserves unknown row properties when updating a cell', () => {
+		const tableWithRowIdentity = deepFreeze( {
+			body: [
+				{
+					__unstableSyncId: 'row-1',
+					cells: [
+						{
+							content: '',
+							tag: 'td',
+						},
+					],
+				},
+			],
+		} );
+		const cellSelection = {
+			type: 'cell',
+			sectionName: 'body',
+			rowIndex: 0,
+			columnIndex: 0,
+		};
+		const updated = updateSelectedCell(
+			tableWithRowIdentity,
+			cellSelection,
+			( cell ) => ( {
+				...cell,
+				content: 'test',
+			} )
+		);
+
+		expect( updated.body[ 0 ].__unstableSyncId ).toBe( 'row-1' );
+	} );
+
+	it( 'preserves symbol row and cell properties when updating a cell', () => {
+		const syncId = Symbol( 'syncId' );
+		const tableWithSymbolIdentity = deepFreeze( {
+			body: [
+				{
+					[ syncId ]: 'row-1',
+					cells: [
+						{
+							[ syncId ]: 'cell-1',
+							content: '',
+							tag: 'td',
+						},
+					],
+				},
+			],
+		} );
+		const cellSelection = {
+			type: 'cell',
+			sectionName: 'body',
+			rowIndex: 0,
+			columnIndex: 0,
+		};
+		const updated = updateSelectedCell(
+			tableWithSymbolIdentity,
+			cellSelection,
+			( cell ) => ( {
+				...cell,
+				content: 'test',
+			} )
+		);
+
+		expect( updated.body[ 0 ][ syncId ] ).toBe( 'row-1' );
+		expect( updated.body[ 0 ].cells[ 0 ][ syncId ] ).toBe( 'cell-1' );
+	} );
+
 	it( 'updates every cell in the column when the selection type is `column`', () => {
 		const cellSelection = { type: 'column', columnIndex: 1 };
 		const updated = updateSelectedCell(

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

@@ -61,6 +61,8 @@ export type YBlocks = Y.Array< YBlock >;
 // Attribute values will be typed as the union of `Y.Text` and `unknown`.
 export type YBlockAttributes = Y.Map< Y.Text | unknown >;
 
+const ARRAY_ELEMENT_ID_KEY = '__unstableSyncId';
+const ARRAY_ELEMENT_ID_SYMBOL = Symbol( 'wpSyncArrayElementId' );
 const serializableBlocksCache = new WeakMap< WeakKey, Block[] >();
 
 /**
@@ -84,10 +86,20 @@ function serializeAttributeValue( value: unknown ): unknown {
 	// e.g. a single row inside core/table `body`: { cells: [ ... ] }
 	if ( value && typeof value === 'object' ) {
 		const result: Record< string, unknown > = {};
+		const arrayElementId = getArrayElementId( value );
 
 		for ( const [ k, v ] of Object.entries( value ) ) {
+			if ( k === ARRAY_ELEMENT_ID_KEY ) {
+				continue;
+			}
+
 			result[ k ] = serializeAttributeValue( v );
 		}
+
+		if ( arrayElementId ) {
+			result[ ARRAY_ELEMENT_ID_KEY ] = arrayElementId;
+		}
+
 		return result;
 	}
 
@@ -150,16 +162,25 @@ function deserializeAttributeValue(
 	// e.g. a single row inside core/table `body`: { cells: [ ... ] }
 	if ( value && typeof value === 'object' ) {
 		const result: Record< string, unknown > = {};
+		const arrayElementId = getArrayElementId( value );
 
 		for ( const [ key, innerValue ] of Object.entries(
 			value as Record< string, unknown >
 		) ) {
+			if ( key === ARRAY_ELEMENT_ID_KEY ) {
+				continue;
+			}
+
 			result[ key ] = deserializeAttributeValue(
 				schema?.query?.[ key ],
 				innerValue
 			);
 		}
 
+		if ( arrayElementId ) {
+			defineArrayElementId( result, arrayElementId );
+		}
+
 		return result;
 	}
 
@@ -329,12 +350,15 @@ function createYMapFromQuery(
 		return new Y.Map();
 	}
 
-	const entries: [ string, unknown ][] = Object.entries( obj ).map(
-		( [ key, val ] ): [ string, unknown ] => {
+	const arrayElementId = getArrayElementId( obj ) ?? uuidv4();
+	const entries: [ string, unknown ][] = Object.entries( obj )
+		.filter( ( [ key ] ) => key !== ARRAY_ELEMENT_ID_KEY )
+		.map( ( [ key, val ] ): [ string, unknown ] => {
 			const subSchema = query[ key ];
 			return [ key, createYValueFromSchema( subSchema, val ) ];
-		}
-	);
+		} );
+
+	entries.push( [ ARRAY_ELEMENT_ID_KEY, arrayElementId ] );
 
 	return new Y.Map( entries );
 }
@@ -594,10 +618,122 @@ function areArrayElementsEqual(
 	yElement: unknown
 ): boolean {
 	if ( yElement instanceof Y.Map && isRecord( newElement ) ) {
-		return fastDeepEqual( newElement, yElement.toJSON() );
+		return fastDeepEqual(
+			stripArrayElementIds( newElement ),
+			stripArrayElementIds( yElement.toJSON() )
+		);
+	}
+
+	return fastDeepEqual(
+		stripArrayElementIds( newElement ),
+		stripArrayElementIds( yElement )
+	);
+}
+
+function getArrayElementId( value: unknown ): string | undefined {
+	if ( value instanceof Y.Map ) {
+		const id = value.get( ARRAY_ELEMENT_ID_KEY );
+		return typeof id === 'string' ? id : undefined;
+	}
+
+	if ( isRecord( value ) ) {
+		const id = value[ ARRAY_ELEMENT_ID_KEY ];
+		if ( typeof id === 'string' ) {
+			return id;
+		}
+
+		const symbolId = ( value as Record< symbol, unknown > )[
+			ARRAY_ELEMENT_ID_SYMBOL
+		];
+		return typeof symbolId === 'string' ? symbolId : undefined;
+	}
+
+	return undefined;
+}
+
+function defineArrayElementId(
+	value: Record< string, unknown >,
+	id: string
+): void {
+	Object.defineProperty( value, ARRAY_ELEMENT_ID_SYMBOL, {
+		configurable: true,
+		enumerable: true,
+		value: id,
+	} );
+}
+
+function stripArrayElementIds( value: unknown ): unknown {
+	if ( Array.isArray( value ) ) {
+		return value.map( stripArrayElementIds );
+	}
+
+	if ( isRecord( value ) ) {
+		return Object.fromEntries(
+			Object.entries( value )
+				.filter( ( [ key ] ) => key !== ARRAY_ELEMENT_ID_KEY )
+				.map( ( [ key, innerValue ] ) => [
+					key,
+					stripArrayElementIds( innerValue ),
+				] )
+		);
+	}
+
+	return value;
+}
+
+function mergeYArrayByElementIds(
+	yArray: Y.Array< unknown >,
+	newValue: unknown[],
+	query: Record< string, BlockAttributeSchema >,
+	cursorPosition: number | null
+): boolean {
+	if ( ! newValue.some( getArrayElementId ) ) {
+		return false;
+	}
+
+	let index = 0;
+
+	for ( const newElement of newValue ) {
+		const newId = getArrayElementId( newElement );
+		let currentIndex = -1;
+
+		if ( newId ) {
+			for ( let i = index; i < yArray.length; i++ ) {
+				if ( getArrayElementId( yArray.get( i ) ) === newId ) {
+					currentIndex = i;
+					break;
+				}
+			}
+		}
+
+		if ( currentIndex > index ) {
+			yArray.delete( index, currentIndex - index );
+		}
+
+		if ( currentIndex >= index ) {
+			const currentElement = yArray.get( index );
+			if ( currentElement instanceof Y.Map && isRecord( newElement ) ) {
+				mergeYMapValues(
+					currentElement,
+					newElement,
+					query,
+					cursorPosition
+				);
+			}
+		} else {
+			yArray.insert( index, [
+				createYMapFromQuery( query, newElement ),
+			] );
+		}
+
+		index++;
+	}
+
+	if ( yArray.length > index ) {
+		yArray.delete( index, yArray.length - index );
 	}
 
-	return fastDeepEqual( newElement, yElement );
+	return true;
 }
 
 /**
@@ -625,6 +761,11 @@ function mergeYArray(
 	}
 
 	const query = schema.query;
+
+	if ( mergeYArrayByElementIds( yArray, newValue, query, cursorPosition ) ) {
+		return;
+	}
+
 	const numOfCommonEntries = Math.min( newValue.length, yArray.length );
 
 	let left = 0;
@@ -786,7 +927,7 @@ function mergeYMapValues(
 
 	// Delete properties absent from the incoming object.
 	for ( const key of yMap.keys() ) {
-		if ( ! Object.hasOwn( newObj, key ) ) {
+		if ( key !== ARRAY_ELEMENT_ID_KEY && ! Object.hasOwn( newObj, key ) ) {
 			yMap.delete( key );
 		}
 	}