Skip to content

Commit 22e067b

Browse files
ingeniumedchriszarate
ingeniumed
and
chriszarate
authored
Real-time Collaboration: Use Y.text for title, content and excerpt (#75448)
* Revert change to restoreRevision Reverts this change: #75233 Reverts to: https://github.com/WordPress/gutenberg/blob/8bb681001d7267ea57cceed90d2e3650c6349e2a/packages/editor/src/store/private-actions.js#L619 * Sync post content and undefined blocks value * Add unit tests * Convert content, title and excerpt to Y.Text * Delete the key if its not the right format * No more deleting when undefined, instead make it an empty string * Update tests to work with the new paradigm * Simplify the code --------- Co-authored-by: chriszarate <chris.zarate@automattic.com>
1 parent 71cdc6a commit 22e067b

3 files changed

Lines changed: 111 additions & 35 deletions

File tree

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -473,10 +473,10 @@ let localDoc: Y.Doc;
473473
* @param updatedValue The updated value.
474474
* @param cursorPosition The position of the cursor after the change occurs.
475475
*/
476-
function mergeRichTextUpdate(
476+
export function mergeRichTextUpdate(
477477
blockYText: Y.Text,
478478
updatedValue: string,
479-
cursorPosition: number | null
479+
cursorPosition: number | null = null
480480
): void {
481481
// Gutenberg does not use Yjs shared types natively, so we can only subscribe
482482
// to changes from store and apply them to Yjs types that we create and

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

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {
2121
import { BaseAwareness } from '../awareness/base-awareness';
2222
import {
2323
mergeCrdtBlocks,
24+
mergeRichTextUpdate,
2425
type Block,
2526
type YBlock,
2627
type YBlocks,
@@ -56,11 +57,11 @@ export interface YPostRecord extends YMapRecord {
5657
author: number;
5758
// Blocks are undefined when they need to be re-parsed from content.
5859
blocks: YBlocks | undefined;
59-
content: string;
60+
content: Y.Text;
6061
categories: number[];
6162
comment_status: string;
6263
date: string | null;
63-
excerpt: string;
64+
excerpt: Y.Text;
6465
featured_media: number;
6566
format: string;
6667
meta: YMapWrap< YMapRecord >;
@@ -70,7 +71,7 @@ export interface YPostRecord extends YMapRecord {
7071
sticky: boolean;
7172
tags: number[];
7273
template: string;
73-
title: string;
74+
title: Y.Text;
7475
}
7576

7677
// Properties that are allowed to be synced for a post.
@@ -188,11 +189,28 @@ export function applyPostChangesToCRDTDoc(
188189
}
189190

190191
case 'content':
191-
case 'excerpt': {
192+
case 'excerpt':
193+
case 'title': {
192194
const currentValue = ymap.get( key );
193-
const rawNewValue = getRawValue( newValue );
195+
let rawValue = getRawValue( newValue );
196+
197+
// Copy logic from prePersistPostType to ensure that the "Auto
198+
// Draft" template title is not synced.
199+
if (
200+
key === 'title' &&
201+
! currentValue &&
202+
'Auto Draft' === rawValue
203+
) {
204+
rawValue = '';
205+
}
206+
207+
if ( currentValue instanceof Y.Text ) {
208+
mergeRichTextUpdate( currentValue, rawValue ?? '' );
209+
} else {
210+
const newYText = new Y.Text( rawValue ?? '' );
211+
ymap.set( key, newYText );
212+
}
194213

195-
updateMapValue( ymap, key, currentValue, rawNewValue );
196214
break;
197215
}
198216

@@ -237,20 +255,6 @@ export function applyPostChangesToCRDTDoc(
237255
break;
238256
}
239257

240-
case 'title': {
241-
const currentValue = ymap.get( key );
242-
243-
// Copy logic from prePersistPostType to ensure that the "Auto
244-
// Draft" template title is not synced.
245-
let rawNewValue = getRawValue( newValue );
246-
if ( ! currentValue && 'Auto Draft' === rawNewValue ) {
247-
rawNewValue = '';
248-
}
249-
250-
updateMapValue( ymap, key, currentValue, rawNewValue );
251-
break;
252-
}
253-
254258
// Add support for additional properties here.
255259

256260
default: {

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

Lines changed: 85 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,9 @@ describe( 'crdt', () => {
5353

5454
applyPostChangesToCRDTDoc( doc, changes, mockPostType );
5555

56-
expect( map.get( 'title' ) ).toBe( 'New Title' );
56+
const title = map.get( 'title' );
57+
expect( title ).toBeInstanceOf( Y.Text );
58+
expect( title?.toString() ).toBe( 'New Title' );
5759
} );
5860

5961
it( 'does not sync disallowed properties', () => {
@@ -65,7 +67,7 @@ describe( 'crdt', () => {
6567
applyPostChangesToCRDTDoc( doc, changes, mockPostType );
6668

6769
expect( map.has( 'unsyncedProperty' ) ).toBe( false );
68-
expect( map.get( 'title' ) ).toBe( 'New Title' );
70+
expect( map.get( 'title' )?.toString() ).toBe( 'New Title' );
6971
} );
7072

7173
it( 'does not sync function values', () => {
@@ -85,7 +87,9 @@ describe( 'crdt', () => {
8587

8688
applyPostChangesToCRDTDoc( doc, changes, mockPostType );
8789

88-
expect( map.get( 'title' ) ).toBe( 'Raw Title' );
90+
const title = map.get( 'title' );
91+
expect( title ).toBeInstanceOf( Y.Text );
92+
expect( title?.toString() ).toBe( 'Raw Title' );
8993
} );
9094

9195
it( 'skips "Auto Draft" template title when no current value exists', () => {
@@ -95,7 +99,9 @@ describe( 'crdt', () => {
9599

96100
applyPostChangesToCRDTDoc( doc, changes, mockPostType );
97101

98-
expect( map.get( 'title' ) ).toBe( '' );
102+
const title = map.get( 'title' );
103+
expect( title ).toBeInstanceOf( Y.Text );
104+
expect( title?.toString() ).toBe( '' );
99105
} );
100106

101107
it( 'handles excerpt with RenderedText format', () => {
@@ -109,7 +115,9 @@ describe( 'crdt', () => {
109115

110116
applyPostChangesToCRDTDoc( doc, changes, mockPostType );
111117

112-
expect( map.get( 'excerpt' ) ).toBe( 'Raw excerpt' );
118+
const excerpt = map.get( 'excerpt' );
119+
expect( excerpt ).toBeInstanceOf( Y.Text );
120+
expect( excerpt?.toString() ).toBe( 'Raw excerpt' );
113121
} );
114122

115123
it( 'does not sync empty slug', () => {
@@ -178,14 +186,16 @@ describe( 'crdt', () => {
178186
expect( map.get( 'blocks' ) ).toBeUndefined();
179187
} );
180188

181-
it( 'syncs content as a string', () => {
189+
it( 'syncs content as Y.Text', () => {
182190
const changes = {
183191
content: 'Hello, world!',
184192
} as PostChanges;
185193

186194
applyPostChangesToCRDTDoc( doc, changes, mockPostType );
187195

188-
expect( map.get( 'content' ) ).toBe( 'Hello, world!' );
196+
const content = map.get( 'content' );
197+
expect( content ).toBeInstanceOf( Y.Text );
198+
expect( content?.toString() ).toBe( 'Hello, world!' );
189199
} );
190200

191201
it( 'syncs content with RenderedText format', () => {
@@ -198,11 +208,73 @@ describe( 'crdt', () => {
198208

199209
applyPostChangesToCRDTDoc( doc, changes, mockPostType );
200210

201-
expect( map.get( 'content' ) ).toBe(
211+
const content = map.get( 'content' );
212+
expect( content ).toBeInstanceOf( Y.Text );
213+
expect( content?.toString() ).toBe(
202214
'<!-- wp:paragraph --><p>Hello</p><!-- /wp:paragraph -->'
203215
);
204216
} );
205217

218+
it( 'updates existing Y.Text title in place via mergeRichTextUpdate', () => {
219+
// First apply to create the Y.Text.
220+
applyPostChangesToCRDTDoc(
221+
doc,
222+
{ title: 'Old Title' } as PostChanges,
223+
mockPostType
224+
);
225+
const titleRef = map.get( 'title' );
226+
227+
// Apply again — should update in place, not replace.
228+
applyPostChangesToCRDTDoc(
229+
doc,
230+
{ title: 'New Title' } as PostChanges,
231+
mockPostType
232+
);
233+
234+
expect( map.get( 'title' ) ).toBe( titleRef );
235+
expect( map.get( 'title' )?.toString() ).toBe( 'New Title' );
236+
} );
237+
238+
it( 'updates existing Y.Text content in place via mergeRichTextUpdate', () => {
239+
// First apply to create the Y.Text.
240+
applyPostChangesToCRDTDoc(
241+
doc,
242+
{ content: 'Old content' } as PostChanges,
243+
mockPostType
244+
);
245+
const contentRef = map.get( 'content' );
246+
247+
// Apply again — should update in place, not replace.
248+
applyPostChangesToCRDTDoc(
249+
doc,
250+
{ content: 'New content' } as PostChanges,
251+
mockPostType
252+
);
253+
254+
expect( map.get( 'content' ) ).toBe( contentRef );
255+
expect( map.get( 'content' )?.toString() ).toBe( 'New content' );
256+
} );
257+
258+
it( 'updates existing Y.Text excerpt in place via mergeRichTextUpdate', () => {
259+
// First apply to create the Y.Text.
260+
applyPostChangesToCRDTDoc(
261+
doc,
262+
{ excerpt: 'Old excerpt' } as PostChanges,
263+
mockPostType
264+
);
265+
const excerptRef = map.get( 'excerpt' );
266+
267+
// Apply again — should update in place, not replace.
268+
applyPostChangesToCRDTDoc(
269+
doc,
270+
{ excerpt: 'New excerpt' } as PostChanges,
271+
mockPostType
272+
);
273+
274+
expect( map.get( 'excerpt' ) ).toBe( excerptRef );
275+
expect( map.get( 'excerpt' )?.toString() ).toBe( 'New excerpt' );
276+
} );
277+
206278
it( 'syncs meta fields', () => {
207279
const changes = {
208280
meta: {
@@ -266,7 +338,7 @@ describe( 'crdt', () => {
266338

267339
beforeEach( () => {
268340
map = getRootMap< YPostRecord >( doc, CRDT_RECORD_MAP_KEY );
269-
map.set( 'title', 'CRDT Title' );
341+
map.set( 'title', new Y.Text( 'CRDT Title' ) );
270342
map.set( 'status', 'draft' );
271343
map.set( 'date', '2025-01-01' );
272344
} );
@@ -287,7 +359,7 @@ describe( 'crdt', () => {
287359
} );
288360

289361
it( 'filters out disallowed properties', () => {
290-
map.set( 'title', 'Test title' );
362+
map.set( 'title', new Y.Text( 'Test title' ) );
291363
map.set( 'unsyncedProp', 'value' );
292364

293365
const editedRecord = {} as Post;
@@ -416,7 +488,7 @@ describe( 'crdt', () => {
416488
} );
417489

418490
it( 'detects content changes from string value', () => {
419-
map.set( 'content', 'New content' );
491+
map.set( 'content', new Y.Text( 'New content' ) );
420492

421493
const editedRecord = {
422494
content: 'Old content',
@@ -432,7 +504,7 @@ describe( 'crdt', () => {
432504
} );
433505

434506
it( 'detects content changes from RenderedText value', () => {
435-
map.set( 'content', 'New content' );
507+
map.set( 'content', new Y.Text( 'New content' ) );
436508

437509
const editedRecord = {
438510
content: { raw: 'Old content', rendered: 'Old content' },
@@ -448,7 +520,7 @@ describe( 'crdt', () => {
448520
} );
449521

450522
it( 'excludes content when unchanged from RenderedText value', () => {
451-
map.set( 'content', 'Same content' );
523+
map.set( 'content', new Y.Text( 'Same content' ) );
452524

453525
const editedRecord = {
454526
content: { raw: 'Same content', rendered: 'Same content' },

0 commit comments

Comments
 (0)