diff --git a/packages/core-data/src/entities.js b/packages/core-data/src/entities.js index 75cc7fc34faa40..3623bd8a734d9f 100644 --- a/packages/core-data/src/entities.js +++ b/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: { diff --git a/packages/core-data/src/utils/crdt.ts b/packages/core-data/src/utils/crdt.ts index 59f0c9439385c8..4278b2109c29b7 100644 --- a/packages/core-data/src/utils/crdt.ts +++ b/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`). diff --git a/packages/core-data/src/utils/test/crdt.ts b/packages/core-data/src/utils/test/crdt.ts index d14af7ccc68d23..d8c1a87dc4c5c1 100644 --- a/packages/core-data/src/utils/test/crdt.ts +++ b/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; diff --git a/packages/sync/src/manager.ts b/packages/sync/src/manager.ts index 18b5f9982b0148..dcca0c5d9970e6 100644 --- a/packages/sync/src/manager.ts +++ b/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 } ); diff --git a/packages/sync/src/test/manager.ts b/packages/sync/src/test/manager.ts index c3bd3d5595b1f1..75f93f0a9f8470 100644 --- a/packages/sync/src/test/manager.ts +++ b/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. diff --git a/packages/sync/src/types.ts b/packages/sync/src/types.ts index c4b3bb2435e74f..0c3f3bf8c65782 100644 --- a/packages/sync/src/types.ts +++ b/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 {