-
-
Notifications
You must be signed in to change notification settings - Fork 538
feat: pausing & resuming of a ydoc #1639
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
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@blocknote/ariakit
@blocknote/code-block
@blocknote/core
@blocknote/mantine
@blocknote/react
@blocknote/server-util
@blocknote/shadcn
@blocknote/xl-multi-column
@blocknote/xl-odt-exporter
@blocknote/xl-pdf-exporter
@blocknote/xl-docx-exporter
commit: |
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.
awesome!!! Looks solid.
It does not yet implement merging the changes back into the old ydoc, but it should be fairly straight-forward to do.
I might be missing something here; but changes are merged right?
Whoops outdated comment, yep. Changes get merged with Kevin's help |
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.
super cool, including the demo!
- maybe extract the demo to a separate example?
- can we add unit tests?
- do you see good ways to organize the better? my concern is that
BlockNoteEditor
will grow in size infinitely if keep adding this kind of logic there
*/ | ||
public forkYjsSync() { | ||
if (this.forkedState) { | ||
return; |
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 saw you switched from throwing an error here to returning silently. imo it's perfectly valid to throw an error if we say calling this method is invalid when the document is already forked - and this could help detect application errors earlier
|
||
if (!originalFragment) { | ||
// No original fragment found, so no need to fork | ||
return; |
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.
can this happen or should we also throw an error here?
@nperez0111 is there a good reason for you to occasionally use force-push? We had the recent issue with lost data on matthew's branch, but for example when reviewing this PR I also run into this: ![]() |
I use a rebase workflow https://git-scm.com/book/en/v2/Git-Branching-Rebasing rather than a merge workflow for consistent histories. I've never seen that message before. But this branch was based on another branch which got Squash merged. So this branch would have been conflicting with main. So a rebase removes the old commits and re-bases them on to the most recent main (which already moved). I can see the diff just fine in my GitHub view though 🤷 Side note I could've recovered Matthew's branch I just wasn't at my computer at the time |
it's because I select "changes since last review" (which you probably don't have on your side, right?) |
This implements pausing and resuming of a ydoc, switching to a new ydoc that can be written to, and then switching back.
Screen.Recording.2025-04-24.at.19.22.27.mp4