search

joebobmiles / zustand-middleware-yjs

Zustand middleware that enables sharing of state between clients via Yjs.
MIT License
103 stars 10 forks source link

it seems to have some error with new version(1.3.0-rc.1) #41

Closed arvinxx closed 1 year ago

arvinxx commented 1 year ago

it doesn't sync

eror

arvinxx commented 1 year ago

I have maked a sandbox:

v1.2.8 work well: https://codesandbox.io/s/zustand-yjs-middleware-v1-2-8-eguvxj 123

v1.3.0-rc.1 doesn;t sync: https://codesandbox.io/s/zustand-yjs-middleware-new-version-30uvnb

234

joebobmiles commented 1 year ago

I'm going to have to return to this with renewed patience at a later time. For some reason, your CodeSandboxes either 1) don't load (Firefox) or 2) don't compile (Chrome). When I tried one of the projects from the repository's examples folder, I couldn't get my WebRTC provider to connect to the new signaling server that dmonad added a few weeks ago in either Chrome or Firefox.

However, there was a fleeting period where I did see the error you reported, so I know it happens. Still, I can't reliably reproduce it due to the signaling server fiasco. My best bet now would be to spin up my own signaling server, and I need to set aside more time to tackle that.

arvinxx commented 1 year ago

Thanks for your replying. Take your time to work on it~ 😁

joebobmiles commented 1 year ago

OK, using a local signaling server, I can't replicate your issue with the counter example project. But I have found a different issue and I don't like the implications of it.

The issue I have found is that all of the instanceof checks in my patching code are failing when checking Yjs shared type objects, even when, as far as I'm aware, they shouldn't be. This leads to no data being transferred between peers.

Of course, that's not what your seeing - your seeing data transfer, just no update when the state is being updated by a peer. But I have a sneaking suspicion that this instanceof nonsense is related.

arvinxx commented 1 year ago

maybe, you can try to fix it and then let me try~

joebobmiles commented 1 year ago

Double-check your CodeSandbox. I can't reproduce the exact bug you reported. Instead, I see the same bug I saw this morning... And I am still trying to figure out why. At least the good news is that both my local and your CodeSandbox are exhibiting the same behavior, so I'm not hunting down a Heisen-bug.

Now I need to figure out how to reproduce it in a test.

joebobmiles commented 1 year ago

Ok, so I pushed a new release candidate, but there's no fix. It's just a lot of upgrades for heavily out of date dependencies to see if that helps.

arvinxx commented 1 year ago

it seems still don't work on 1.3.0-rc.2

joebobmiles commented 1 year ago

Ok, finally got back around to this.

As I slowly step through the code when a remote update is received, it seems that Zustand store is being improperly modified. This probably has something to do with how the middleware registers itself with Zustand, so I'm going to look into middleware best practices.

joebobmiles commented 1 year ago

Hmph.

It's not an issue with how the middleware registers with Zustand. The middleware is side-stepping Zustand's setState() by modifying a reference to Zustand's underlying state object when a remote update is received. I need to figure out how to duplicate this locally to get a closer look at what is happening.

joebobmiles commented 1 year ago

OK. RC 3 is live. Tested a fork of your CodeSandbox, and everything is in working order. When you are able, test it yourself to make sure everything is working as you expect.

arvinxx commented 1 year ago

it works well! Thanks for your great job~