YousefED / SyncedStore

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

Feat/solid bindings #88

Open bigmistqke opened 1 year ago

bigmistqke commented 1 year ago

this PR adds solid-bindings to SyncedStore. The PR is quite minimal: Solid's createMutable proved to be an excellent candidate for wiring SyncedStore's reactivity with Solid.

Besides that I introduced an extra proxy-trap (i think, not sure about the terminology) in the array-getter.

Instead of mapping (like react), solid has control flow-components to deal with arrays (and objects with solid-primitive's Entries):

a react'sstore.todos.map(todo => todo.title) would be idiomatically written in solid like <For each={store.todos}>{todo => todo.title}</For>. <For each={store.todos}> was currently not working, as store.todos returned a reference to the Yjs-object (I assume) instead of the array of proxies. A spread was needed <For each={[...store.todos]}> which I believe would be cause of confusion and bugs when being used in a solid codebase.

An extra proxy-trap is introduced to allow an idiomatic use of arrays and objects in Solid's control flow components: in the Proxy-getter of array we check if the array is being accessed with a symbol that isn't one of SyncedStore's if (typeof p === "symbol" && p !== $reactiveproxy && p !== $reactive && p !== $skipreactive). If this is the case we return a map of the YjsReturnValues.

With this proxy-trap SyncedStore's store behaves very similar to createMutable (besides the fact that you can not set arrays directly in SyncedStore's store) and all control flow-components behave as you would expect.

I am unsure if this proxy-trap has any implications for other frameworks.

YousefED commented 1 year ago

@bigmistqke I'd love to merge this, but my knowledge of solidjs is limited. I'm trying to add a unit test to see if we can test whether nested updates trigger effects (see my latest commit). I'm probably doing something wrong when bootstrapping Solid unit tests. Would you have some time to have a look?

I now also commented out your Proxy trap, I'd like to test this later by creating a separate unit test with a For-construct (but we should first get a basic unit test working).

Thanks again for your contribution

bigmistqke commented 1 year ago

looking back at this pr... it really shouldn't be merged yet, was too optimistic lol. I'll set the PR to draft.

  1. Solid assumes we are in SSR-mode, since we are running on the server. Solid deactivates reactivity on the server for performance-reasons. There is currently not yet a DX-friendly way to have reactivity working on the server, the only way to go around it is to link directly to the browser-version of solid.
  2. const implicitStore1 = solid.createMutable(store); is this a common pattern? I don't think currently it is possible to wrap a syncedstore inside a createMutable, because of conflicting proxy-traps. I was under the assumption you would mutate the store directly?
  3. createEffect always runs when it is initialized, but this call is queued, after that it synchronously gets called when a reactive value is updated. playground to illustrate
  4. I updated the test, but it fails: there is no effect-call when updating the store. This is possibly a bug in my implementation. I also get the infamous Yjs was already imported. This breaks constructor checks and will lead to issues!-error.
YousefED commented 1 year ago

Alright!

2) This was just me trying to try getting the binding working, don't think it's necessary

4) maybe try cleaning node_modules (also in packages/core), make sure you're on node 16 (nvm use if using nvm) , and use npm install to install again. Or try a clean checkout?

edemaine commented 1 year ago

Regarding 1: Apparently Solid will soon support import ... from "@solidjs/reactivity", even server-side. I'm not sure whether it works now or whether this is a 2.0 thing, but at least it's coming.

bigmistqke commented 1 year ago

that is very exciting news, thanks for sharing.

bigmistqke commented 1 year ago

maybe try cleaning node_modules (also in packages/core), make sure you're on node 16 (nvm use if using nvm) , and use npm install to install again. Or try a clean checkout?

Just reinstalled with npm instead of pnpm and that did the job 😊 the updated test is passing now.