FCE-2487: Overwrite MediaStream with the one from RN WebRTC #451
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses type inconsistencies between the web-based MediaStream and React Native WebRTC's MediaStream implementation by overwriting return types in the mobile client package. The changes ensure that hooks and components working with media streams use the correct RNMediaStream type, which includes the toURL() method required by React Native.
Changes:
- Updated mobile-client package to re-export hooks with
RNMediaStreamtypes instead of webMediaStream - Removed workaround type assertions (
MediaStreamWithURL) from example applications - Added main entry point configuration to video-player example
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/mobile-client/src/index.ts | Created wrapper functions for hooks to override MediaStream types with RNMediaStream; re-exported custom types |
| examples/mobile-client/video-player/package.json | Added main entry point field |
| examples/mobile-client/video-player/index.js | Added new root component registration file |
| examples/mobile-client/video-player/app.json | Added main entry point field |
| examples/mobile-client/video-player/components/FishjamPlayerViewer.tsx | Removed MediaStreamWithURL workaround; simplified stream usage |
| examples/mobile-client/video-player/components/FishjamPlayerStreamer.tsx | Removed MediaStreamWithURL workaround; simplified stream usage |
| examples/mobile-client/minimal-react-native/components/VideosGridItem.tsx | Removed MediaStreamWithURL workaround and TODO comments |
| examples/mobile-client/fishjam-chat/components/VideosGrid.tsx | Removed MediaStreamWithURL workaround and TODO comments |
| examples/mobile-client/fishjam-chat/app/room/preview.tsx | Removed MediaStreamWithURL workaround and unused videoRoomEnv parameter |
| examples/mobile-client/fishjam-chat/app/livestream/viewer.tsx | Removed MediaStreamWithURL workaround |
| examples/mobile-client/fishjam-chat/app/livestream/streamer.tsx | Removed MediaStreamWithURL workaround |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| export const VideosGridItem = ({ peer }: { peer: GridTrack }) => { | ||
| //TODO: FCE-2487 overwrite Track to include MediaStream from react-native-webrtc |
There was a problem hiding this comment.
issue (non-blocking): Should this comment be deleted as well? Task name matches.
|
|
||
| import App from './App'; | ||
|
|
||
| registerRootComponent(App); |
There was a problem hiding this comment.
question: Why was this needed?
There was a problem hiding this comment.
I'm not sure, but this example wasn't working without it.
| }; | ||
| } | ||
|
|
||
| export function useCamera(): Omit<ReturnType<typeof useCameraReactClient>, 'cameraStream'> & { |
There was a problem hiding this comment.
issue (blocking): Can we rewrite the types in pure typescript way? Right now if something changes in the react code it might not be reflected in mobile version. For example for the useCamera hook, it could be rewritten as:
export const useCamera = useCameraReactClient as () => Omit<
ReturnType<typeof useCameraReactClient>,
'cameraStream'
> & {
cameraStream: RNMediaStream | null;
};that way the output JS code will not change but only TS types.
Description
Overwrite MediaStream for mobile with the one from RN WebRTC.
Documentation impact
Types of changes
not work as expected)