Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion packages/core-data/src/resolvers.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
} from './utils';
import { fetchBlockPatterns } from './fetch';
import { restoreSelection, getSelectionHistory } from './utils/crdt-selection';
import { stripServerManagedMeta } from './utils/crdt';

/**
* Requests authors from the REST API.
Expand Down Expand Up @@ -247,13 +248,25 @@ export const getEntityRecord =
return;
}

// Shallow-clone the record and its meta so we
// don't mutate the cached selector result.
// Strip server-managed meta keys before saving —
// sending their (possibly stale) client values
// can overwrite what the server calculates
// during save (e.g. _wp_ignored_hooked_blocks).
const recordToSave = {
...editedRecord,
meta: { ...meta },
};
Copy link
Copy Markdown
Contributor

@chriszarate chriszarate Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shallow-cloning in the resolver and mutating via function call feels like the wrong fix.

At the very least, we can move this logic to prePersistPostType in entities.js. There, the record is already cloned in order to amend it with additional edits. I can imagine something like a addPersistedCrdtDocToPostRecord function that is exported from crdt.ts and called from prePersistPostType.

However, what nags is that the root cause of this issue is something different: We are calling saveEntityRecord in the persistCrdtDoc record handler, which results in saving the entire record, not just fields that have changed.

User-initiated saves, on the other hand, are conducted via saveEditedEntityRecord which saves only the "dirty" fields. Therefore _wp_ignored_hooked_blocks would never be among them.

Most likely, the best fix is to persist CRDT documents outside of post meta (i.e., in the sync data table) where it does not need to interfere with the save data.

Short of that, moving this logic to entities.js is probably better.

stripServerManagedMeta( recordToSave );

// Trigger a save to persist the CRDT document. The entity's
// pre-persist hooks will create the persisted CRDT document
// and apply it to the record's meta.
dispatch.saveEntityRecord(
kind,
name,
editedRecord
recordToSave
);
} );
},
Expand Down
27 changes: 26 additions & 1 deletion packages/core-data/src/utils/crdt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,36 @@ export interface YPostRecord extends YMapRecord {

export const POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE = '_crdt_document';

// Post meta keys that should *not* be synced.
const disallowedPostMetaKeys = new Set< string >( [
POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE,
] );

// Post meta keys that are server-managed and must not be sent by the client
// during saves. Sending stale client values for these keys overwrites the
// server's calculations (e.g. update_ignored_hooked_blocks_postmeta).
const serverManagedPostMetaKeys = new Set< string >( [
'_wp_ignored_hooked_blocks',
] );

/**
* Remove server-managed meta keys from a record's meta object in place.
* These keys are calculated by the server during save and must not be
* overwritten by possibly stale client values.
*
* @param {Object} record An entity record with an optional `meta` property.
*/
export function stripServerManagedMeta(
record: Record< string, unknown >
): void {
const meta = record.meta;
if ( ! meta || typeof meta !== 'object' ) {
return;
}
serverManagedPostMetaKeys.forEach( ( metaKey ) => {
delete ( meta as Record< string, unknown > )[ metaKey ];
} );
}

Comment on lines +83 to +108
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without knowing too much about RTC, this seems backwards -- the _wp_ignored_hooked_blocks post meta is added precisely to prevent duplication of hooked blocks (see).

/**
* Given a set of local changes to a generic entity record, apply those changes
* to the local Y.Doc.
Expand Down
32 changes: 32 additions & 0 deletions packages/core-data/src/utils/test/crdt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ import {
applyPostChangesToCRDTDoc,
getPostChangesFromCRDTDoc,
POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE,
stripServerManagedMeta,
type PostChanges,
type YPostRecord,
} from '../crdt';
Expand Down Expand Up @@ -932,3 +933,34 @@ function addBlockToDoc(

return ytext;
}

describe( 'stripServerManagedMeta', () => {
it( 'should remove _wp_ignored_hooked_blocks from meta', () => {
const record = {
meta: {
_wp_ignored_hooked_blocks: '',
footnotes: '',
},
};
stripServerManagedMeta( record );
expect( record.meta ).toEqual( { footnotes: '' } );
} );

it( 'should be a no-op when the key is not present', () => {
const record = { meta: { footnotes: '' } };
stripServerManagedMeta( record );
expect( record.meta ).toEqual( { footnotes: '' } );
} );

it( 'should be a no-op when meta is null', () => {
const record = { meta: null };
stripServerManagedMeta( record );
expect( record.meta ).toBeNull();
} );

it( 'should be a no-op when meta is undefined', () => {
const record = {};
stripServerManagedMeta( record );
expect( record ).toEqual( {} );
} );
} );
21 changes: 0 additions & 21 deletions test/e2e/specs/editor/plugins/block-hooks.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,6 @@
*/
const { test, expect } = require( '@wordpress/e2e-test-utils-playwright' );

/**
* Internal dependencies
*/
const {
setCollaboration,
} = require( '../../editor/collaboration/fixtures/collaboration-utils' );

const dummyBlocksContent = `<!-- wp:heading -->
<h2 class="wp-block-heading">This is a dummy heading</h2>
<!-- /wp:heading -->
Expand Down Expand Up @@ -72,12 +65,6 @@ test.describe( 'Block Hooks API', () => {
} else {
containerPost = postObject;
}

/**
* Since the Block Hooks API relies on server-side rendering to insert
* the hooked blocks, there is a fundamental incompatibility with RTC.
*/
await setCollaboration( requestUtils, false );
} );

test.afterAll( async ( { requestUtils } ) => {
Expand All @@ -87,7 +74,6 @@ test.describe( 'Block Hooks API', () => {

await requestUtils.deleteAllPosts();
await requestUtils.deleteAllBlocks();
await setCollaboration( requestUtils, true );
} );

test( `should insert hooked blocks into ${ name } on frontend`, async ( {
Expand Down Expand Up @@ -212,12 +198,6 @@ test.describe( 'Block Hooks API', () => {
} else {
containerPost = postObject;
}

/**
* Since the Block Hooks API relies on server-side rendering to insert
* the hooked blocks, there is a fundamental incompatibility with RTC.
*/
await setCollaboration( requestUtils, false );
} );

test.afterAll( async ( { requestUtils } ) => {
Expand All @@ -227,7 +207,6 @@ test.describe( 'Block Hooks API', () => {

await requestUtils.deleteAllPosts();
await requestUtils.deleteAllBlocks();
await setCollaboration( requestUtils, true );
} );

test( `should insert hooked blocks into ${ name } on frontend`, async ( {
Expand Down
Loading