Skip to content

Commit c3fdb79

Browse files
chriszaratechriszarate
authored
Do not wrap persisted doc applied update in transaction (#74753)
1 parent 4ea0d4f commit c3fdb79

1 file changed

Lines changed: 61 additions & 53 deletions

File tree

‎packages/sync/src/manager.ts‎

Lines changed: 61 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -246,66 +246,74 @@ export function createSyncManager(): SyncManager {
246246
ydoc: targetDoc,
247247
} = entityState;
248248

249-
targetDoc.transact( () => {
250-
if ( ! supports?.crdtPersistence ) {
251-
// Apply the current record as changes.
249+
if ( ! supports?.crdtPersistence ) {
250+
// Apply the current record as changes.
251+
targetDoc.transact( () => {
252252
applyChangesToCRDTDoc( targetDoc, record );
253-
return;
254-
}
253+
}, LOCAL_SYNC_MANAGER_ORIGIN );
254+
return;
255+
}
255256

256-
// Get the persisted CRDT document, if it exists.
257-
const tempDoc = getPersistedCrdtDoc( record );
257+
// Get the persisted CRDT document, if it exists.
258+
const tempDoc = getPersistedCrdtDoc( record );
258259

259-
if ( ! tempDoc ) {
260-
// Apply the current record as changes and trigger a save, which will
261-
// persist the CRDT document. (The entity should call `createEntityMeta`
262-
// via its pre-persist hook.)
260+
if ( ! tempDoc ) {
261+
// Apply the current record as changes and trigger a save, which will
262+
// persist the CRDT document. (The entity should call `createEntityMeta`
263+
// via its pre-persist hook.)
264+
targetDoc.transact( () => {
263265
applyChangesToCRDTDoc( targetDoc, record );
264266
handlers.saveRecord();
265-
return;
266-
}
267+
}, LOCAL_SYNC_MANAGER_ORIGIN );
268+
return;
269+
}
267270

268-
// Apply the persisted document to the current document as a single update.
269-
// This is done even if the persisted document has been invalidated. This
270-
// prevents a newly joining peer (or refreshing user) from re-initializing
271-
// the CRDT document (the "initialization problem").
272-
const update = Y.encodeStateAsUpdateV2( tempDoc );
273-
Y.applyUpdateV2( targetDoc, update );
274-
275-
// Compute the differences between the persisted doc and the current
276-
// record. This can happen when:
277-
//
278-
// 1. The server makes updates on save that mutate the entity. Example: On
279-
// initial save, the server adds the "Uncategorized" category to the
280-
// post.
281-
// 2. An "out-of-band" update occurs. Example: a WP-CLI command or direct
282-
// database update mutates the entity.
283-
// 3. Unsaved changes are synced from a peer _before_ this code runs. We
284-
// can't control when (or if) remote changes are synced, so this is a
285-
// race condition.
286-
const invalidations = getChangesFromCRDTDoc( tempDoc, record );
287-
const invalidatedKeys = Object.keys( invalidations );
288-
289-
// Destroy the temporary document to prevent leaks.
290-
tempDoc.destroy();
291-
292-
if ( 0 === invalidatedKeys.length ) {
293-
// The persisted CRDT document is valid. There are no updates to apply.
294-
return;
295-
}
271+
// Apply the persisted document to the current document as a single update.
272+
// This is done even if the persisted document has been invalidated. This
273+
// prevents a newly joining peer (or refreshing user) from re-initializing
274+
// the CRDT document (the "initialization problem").
275+
//
276+
// IMPORTANT: Do not wrap this in a transaction with the local origin. It
277+
// effectively advances the state vector for the current client, which causes
278+
// Yjs to think that another client is using this client ID.
279+
const update = Y.encodeStateAsUpdateV2( tempDoc );
280+
Y.applyUpdateV2( targetDoc, update );
281+
282+
// Compute the differences between the persisted doc and the current
283+
// record. This can happen when:
284+
//
285+
// 1. The server makes updates on save that mutate the entity. Example: On
286+
// initial save, the server adds the "Uncategorized" category to the
287+
// post.
288+
// 2. An "out-of-band" update occurs. Example: a WP-CLI command or direct
289+
// database update mutates the entity.
290+
// 3. Unsaved changes are synced from a peer _before_ this code runs. We
291+
// can't control when (or if) remote changes are synced, so this is a
292+
// race condition.
293+
const invalidations = getChangesFromCRDTDoc( tempDoc, record );
294+
const invalidatedKeys = Object.keys( invalidations );
295+
296+
// Destroy the temporary document to prevent leaks.
297+
tempDoc.destroy();
298+
299+
if ( 0 === invalidatedKeys.length ) {
300+
// The persisted CRDT document is valid. There are no updates to apply.
301+
return;
302+
}
303+
304+
// Use the invalidated keys to get the updated values from the entity.
305+
const changes = invalidatedKeys.reduce(
306+
( acc, key ) =>
307+
Object.assign( acc, {
308+
[ key ]: record[ key ],
309+
} ),
310+
{}
311+
);
296312

297-
// Use the invalidated keys to get the updated values from the entity.
298-
const changes = invalidatedKeys.reduce(
299-
( acc, key ) =>
300-
Object.assign( acc, {
301-
[ key ]: record[ key ],
302-
} ),
303-
{}
304-
);
305-
306-
// Apply the changes and trigger a save, which will persist the CRDT
307-
// document. (The entity should call `createEntityMeta` via its pre-persist
308-
// hook.)
313+
// Apply the changes and trigger a save, which will persist the CRDT
314+
// document. (The entity should call `createEntityMeta` via its pre-persist
315+
// hook.)
316+
targetDoc.transact( () => {
309317
applyChangesToCRDTDoc( targetDoc, changes );
310318
handlers.saveRecord();
311319
}, LOCAL_SYNC_MANAGER_ORIGIN );

0 commit comments

Comments
 (0)