YousefED / SyncedStore

SyncedStore CRDT is an easy-to-use library for building live, collaborative applications that sync automatically.
https://syncedstore.org
MIT License
1.74k stars 53 forks source link

Svelte Support #24

Closed zsaquarian closed 2 years ago

zsaquarian commented 2 years ago

This PR adds support for svelte. Unfortunately, because svelte reactivity is based on assignments, we need a dummy function. Other than that, usage is identical. We could also use svelte stores, but that would need a rewrite of the entire library as the crdt object would need to become a store.

YousefED commented 2 years ago

Awesome stuff @zsaquarian !

If I understand the refresh function correctly, I think it would mean that the app refreshes whenever any data inside SyncedStore changes, correct?

Ideally (this is similar to how the React / Vue bindings work), a refresh would only be triggered when the actually used (subscribed) data changes.

I suppose this is what you mean why we'd need to use Svelte Stores, right?

Would it be possible to create a store for every "atom" in useSvelteBindings, or is that not how Svelte stores work (I'd have to dive in more deeply to see if this suggetion makes sense)?

zsaquarian commented 2 years ago

Awesome stuff @zsaquarian!

Thanks! :)

If I understand the refresh function correctly, I think it would mean that the app refreshes whenever any data inside SyncedStore changes, correct?

Yes

Ideally (this is similar to how the React / Vue bindings work), a refresh would only be triggered when the actually used (subscribed) data changes.

I didn't really understand what you mean by subscribed data vs. any data. Is it something like this?

The store has a todos field and also other field. We only use store.todos, but the component rerenders even when store.other is changed.

If so, then it doesn't. I just checked it.

I suppose this is what you mean why we'd need to use Svelte Stores, right?

Perhaps I should have been clearer. refreshis just a function which assigns a variable to itself. (This doesn't refresh the page, it instead forcefully rerenders the component. Maybe I should change the name?). This is because svelte doesn't watch for changes in variables, but rather assignments to that variable. According to the docs:

Not all application state belongs inside your application's component hierarchy. Sometimes, you'll have values that need to be accessed by multiple unrelated components, or by a **regular JavaScript module.**

In Svelte, we do this with stores.

Would it be possible to create a store for every "atom" in useSvelteBindings, or is that not how Svelte stores work (I'd have to dive in more deeply to see if this suggetion makes sense)?

I tried that originally, but it seems svelte will only rerender a component if the store is directly used in that component. This basically means that the crdt type would have to become a store.

YousefED commented 2 years ago

The store has a todos field and also other field. We only use store.todos, but the component rerenders even when store.other is changed.

Yes, this is what I meant. If it doesn't rerender now, I think that's because there's no other component using store.other. My suspicion is that if you'd have two components (one using todos, and the other one using other), both would rerender if any of the values change. Could you confirm that?

zsaquarian commented 2 years ago

Yes, it seems like both of them rerender when only one is changed issue

YousefED commented 2 years ago

I think we can make this work using the Reactive library. Could you try something like this? (I haven't tested it yet)

import { readable } from 'svelte/store';
import { Observer, reactive } from "@reactivedata/reactive";

const svelteSyncedStore = (syncedStore: any) => {
    let set: any;
    const observer = new Observer(() => {
        if (set) {
            set(store);
        }
    });
    const store = reactive(syncedStore, observer);

    return readable(store, newSet => {
        set = newSet;

        return () => { set = undefined};
    });
};

In your component:

// where syncedStore would be the return value of the normal syncedStore({ todos: [] }) setup
const store = svelteSyncedStore(syncedStore); 
...

// now use $store
$store.todos[0].completed
zsaquarian commented 2 years ago

Unfortunately no :(

Even if we make it a writable store (so we can modify it), we would still have to do something like this:

$store.todos = [...$store.todos, newItem];

Which throws Uncaught Error: cannot set new elements on root doc. I don't think stores will do what we want them to do as this issue says that updating a store will trigger a rerender for all components using that store. :(

Also, I don't really see the problem with rerendering a component? Am I missing something?

YousefED commented 2 years ago

Which throws Uncaught Error: cannot set new elements on root doc.

That error is expected, the correct pattern for that would be $store.todos.push(newItem). Does that work?

Also, I don't really see the problem with rerendering a component? Am I missing something?

Performance wise, you would want to make sure only the relevant component(s) rerender if the synced data change. Especially when multiple developers / projects will depend on the library

zsaquarian commented 2 years ago

That error is expected, the correct pattern for that would be $store.todos.push(newItem). Does that work?

No, since there is no assignment there.

Performance wise, you would want to make sure only the relevant component(s) rerender if the synced data change. Especially when multiple developers / projects will depend on the library

Ah OK. I understand.

Also, fortunately, I seem to have made it work! Svelte has an afterUpdate method, and I just put a console.log in that method. All I changed is to have the Yjs doc in a separate module and import it into both of them, but not the crdt and useSvelteBindings call. This seems to be working properly now.

https://user-images.githubusercontent.com/79707147/141506735-38052e37-36ad-4744-b86c-1038513c53f7.mp4

Edit: the repetition at the top is because I am inserting that text into other. I didn't show it in the video, but that correctly updates only the other and not todos.

YousefED commented 2 years ago

Cool! I just gave it a shot as well, see https://github.com/YousefED/SyncedStore/tree/svelte.

What do you think? I'd move svelteSyncedStore to a separate module if we want to support it officially

zsaquarian commented 2 years ago

Awesome! I had meant that I had gotten my previous approach to work, but this is much more elegant. :)

What do you think? I'd move svelteSyncedStore to a separate module if we want to support it officially

I agree.

Thanks!

YousefED commented 2 years ago

This has been published in 0.3.4 and PR #25