Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] refactoring of y-webrtc in typescript #31

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

YousefED
Copy link
Contributor

@YousefED YousefED commented Oct 31, 2021

For a different project, I had to make some modifications to y-webrtc. To understand the code and to be able to make some modifications, I decided to implement a refactoring and convert the existing code to Typescript.

There might not be any desire to migrate y-webrtc to TypeScript, so I understand if there is no appetite for this PR to be merged. Still opening this issue as it might be useful for other community members to understand the internals of y-webrtc, but feel free to close it.

This PR is a conversion to typescript + a refactoring of y-webrtc. The following concepts have been refactored:

  • classes moved to separate files
  • Most code in y-webrtc was for handling broadcastchannel, signalling and setting up webrtc channels. I've made this explicit by creating a BaseWebrtcProvider.ts provider which handles setting up these connection. Then, the only yjs specific code is implemented in WebrtcProvider.ts. Messages passed over the channels specific to connections / peers are handled in 'Room.ts'. Application / yjs specific messages are then wrapped in a customMessage and handled in WebrtcProvider.ts:onCustomMessage:
  • We define a onPeerConnected and onCustomMessage callback that are implemented in WebrtcProvider.ts to send messages in response to initiating a new connection, and in response to custom messages from peers. This means yjs specific logic for broadcast channel vs webrtc peers are now using the same code. The original code had custom behaviour for initiating broadcastchannels (writing messageQueryAwareness, and immediately writing syncstep2). This has now been removed. I don't think this has any impact, but this should be verified

TODOs:
[ ] "synced" event has not been implemented yet
[ ] I'm not an expert of rollup, so have now moved to microbundle. Maybe sticking to rollup is better
[ ] related to the previous point; the demo and test have not been converted

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant