BitPhinix / slate-yjs

Yjs binding for Slate
https://docs.slate-yjs.dev
MIT License
514 stars 73 forks source link

use editor.sharedRoot in connect/disconnect #358

Closed zarv1k closed 2 years ago

zarv1k commented 2 years ago

Hi!

First of all I wanna thank you for the great library!

That PR fixes subscribing for events on actual editor.sharedRoot that could be replaced/overwritten by developers in their own plugins.

In some use cases there should be possible to replace/overwrite editor.sharedRoot in your own plugins, e.g. to be able to initialize Y.Doc/sharedRoot in your own plugins (lazily in sync or even async way), e.g. custom plugin that fetches initial StateVector from backend and after that to call editor.connect() . But when I overwrite editor.sharedRoot in my own plugin and call editor.connect() it continue subscribing to initial sharedRoot from withYjs plugin argument.

changeset-bot[bot] commented 2 years ago

🦋 Changeset detected

Latest commit: 27d39ddc3cf79797a878d0540f8b2605fcf38988

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages | Name | Type | | -------------------------- | ----- | | @slate-yjs/core | Patch | | @slate-yjs/example-backend | Patch | | @slate-yjs/example | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

codecov[bot] commented 2 years ago

Codecov Report

Merging #358 (551368b) into main (576b809) will not change coverage. The diff coverage is 50.00%.

:exclamation: Current head 551368b differs from pull request most recent head 27d39dd. Consider uploading reports for the commit 27d39dd to get more accurate results

@@           Coverage Diff           @@
##             main     #358   +/-   ##
=======================================
  Coverage   71.31%   71.31%           
=======================================
  Files          30       30           
  Lines        2301     2301           
  Branches      297      297           
=======================================
  Hits         1641     1641           
  Misses        660      660           
Impacted Files Coverage Δ
packages/core/src/plugins/withYjs.ts 81.07% <50.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

BitPhinix commented 2 years ago

Thanks for this PR! Just released a new version with the change. I'd be curious to see the details of what exactly you are doing though, sounds interesting 😅

zarv1k commented 2 years ago

@BitPhinix Thank you for such a lightning reply and release!

If you asked about some details, I'll try to explain my needs in short: I believe that there could be different approaches to how to initialize your editor. I need all the stuff related to slate for collaborative needs encapsulated in different plugins. I need my editor to be able to work in poor network, so I need to sync as with backend (via websocket async provider but getting initial update (diffUpdate by local stateVector) via REST) as with local YDoc provider (e.g. custom indexeddb async provider). I need to turn off local provider or even backend sync plugin just by removing detaching plugin which is responsible for it.

So in my setup I split my collaboration functionalities by plugins:

Such an approach allows me to disable some features on demand. With such a set of plugins, I can have either only offline editor, or only online one, or both way. And most important here is that I can implement and add any other plugins next (or in the middle) to add different transport like webRTC.

Hope it's clear for now. Feel free to ask me questions if left.

Actually, current version of withYHistory in package also makes it hard to work with lazily initialized YDoc by plugins, coz. it now instantiates Y.UndoManager that requires already initialized e.sharedRoot before all plugins applied and editor created.