-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Yjs Collab] Reliable sync with the backend #68483
base: trunk
Are you sure you want to change the base?
Conversation
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 Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @dmonad. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. 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. |
👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @dmonad! In case you missed it, we'd love to have you join us in our Slack community. If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great start, thank you! A couple of questions and small suggestions on my first read-through.
I would like to see some automated tests before this gets too far, especially around places where others will extend this.
lib/experimental/synchronization.php
Outdated
// @todo the following regex doesn't catch the ygutenberg comment | ||
// preg_match('/<!-- y:gutenberg version="([a-zA-Z0-9]*)" state="([a-zA-Z0-9+\/]*={0,3})" new-content-clientid="([0-9]*)" -->/', $postcontent, $yinfo); | ||
preg_match('/<!-- y:gutenberg version=\"(.*)\" state=\"(.*)\" new-content-clientid=\"(.*)\" -->/', $postcontent, $yinfo); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this is used both here and in filter_post_content_ydoc
. I wonder if this should be abstracted into it's own get_yinfo
function that can then have automated test to ensure it correctly handles any necessary edge cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed the issue with the proper regex and abstracted it into a gutenberg_get_yinfo
function.
Where would I locate tests for the sync
package? I'm pretty sure that this would be easy to implement.
Previously, Gutenberg implemented a basic Yjs editor binding. Changes were synced using y-webrtc when collaborative editing is enabled. However, the previous implementation did not reliably sync the backend-state with the Yjs document. Hence, the previous implementation could result in content duplication, or loss of changes that were created in the collaborative session. This commit implements the initial approach to sync the backend document (HTML encoded) with the Yjs document without the server understanding Yjs. Following things were implemented: - We save the Yjs document in an HTML comment, that is stored in the HTML document stored in WordPress - When the client notices that the Yjs document stored in the comment does not reflect the state of the HTML document, we assume that the changes were generated by the backend. Hence we update the document that is stored in the HTML comment, and merge the Yjs update to the Yjs document that is used in the collaborative session. Then we save the merged document once again to the backend. The changes from a backend are applied as a simulated "system user". This should guarantee that everyone applies the changes in the same way, so we avoid content duplication. The approach in this commit is still insufficient. We need to store additional information in the backend to ensure that certain edge-cases are handled.
…ntities are always registered.
… sync / webrtc sync
Co-authored-by: Aaron Jorbin <[email protected]>
Co-authored-by: Aaron Jorbin <[email protected]>
As @aaronjorbin rightfully pointed out, the new filters approach don't need to run at the end anymore.
I fixed all tests but the E2E Tests. I also wanted to implement an integration test for a collaborative session. We would need to enable the experimental feature in the e2e test and then perform some actions on two different peers. But I would also need help to implement this. |
What?
This PR implements a reliable sync protocol for collaborative editing.
Why?
Previously, Gutenberg implemented a basic Yjs editor binding. Changes were synced using y-webrtc when collaborative editing is enabled. However, the previous implementation did not reliably sync the backend-state with the Yjs document. The collaborative document was not preserved over different sessions, which might lead to content duplication and even loss of content.
Furthermore, even in non-collaborative sessions, it is quite easy to overwrite content from other users. Changes that are created by different users at the same time are not reconciled.
Related discussion on the considerations to improve the current sync approach: #65012
How?
This PR implements a reliable sync protocol for syncing backend state with a Yjs document.
<!-- y:gutenberg .. -->
, which is stored in the HTML document stored in WordPress<!-- y:gutenberg -->
comment and send it to the backend.Furthermore, this PR improves on the general user-experience of collaborative editing.
I documented the sync protocol in greater detail in
packages/sync/CODE.md
.Testing Instructions
Things to note
TODOs
I hope that the sync approach implemented in this PR will be a good foundation for future work on making different parts of Gutenberg more collaborative. If we want to make Gutenberg / WordPress even more collaborative in the future, we need to share knowledge and make collaborative editing a priority for all future work. It is quite hard to add collaborative editing on-top of an existing code-base. Ideally, future work is built with collaborative editing in mind.
In this regard, I want to host a review & demo session on Monday, January 6 7pm UTC. This would be a good place to ask questions. Also, we can try to break this implementation and measure the overhead of decreasing the autosave interval.
"https://us02web.zoom.us/j/" + "86065095542"