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

Discussion: valtio-yjs binding #5

Closed dai-shi closed 2 years ago

dai-shi commented 3 years ago

Hi, nice work! I'm the author of valtio, and thinking to develop a library valtio-yjs (no repo yet), which binds those two worlds. My plan is to use both public APIs, and create two-way bindings. After I spent a couple of hours, I noticed it's not that trivial. (And then got busy with other stuff.) There might be something I can learn from your work and here's the start of discussion.

I'd be more than happy if we could collaborate on it, but as I see reactive and valtio are different API-wise and maybe for some base concepts. So, not sure what is feasible.

YousefED commented 3 years ago

Hi @dai-shi, excited to connect!

So I spend a bunch of hours trying to implement this in an as abstract way possible, but I'm running into a bit of an architectural issue.

While it was easy to add support for vue / mobx, adding support for other libraries with an "implicit observer" (as discussed on twitter https://twitter.com/YousefED/status/1390365307496189952). Is a bit more cumbersome. Let me explain:

https://github.com/YousefED/reactive-crdt/tree/main/packages/yjs-reactive-bindings is the package that "patches" Yjs to make it observable. In https://github.com/YousefED/reactive-crdt/blob/main/packages/yjs-reactive-bindings/src/observableProvider.ts you can see the different ways it works for Vue Reactive and MobX. It uses a createAtom pattern from Mobx, and for Vue it just creates a new observable as an "atom".

To support "implicit observers", we need to know the "calling context". For my Reactive library, I've solved this in a custom way (part 1, part 2). You can see this is a bit hacky and I'd like to remove the "reactive" library specific code so we can cleanly support both "reactive" and valtio.

Unfortunately, trying to figure out a cleaner solution I ended up in proxy-hell 😂. If you can follow-along, perhaps you have suggestions on how to solve / think about this differently.

Alternatively, if there's an easy way to retrieve the "implicit observer" in Valtio, we can also solve it by adding support for that.

YousefED commented 3 years ago

Update: I see valtio uses a concept of snapshots instead of "implicit observers", which is a bit different. On useSnapshot you create a copy of the entire state. I'm not sure that is ideal for Yjs / reactive-crdt, as it would require reading out all Yjs objects I think, which might have a performance impact. Open to suggestions!

dai-shi commented 3 years ago

valtio's useSnapshot doesn't copy the entire object, but you get the idea correctly. valtio's state will be the entire copy of yjs objects (even if it's vanilla without react). I'm not familiar with yjs implementation, so not sure how it would impact the performance, but that would be only the way to bridge yjs and valtio. Apart from doubling the memory usage, I guess we could only copy things that are changed, right?

dai-shi commented 3 years ago

https://github.com/YousefED/reactive-crdt#motivation

Instead of data types like Y.Map, and Y.Array, can we just use plain Javascript objects and arrays?

This is exactly the same motivation of mine.

YousefED commented 3 years ago

valtio's useSnapshot doesn't copy the entire object, but you get the idea correctly. valtio's state will be the entire copy of yjs objects (even if it's vanilla without react). I'm not familiar with yjs implementation, so not sure how it would impact the performance, but that would be only the way to bridge yjs and valtio. Apart from doubling the memory usage, I guess we could only copy things that are changed, right?

I think that approach is possible (valtio can read out all properties from a reactive-crdt crdt object). But it will be tricky to trigger the right state changes on the valtio "copy". It should be possible but I'm not sure it's the ideal solution. It's also a different model than I'm used to, so might be easier to explore in a call.


Fortunately, I think I've been able to bridge reactive-crdt and valtio's subscribe function differently. It uses a getter function which basically acts as a bridge between both worlds. I think it should work fine and also doesn't require (possibly expensive) copying of all Yjs values. Curious to hear what you think! See https://github.com/YousefED/reactive-crdt/blob/valtio-2/packages/reactive-crdt/test/valtio.test.ts

dai-shi commented 3 years ago

OK, now I started to understand what you are trying to do. You already have a base reactive feature and vue binding. So, the latter binding could be for valtio.

    // simulate useSnapshot. Not sure this is a good simulation, better to set up React test with actual useSnapshot
    let snap = proxy(snapshot(valtioStore));

This is not correct. We don't proxy snapshots. For now, you can forget about useSnapshot, it's a wrapper hook for render optimization with subscribe and snapshot.

So, what we need to test is subscribe and snapshot.


let value = valtioStore.store().property;

My goal is to make this like this:

let value = valtioStore.property;

So, the basic test would be like this.

const yDoc = new Y.Doc();
const valtioStore = valtioCrdt<{ property: number }>(yDoc);

expect(valtioStore.property).toBe(undefined)
yDoc.getMap().set("property", 4);
expect(valtioStore.property).toBe(4)
yDoc.getMap().set("property", 5);
expect(valtioStore.property).toBe(5)

We can add more tests for subscribe and snapshot in addition. It feels like the goal is different, or it's just wip.

YousefED commented 3 years ago

Thanks. I understand you'd prefer let value = valtioStore.property;. This is also how I designed the regular reactive-crdt API to work. However, I'm not sure we can get that to work without modifying Valtio (and my knowledge of Valtio is running into limits here, I'm happy I was able to figure out the workaround).

For example, trying the following proxy(crdt(doc)); won't work, as Valtio doesn't seem to work well with Proxy objects. crdt returns a Proxy, but it is "unproxyfied" in this line: https://github.com/pmndrs/valtio/blob/b7145366da356cd1ec5b7143dd7ec9651a179806/src/vanilla.ts#L96

dai-shi commented 3 years ago

Okay, this conversation motivates me to work on valtio-yjs. I'm not sure how it goes, but having someone to discuss with is super nice.

YousefED commented 3 years ago

Okay, this conversation motivates me to work on valtio-yjs. I'm not sure how it goes, but having someone to discuss with is super nice.

Haha ok! I'd be happy to jump on a call, so we don't have to do duplicate work :) It should definitely be possible to bridge both libraries but I think it would be easier to figure out together.

dai-shi commented 3 years ago

https://github.com/dai-shi/valtio-yjs/ So, here's my incomplete trial. I still need to have better understanding on yjs behavior to proceed.

dai-shi commented 3 years ago

Hi, I made a good progress on valtio-yjs (published as v0.1.0), and want to share it.

Here's how it can be used.

import * as Y from "yjs";
import { proxy } from "valtio";
import { bindProxyAndYMap } from "valtio-yjs";

// create a new Y doc
const ydoc = new Y.Doc();

// create a Y map
const ymap = ydoc.getMap("mymap");

// create a valtio state
const state = proxy({});

// bind them
bindProxyAndYMap(state, ymap);

// now you can mutate the state
state.text = 'hello';

// you can nest objects
state.obj = { count: 0 };

// and mutate the nested object value
++state.obj.count;

// you can use arrays too
state.arr = [1, 2, 3];

// mutating the array is also possible
state.arr.push(4);
YousefED commented 2 years ago

I'm closing this for now (valtio-yjs is available as an alternative. If there's more interest in valtio for SyncedStore, we can reconsider).