Skip to content
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

Some topics on architecture #28

Open
kazekyo opened this issue Sep 21, 2020 · 1 comment
Open

Some topics on architecture #28

kazekyo opened this issue Sep 21, 2020 · 1 comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@kazekyo
Copy link

kazekyo commented Sep 21, 2020

Hi, thank you for making a great plugin.

We previously created our own plugin for our company based on this repository.
I'd like to share some points we noticed when we created it. We deleted it in the end because we no longer need collaborative editing features, though.
This is not an issue, so you can close this “issue” after you read it. I just want to pay you back.
I'm sorry I haven't had time to put out PRs.

Scalability of backend servers

As mentioned in this issue, there is a demand for making backend servers scalable.
We've scaled our servers through Redis.
Use this if you need it. https://github.com/kazekyo/automerge-spider

Eliminating the use of catch

https://github.com/cudr/slate-collaborative/search?q=catch&unscoped_q=catch
catch is used in this repository. I think this is dangerous.
Those catch break the synchronization of Automerge <=> Slate or Client <=> Backend(especially, vector clock synchronization in Automerge.Connection). In most cases, there is no automatic recovery.
This leads to claims like "Hey, the text I wrote wasn't saved!".
If there are errors that you can't resolve as a result of removing the catch, I will be happy to help you with your investigation.

Eliminating the use of temporary tree

https://github.com/cudr/slate-collaborative/blob/master/packages/bridge/src/convert/index.ts#L30-L35
I know this is not your idea, but I don’t think the temporary tree is a good idea.
If the create operation is not included in the Diff, no elements will be created in the temporary tree. So set_node may not work properly.
Also, using a temporary tree makes the code more complex.
Anyway, it is safer to generate slate operations with the docs before and after applying Diff.
I'll share the code.
https://github.com/kazekyo/collaborative-editing-fragment/blob/master/generateSlateOperation.ts#L198-L235


This is a part of our code that we can share. We didn't use cursors, so we don't have any code associated with them.
https://github.com/kazekyo/collaborative-editing-fragment

Again, thank you so much! You've saved us a lot of time.

@cudr
Copy link
Owner

cudr commented Sep 29, 2020

Hi, @kazekyo! Thanks for the interesting comments, I will try to look at them and possibly make improvements to the code base. Glad this library saved you time!

@cudr cudr added enhancement New feature or request help wanted Extra attention is needed good first issue Good for newcomers labels Oct 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants