automerge / mpl

a p2p document synchronization system for automerge
MIT License
281 stars 20 forks source link

Remove DeltaRouter, use Automerge.Connection instead #11

Closed ept closed 7 years ago

ept commented 7 years ago

This PR removes DeltaRouter from MPL, and instead uses the peer connection logic in automerge/automerge#36 (currently still on a branch). I have tried upgrading Trellis to use this version of MPL, and it seems to just work — I didn't notice any obvious problems in a brief test.

@pvh would you have a chance to review this sometime? I don't fully understand all the WebRTC stuff, so I'd appreciate a sanity check from you.

ept commented 7 years ago

I think I tracked down a remaining bug. When two Trellis instances connected, they initially wouldn't exchange changes until each instance had made at least one change. I think this happened because the initial message of the Automerge.Connection protocol (which advertises the sender's vector clock) was getting dropped by MPL.

To fix the issue, I added a simple queue to the Peer class in 141ed9f939b40fd5a9b18620f721dfbfffb78037: now, if Automerge attempts to send a message before the data channel is open, the message is enqueued instead of being dropped on the floor. This seems to do the job.

ept commented 7 years ago

This seems to be working smoothly now, so I'm merging the PR and making a release of the npm package.