Send one full update from y-websocket #6650
Open
Conversation
This file contains 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
max-nextcloud
force-pushed
the
fix/y-websocket-with-full-updates
branch
from
2cfb371
to
4675077
Compare
max-nextcloud
force-pushed
the
fix/y-websocket-with-full-updates
branch
2 times, most recently
from
1702eee
to
9b78b88
Compare
Signed-off-by: Max <[email protected]>
Signed-off-by: Max <[email protected]>
Keep an internal ydoc tracking updates that came from the server. Send updates, that would sync this doc with the current doc state. Signed-off-by: Max <[email protected]>
Signed-off-by: Max <[email protected]>
Signed-off-by: Max <[email protected]>
The pushed steps are echoed back with all other steps since version immediately. Processing them reduces the size of the following pushes and syncs. Signed-off-by: Max <[email protected]>
It has always been publicly called from the PollingBackend. Signed-off-by: Max <[email protected]>
Updates now include all the local structs that were not yet received from remote. No need to compute a separate update message anymore. Signed-off-by: Max <[email protected]>
max-nextcloud
force-pushed
the
fix/y-websocket-with-full-updates
branch
from
9b78b88
to
1495034
Compare
Apply document state as a step. Process it like other steps received from the remote. In particular include it in the tracking of steps already applied and set the version accordingly. Signed-off-by: Max <[email protected]>
This was a very inefficient attempt to resync that we did not even process on the client side. Only the steps since the last save may not be enough to get back in sync. However we can expand this by including the document state or storing it as the first step after a save. Signed-off-by: Max <[email protected]>
Signed-off-by: Max <[email protected]>
Do not process document state from create response. During create the editor has not been initialized fully and the cursor position is 0 - which is invalid as it is not inside a node with inline content. (It is inside the doc before the initial paragraph.) This also allows processing document state later on in order to recover from out of sync situations. But we do not make use of that yet. Signed-off-by: Max <[email protected]>
max-nextcloud
force-pushed
the
fix/y-websocket-with-full-updates
branch
from
1495034
to
f08a168
Compare
juliusknorr
reviewed
Nov 18, 2024
There was a problem hiding this comment.
Gave this a first try and basic syncing is still working fine, also some offline online switching on two browser sessions worked as expected.
One small difference I noticed (without reading the code yet in detail):
- When loading the file it seems we no longer wait until the content is loaded to hide the loading indicator, we start with an empty document and can type before any document state is propagated to us. @max-nextcloud is this expected or can it cause any side effects?
juliusknorr
reviewed
Nov 18, 2024
| @@ -0,0 +1,539 @@ | |||
| /** | |||
| * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors | |||
| * SPDX-License-Identifier: AGPL-3.0-or-later | |||
There was a problem hiding this comment.
The file license is probably still MIT then, but not entirely sure, probably something to clarify
I didin't diff how much it is based on it (https://github.com/yjs/y-websocket/blob/master/LICENSE)
max-nextcloud
commented
Nov 19, 2024
lib/Service/DocumentService.php
Outdated
| $getStepsSinceVersion = count($querySteps) > 0 ? 0 : $version; | ||
| // If there were any queries in the steps send all steps since last save. | ||
| $getStepsSinceVersion = count($querySteps) > 0 | ||
| ? $document->getLastSavedVersion() |
There was a problem hiding this comment.
this needs to be min($document->getLastSavedVersion(), $version) to make sure we don't skip any steps
There was a problem hiding this comment.
addressed in the next commit by including the document state that matches $document->getLastSavedVersion
There was a problem hiding this comment.
Still using the smaller would mean we have a redundancy in case the client that did the save was out of sync.
|
What's missing:
|
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.
Always send full diff to server state.
received from the server.
Todo: