RTC: Add optional shouldSync function to entity sync config

packages/core-data/src/entities.js

@@ -17,6 +17,7 @@ import { PostEditorAwareness } from './awareness/post-editor-awareness';
 import { getSyncManager } from './sync';
 import {
 	applyPostChangesToCRDTDoc,
+	defaultCollectionSyncConfig,
 	defaultSyncConfig,
 	getPostChangesFromCRDTDoc,
 	POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE,
@@ -141,6 +142,7 @@ export const rootEntitiesConfig = [
 		plural: 'comments',
 		label: __( 'Comment' ),
 		supportsPagination: true,
+		syncConfig: defaultCollectionSyncConfig,
 	},
 	{
 		name: 'menu',
@@ -237,14 +239,7 @@ export const rootEntitiesConfig = [
 		plural: 'icons',
 		key: 'name',
 	},
-].map( ( entity ) => {
-	const syncEnabledRootEntities = new Set( [ 'comment' ] );
-
-	if ( syncEnabledRootEntities.has( entity.name ) ) {
-		entity.syncConfig = defaultSyncConfig;
-	}
-	return entity;
-} );
+];
 
 export const deprecatedEntities = {
 	root: {

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

@@ -11,6 +11,8 @@ import { __unstableSerializeAndClean } from '@wordpress/blocks';
 import {
 	type CRDTDoc,
 	type ObjectData,
+	type ObjectID,
+	type ObjectType,
 	type SyncConfig,
 	Y,
 } from '@wordpress/sync';
@@ -428,6 +430,19 @@ export const defaultSyncConfig: SyncConfig = {
 	getChangesFromCRDTDoc: defaultGetChangesFromCRDTDoc,
 };
 
+/**
+ * This default collection sync config can be used to sync entity collections
+ * (e.g., block comments) where we are not interested in merging changes at the
+ * individual record level, but instead want to replace the entire collection
+ * when changes are detected.
+ */
+export const defaultCollectionSyncConfig: SyncConfig = {
+	applyChangesToCRDTDoc: () => {},
+	getChangesFromCRDTDoc: () => ( {} ),
+	shouldSync: ( _: ObjectType, objectId: ObjectID | null ) =>
+		null === objectId,
+};
+
 /**
  * Extract the raw string value from a property that may be a string or an object
  * with a `raw` property (`RenderedText`).

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

@@ -57,6 +57,7 @@ import { RichTextData } from '@wordpress/rich-text';
 import { CRDT_RECORD_MAP_KEY } from '../../sync';
 import {
 	applyPostChangesToCRDTDoc,
+	defaultCollectionSyncConfig,
 	getPostChangesFromCRDTDoc,
 	POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE,
 	type PostChanges,
@@ -82,6 +83,43 @@ const defaultSyncedProperties = new Set< string >( [
 	'title',
 ] );
 
+describe( 'defaultCollectionSyncConfig', () => {
+	it( 'has no-op applyChangesToCRDTDoc', () => {
+		const doc = new Y.Doc();
+		// Should not throw and return undefined.
+		expect(
+			defaultCollectionSyncConfig.applyChangesToCRDTDoc( doc, {
+				title: 'test',
+			} )
+		).toBeUndefined();
+		doc.destroy();
+	} );
+
+	it( 'has getChangesFromCRDTDoc that returns empty object', () => {
+		const doc = new Y.Doc();
+		const result = defaultCollectionSyncConfig.getChangesFromCRDTDoc( doc, {
+			title: 'test',
+		} );
+		expect( result ).toEqual( {} );
+		doc.destroy();
+	} );
+
+	it( 'shouldSync returns true when objectId is null (collection)', () => {
+		expect(
+			defaultCollectionSyncConfig.shouldSync?.( 'comment', null )
+		).toBe( true );
+	} );
+
+	it( 'shouldSync returns false when objectId is provided (individual record)', () => {
+		expect(
+			defaultCollectionSyncConfig.shouldSync?.( 'comment', '123' )
+		).toBe( false );
+		expect(
+			defaultCollectionSyncConfig.shouldSync?.( 'comment', 'foo' )
+		).toBe( false );
+	} );
+} );
+
 describe( 'crdt', () => {
 	let doc: Y.Doc;
 

packages/sync/src/manager.ts

@@ -170,6 +170,11 @@ export function createSyncManager( debug = false ): SyncManager {
 			return; // Already bootstrapped.
 		}
 
+		if ( false === syncConfig.shouldSync?.( objectType, objectId ) ) {
+			log( 'loadEntity', 'shouldSync false, skipping', entityId );
+			return; // Sync config indicates that this entity should not be synced.
+		}
+
 		log( 'loadEntity', 'loading', entityId );
 
 		handlers = {
@@ -317,6 +322,11 @@ export function createSyncManager( debug = false ): SyncManager {
 			return; // Already loaded.
 		}
 
+		if ( false === syncConfig.shouldSync?.( objectType, null ) ) {
+			log( 'loadCollection', 'shouldSync false, skipping', entityId );
+			return; // Sync config indicates that this entity should not be synced.
+		}
+
 		log( 'loadCollection', 'loading', entityId );
 
 		const ydoc = createYjsDoc( { collection: true, objectType } );

packages/sync/src/test/manager.ts

@@ -587,6 +587,138 @@ describe( 'SyncManager', () => {
 		} );
 	} );
 
+	describe( 'shouldSync', () => {
+		it( 'skips loading entity when shouldSync returns false', async () => {
+			const manager = createSyncManager();
+
+			mockSyncConfig.shouldSync = jest.fn( () => false );
+
+			await manager.load(
+				mockSyncConfig,
+				'post',
+				'123',
+				mockRecord,
+				mockHandlers
+			);
+
+			expect( mockSyncConfig.shouldSync ).toHaveBeenCalledWith(
+				'post',
+				'123'
+			);
+			expect(
+				mockSyncConfig.applyChangesToCRDTDoc
+			).not.toHaveBeenCalled();
+			expect( mockProviderCreator ).not.toHaveBeenCalled();
+		} );
+
+		it( 'loads entity when shouldSync returns true', async () => {
+			const manager = createSyncManager();
+
+			mockSyncConfig.shouldSync = jest.fn( () => true );
+
+			await manager.load(
+				mockSyncConfig,
+				'post',
+				'123',
+				mockRecord,
+				mockHandlers
+			);
+
+			expect( mockSyncConfig.shouldSync ).toHaveBeenCalledWith(
+				'post',
+				'123'
+			);
+			expect(
+				mockSyncConfig.applyChangesToCRDTDoc
+			).toHaveBeenCalledTimes( 1 );
+			expect( mockProviderCreator ).toHaveBeenCalledTimes( 1 );
+		} );
+
+		it( 'loads entity when shouldSync is not defined', async () => {
+			const manager = createSyncManager();
+
+			delete mockSyncConfig.shouldSync;
+
+			await manager.load(
+				mockSyncConfig,
+				'post',
+				'123',
+				mockRecord,
+				mockHandlers
+			);
+
+			expect(
+				mockSyncConfig.applyChangesToCRDTDoc
+			).toHaveBeenCalledTimes( 1 );
+			expect( mockProviderCreator ).toHaveBeenCalledTimes( 1 );
+		} );
+
+		it( 'skips loading collection when shouldSync returns false', async () => {
+			const manager = createSyncManager();
+
+			mockSyncConfig.shouldSync = jest.fn( () => false );
+
+			const mockCollectionHandlers = {
+				onStatusChange: jest.fn(),
+				refetchRecords: jest.fn( async () => Promise.resolve() ),
+			};
+
+			await manager.loadCollection(
+				mockSyncConfig,
+				'comment',
+				mockCollectionHandlers
+			);
+
+			expect( mockSyncConfig.shouldSync ).toHaveBeenCalledWith(
+				'comment',
+				null
+			);
+			expect( mockProviderCreator ).not.toHaveBeenCalled();
+		} );
+
+		it( 'loads collection when shouldSync returns true', async () => {
+			const manager = createSyncManager();
+
+			mockSyncConfig.shouldSync = jest.fn( () => true );
+
+			const mockCollectionHandlers = {
+				onStatusChange: jest.fn(),
+				refetchRecords: jest.fn( async () => Promise.resolve() ),
+			};
+
+			await manager.loadCollection(
+				mockSyncConfig,
+				'comment',
+				mockCollectionHandlers
+			);
+
+			expect( mockSyncConfig.shouldSync ).toHaveBeenCalledWith(
+				'comment',
+				null
+			);
+			expect( mockProviderCreator ).toHaveBeenCalledTimes( 1 );
+		} );
+
+		it( 'loads collection when shouldSync is not defined', async () => {
+			const manager = createSyncManager();
+
+			delete mockSyncConfig.shouldSync;
+
+			const mockCollectionHandlers = {
+				onStatusChange: jest.fn(),
+				refetchRecords: jest.fn( async () => Promise.resolve() ),
+			};
+
+			await manager.loadCollection(
+				mockSyncConfig,
+				'comment',
+				mockCollectionHandlers
+			);
+
+			expect( mockProviderCreator ).toHaveBeenCalledTimes( 1 );
+		} );
+	} );
+
 	describe( 'CRDT doc observation', () => {
 		it( 'edits the local entity record when remote updates arrive', async () => {
 			// Capture the Y.Doc from provider creator.

packages/sync/src/types.ts

@@ -144,6 +144,10 @@ export interface SyncConfig {
 		editedRecord: ObjectData
 	) => ObjectData;
 	getPersistedCRDTDoc?: ( record: ObjectData ) => string | null;
+	shouldSync?: (
+		objectType: ObjectType,
+		objectId: ObjectID | null
+	) => boolean;
 }
 
 export interface SyncManager {