Real-time collaboration: Enable CRDT doc state map changes #72763
+205
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +97 B (0%) Total Size: 2.19 MB
ℹ️ View Unchanged
|
chriszarate
reviewed
Oct 29, 2025
packages/core-data/src/entities.js
Outdated
| }; | ||
|
|
||
| // Mark the doc as having been persisted. | ||
| getSyncManager()?.markPersisted( |
Contributor
There was a problem hiding this comment.
It has not yet been persisted at this point, so this creates a race condition if peers use it as a signal to refetch the entity
Contributor
Author
There was a problem hiding this comment.
Do we want that ability in place? I took away the refetch the entity comment for that reason and made it seem instead its for notifications.
I had tried to put it under actions like it used to be, but that didn't consistently get triggered which was odd.
Contributor
Author
There was a problem hiding this comment.
The other option that I had in mind actually was the action - editor.savePost. That's fired right after the post update finishes.
Contributor
There was a problem hiding this comment.
I took away the refetch the entity comment for that reason
It served an important purpose in syncing the state of the Save / Publish button, so that peers can update their entity record in their local store and correctly compute "dirty" state.
Contributor
Author
There was a problem hiding this comment.
But then the question is, where would that be called? In the VIP-RTC plugin maybe? So once could decide if they truly want this feature or not and how they use it?
Contributor
There was a problem hiding this comment.
It would be called in the sync manager. You can see how it previously worked here:
Right now in our plugin branch, we've regressed to a non-synced Save / Publish button state. Refetching is how we solved that.
Contributor
Author
There was a problem hiding this comment.
Apologies, I meant to say that do we want that ability in this place not at all.
I definitely know why we had that in place, as we went through several PRs to figure out the right solution for it. My thought was that, we'd use this state key way just for notifications so if it is a touch fragile it won't be a big deal. But, for getting the publish/save state synced we'd need to make sure that's perfect.
I have an alternative area that I'm experimenting with, which is the editor.savePost action.
Would perhaps re-naming these two keys to something else help? Or do you think it's better to just do it all in one way similar to how we did it before?
I'm approaching separately, so notifications and features like re-fetching are done separately.
chriszarate
reviewed
Oct 29, 2025
Comment on lines
+340
to
+341
| stateMap.set( PERSISTED_AT_KEY, lastPersistedAt ); | ||
| stateMap.set( PERSISTED_BY_KEY, ydoc.clientID ); |
Contributor
There was a problem hiding this comment.
I've been wondering if this is really the best approach. It's not really document state and it feels fragile.
Contributor
Author
There was a problem hiding this comment.
The problem I guess is that, we need to be able to tell the other clients that the document has been updated and by whom without spamming notifications.
If we hook into each change, we'd be spamming notifications. If we hook into when the post is saved, then those changes are batched at least.
That's why I stuck with this approach.
Contributor
Author
There was a problem hiding this comment.
I realized why I like this approach. It's like our very own redux action that we can fire that plugins can hook into to react the way they want.
ingeniumed
commented
Oct 29, 2025
| handlers.saveRecord(); | ||
| } | ||
|
|
||
| addAction( |
Contributor
Author
There was a problem hiding this comment.
@chriszarate - lemme know what you think about this new way to track a post being saved. This action is fired async at the very end of a post save, so we know it's been persisted.
Now, it's possible to actually use this key to track if a refetch needs to happen or stick to just notifications.
Contributor
Author
There was a problem hiding this comment.
I kept this here as it felt like the right place, since it's where the ydoc is bootstrapped. We aren't changing anything about the post just tracking it really.
ingeniumed
commented
Oct 29, 2025
| 'sync.markCRDTDocPersisted', | ||
| async ( post, options ) => { | ||
| // Skip if the post's id doesn't match (including different ID types), or if it's an autosave. | ||
| if ( post.id !== objectId || options.isAutosave ) { |
Contributor
Author
There was a problem hiding this comment.
I skipped auto saves at the moment, to keep it similar to Gutenberg where a notice would be sent if you hit the save yourself.
The post ID check is for safety, and would account for even the ID format not being supported.
Is it worth adding an extra post type check too?
Contributor
Author
|
Closing this PR in favour of #72851. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
We disabled the ability to observe keys on the state map of a CRDT doc, that could then power notifications within the block editor. This change re-enables that, albeit with a simpler system as it only targets a post being updated rather than also being restored.
Why?
It's important to provide awareness into when a post is updated, across all clients rather than just the one making the changes.
How?
The state map on a CRDT doc will now have two properties:
Together this would allow things like sending a notification to all clients when a post is updated.
Testing Instructions
Screenshots or screencast