-
Notifications
You must be signed in to change notification settings - Fork 120
Add new useRoomConnection
hook for managing room connections
#1197
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
Changes from all commits
34e09cd
267d93d
80d7fa6
b6084aa
aec1262
dce6f2a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,97 @@ | ||
import { Mutex, type TrackPublishOptions, Room } from 'livekit-client'; | ||
import { useEffect, useMemo, useState } from 'react'; | ||
import { useMaybeRoomContext } from '../context'; | ||
|
||
/** @public */ | ||
export type UseRoomConnectionDetails = { | ||
serverUrl: string; | ||
participantToken: string; | ||
}; | ||
|
||
/** @public */ | ||
export type UseRoomConnectionOptions = { | ||
getConnectionDetails: () => Promise<UseRoomConnectionDetails>; | ||
onConnectionError?: (err: Error) => void; | ||
|
||
room?: Room; | ||
/** Should the room attempt to connect? If false, the room will disconnect. */ | ||
connected?: boolean; | ||
trackPublishOptions?: TrackPublishOptions; | ||
}; | ||
|
||
/** @public */ | ||
export type UseRoomConnectionResult = { | ||
room: Room; | ||
|
||
/** What operation is the useRoomConnection hook currently in the midst of performing? */ | ||
status: 'idle' | 'connecting' | 'disconnecting'; | ||
}; | ||
|
||
/** | ||
* The `useRoomConnection` hook provides a fully managed way to connect / disconnect from a LiveKit | ||
* room. To control whether the connection is active or not, use `options.connected`. | ||
* @remarks | ||
* Can be called inside a `RoomContext` or by passing a `Room` instance, otherwise creates a Room | ||
* itself. | ||
* | ||
* @example | ||
* ```tsx | ||
* const { room } = useRoomConnection({ | ||
* getConnectionDetails: async () => { | ||
* // compute the below value out of band: | ||
* return { serverUrl: "...", participantToken: "..." }; | ||
* }, | ||
* }); | ||
* ``` | ||
* @public | ||
*/ | ||
export function useRoomConnection(options: UseRoomConnectionOptions): UseRoomConnectionResult { | ||
const roomFromContext = useMaybeRoomContext(); | ||
const room = useMemo(() => { | ||
return roomFromContext ?? options.room ?? new Room(); | ||
}, [options.room, roomFromContext]); | ||
|
||
const connected = options.connected ?? true; | ||
|
||
const [status, setStatus] = useState<'idle' | 'connecting' | 'disconnecting'>('idle'); | ||
|
||
// NOTE: it would on the surface seem that managing a room's connection with a useEffect would be | ||
// straightforward, but `room.disconnect()` is async and useEffect doesn't support async cleanup | ||
// functions, which means `room.connect()` can run in the midst of `room.disconnect()`, causing | ||
// race conditions. | ||
const connectDisconnectLock = useMemo(() => new Mutex(), []); | ||
useEffect(() => { | ||
connectDisconnectLock.lock().then(async (unlock) => { | ||
if (connected) { | ||
setStatus('connecting'); | ||
const connectionDetails = await options.getConnectionDetails(); | ||
try { | ||
await Promise.all([ | ||
room.localParticipant.setMicrophoneEnabled( | ||
true, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this limits the usage of this hook to scenarios where users connect to rooms with their mic (and only their mic) enabled 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yea, you are correct, but this doesn't seem bad to me / was intentional. I suppose maybe it could be nice to be able to configure whether this |
||
undefined, | ||
options.trackPublishOptions, | ||
), | ||
room.connect(connectionDetails.serverUrl, connectionDetails.participantToken), | ||
]); | ||
} catch (error) { | ||
options.onConnectionError?.(error as Error); | ||
} finally { | ||
setStatus('idle'); | ||
} | ||
} else { | ||
setStatus('disconnecting'); | ||
try { | ||
await room.disconnect(); | ||
} catch (error) { | ||
options.onConnectionError?.(error as Error); | ||
} finally { | ||
setStatus('idle'); | ||
} | ||
} | ||
unlock(); | ||
}); | ||
}, [connected]); | ||
|
||
return { room, status }; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. returning the room here might lead to patterns that we don't really want to encourage, i.e. trying to call methods on the room itself again. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I had figured that was kinda the whole point, I'm not too well versed with this package yet. It sounds like the intention maybe is that the room would always be in the |
||
} |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we employ this pattern we have to think about what happens if retrieving connection details here fails. E.g. is a user expected to flip
connected
to false and back to true on an error?Also wondering if decoupling the two (connection and connectionDetails) makes more sense, as it can speed up the actual connection if connection Details retrieval happens in advance (e.g. like prefetch before you actually click)