FCE-2596 Add support for data channels #447
Conversation
Qizot
changed the title
There was a problem hiding this comment.
Pull request overview
This PR adds support for data channels to the WebRTC client, enabling bidirectional binary data transmission between peers through reliable and lossy channels.
Changes:
- Implemented
DataChannelManagerandDataChannelclasses to handle WebRTC data channel lifecycle and messaging - Added public APIs (
createDataPublishers,publishData,subscribeData) across webrtc-client, ts-client, and react-client packages - Created a React chat example application demonstrating data channel usage
Reviewed changes
Copilot reviewed 25 out of 27 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/webrtc-client/src/webRTCEndpoint.ts | Added data channel manager integration and public API methods for data publishing |
| packages/webrtc-client/src/types.ts | Defined data channel configuration types, events, and message payload interfaces |
| packages/webrtc-client/src/logger.ts | Added debug logging level |
| packages/webrtc-client/src/index.ts | Exported new data channel types |
| packages/webrtc-client/src/dataChannels/DataChannelManager.ts | Core manager for creating and coordinating reliable/lossy data channels |
| packages/webrtc-client/src/dataChannels/DataChannel.ts | Wrapper class for RTCDataChannel with event handling and type conversion |
| packages/webrtc-client/src/ConnectionManager.ts | Added createDataChannel method |
| packages/ts-client/src/types.ts | Added data publisher configuration and events |
| packages/ts-client/src/index.ts | Exported data channel types |
| packages/ts-client/src/FishjamClient.ts | Integrated data publisher methods and event forwarding |
| packages/react-client/src/types/public.ts | Defined React hook return type for data publisher |
| packages/react-client/src/index.ts | Exported data publisher hook and types |
| packages/react-client/src/hooks/useDataPublisher.ts | React hook for data channel operations |
| packages/react-client/src/FishjamProvider.tsx | Added dataPublisher prop |
| packages/react-client/.tool-versions | Fixed Node.js version format |
| examples/react-client/chat/* | Complete chat application demonstrating data channels |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 32 out of 34 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const client = fishjamClientRef.current; | ||
| const [isConnected, setIsConnected] = useState(false); | ||
| const [ready, setReady] = useState(false); | ||
| const [loading, setLoading] = useState(false); |
There was a problem hiding this comment.
question: do we need the loading state? isn't it implied that (!ready && !error) === loading?
There was a problem hiding this comment.
if you don't call initialize then you have both !ready and !error which is not necessairly loading.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 30 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
czerwiukk
left a comment
czerwiukk
left a comment
There was a problem hiding this comment.
regarding the code - it's fine for me, but i'd change the name of the feature to useDataChannel, also it'd be good to add a js hook to the room explaining that the messages are going to be published to the room the peer is currently connected to
Description
Describe your changes.
Motivation and Context
Why is this change required? What problem does it solve? If it fixes an open
issue, please link to the issue here.
Documentation impact
Types of changes
not work as expected)