-
Notifications
You must be signed in to change notification settings - Fork 151
Add room connect disconnect hack #1199
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
Merged
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
3a2fd8b
feat: add useSequentialRoomConnectDisconnect to fix react useEffect r…
1egoman c5622cf
feat: add changeset
1egoman c873b3e
feat: make api-extractor happy
1egoman 0a7c1c4
fix: run npm run format:write
1egoman 0e03334
refactor: remove docs comment
1egoman d416b8d
feat: add logging when rapid room connect / disconnect occurs
1egoman 55f20a3
feat: add logic to ensure that room can change reference
1egoman 074d821
feat: add warning if room changed reference too often
1egoman 1e7b44e
fix: run npm run format:write
1egoman 67e41a3
fix: add api-extractor data
1egoman e566b8a
feat: adjust type formatting
1egoman File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@livekit/components-react': patch | ||
| --- | ||
|
|
||
| add useSequentialRoomConnectDisconnect to fix react useEffect room connection issue |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
171 changes: 171 additions & 0 deletions
171
packages/react/src/hooks/useSequentialRoomConnectDisconnect.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,171 @@ | ||
| import { Mutex, type Room } from 'livekit-client'; | ||
| import { useCallback, useEffect, useMemo, useRef } from 'react'; | ||
| import { log } from '@livekit/components-core'; | ||
|
|
||
| const CONNECT_DISCONNECT_WARNING_THRESHOLD_QUANTITY = 2; | ||
| const CONNECT_DISCONNECT_WARNING_THRESHOLD_MS = 400; | ||
|
|
||
| const ROOM_CHANGE_WARNING_THRESHOLD_QUANTITY = 3; | ||
| const ROOM_CHANGE_WARNING_THRESHOLD_MS = 1000; | ||
|
|
||
| /** @public */ | ||
| export type UseSequentialRoomConnectDisconnectResults<R extends Room | undefined> = { | ||
| connect: typeof Room.prototype.connect & (R extends undefined ? null : unknown); | ||
| disconnect: typeof Room.prototype.disconnect & (R extends undefined ? null : unknown); | ||
| }; | ||
|
|
||
| /** | ||
| * When calling room.disconnect() as part of a React useEffect cleanup function, it is possible for | ||
| * a room.connect(...) in the effect body to start running while the room.disconnect() is still | ||
| * running. This hook sequentializes these two operations, so they always happen in order and | ||
| * never overlap. | ||
| * | ||
| * @example | ||
| * ```ts | ||
| * const { connect, disconnect } = useSequentialRoomConnectDisconnect(room); | ||
| * | ||
| * // Connecting to a room: | ||
| * useEffect(() => { | ||
| * connect(); | ||
| * return () => disconnect(); | ||
| * }, [connect, disconnect]); | ||
| * ``` | ||
| * | ||
| * @public | ||
| */ | ||
| export function useSequentialRoomConnectDisconnect<R extends Room | undefined>( | ||
| room: R, | ||
| ): UseSequentialRoomConnectDisconnectResults<R> { | ||
| const connectDisconnectQueueRef = useRef< | ||
| Array< | ||
| | { | ||
| type: 'connect'; | ||
| room: Room; | ||
| args: Parameters<typeof Room.prototype.connect>; | ||
| resolve: (value: Awaited<ReturnType<typeof Room.prototype.connect>>) => void; | ||
| reject: (err: Error) => void; | ||
| } | ||
| | { | ||
| type: 'disconnect'; | ||
| room: Room; | ||
| args: Parameters<typeof Room.prototype.disconnect>; | ||
| resolve: (value: Awaited<ReturnType<typeof Room.prototype.disconnect>>) => void; | ||
| reject: (err: Error) => void; | ||
| } | ||
| > | ||
| >([]); | ||
|
|
||
| // Process room connection / disconnection events and execute them in series | ||
| // The main queue is a ref, so one invocation of this function can continue to process newly added | ||
| // events | ||
| const processConnectsAndDisconnectsLock = useMemo(() => new Mutex(), []); | ||
| const processConnectsAndDisconnects = useCallback(async () => { | ||
| return processConnectsAndDisconnectsLock.lock().then(async (unlock) => { | ||
| while (true) { | ||
| const message = connectDisconnectQueueRef.current.pop(); | ||
| if (!message) { | ||
| unlock(); | ||
| break; | ||
| } | ||
|
|
||
| switch (message.type) { | ||
| case 'connect': | ||
| await message.room | ||
| .connect(...message.args) | ||
| .then(message.resolve) | ||
| .catch(message.reject); | ||
| break; | ||
| case 'disconnect': | ||
| await message.room | ||
| .disconnect(...message.args) | ||
| .then(message.resolve) | ||
| .catch(message.reject); | ||
| break; | ||
| } | ||
| } | ||
| }); | ||
| }, []); | ||
|
|
||
| const roomChangedTimesRef = useRef<Array<Date>>([]); | ||
| const checkRoomThreshold = useCallback((now: Date) => { | ||
| let roomChangesInThreshold = 0; | ||
| roomChangedTimesRef.current = roomChangedTimesRef.current.filter((i) => { | ||
| const isWithinThreshold = now.getTime() - i.getTime() < ROOM_CHANGE_WARNING_THRESHOLD_MS; | ||
| if (isWithinThreshold) { | ||
| roomChangesInThreshold += 1; | ||
| } | ||
| return isWithinThreshold; | ||
| }); | ||
|
|
||
| if (roomChangesInThreshold > ROOM_CHANGE_WARNING_THRESHOLD_QUANTITY) { | ||
| log.warn( | ||
| `useSequentialRoomConnectDisconnect: room changed reference rapidly (over ${ROOM_CHANGE_WARNING_THRESHOLD_QUANTITY}x in ${ROOM_CHANGE_WARNING_THRESHOLD_MS}ms). This is not recommended.`, | ||
| ); | ||
| } | ||
| }, []); | ||
|
|
||
| // When the room changes, clear any pending connect / disconnect calls and log when it happened | ||
| useEffect(() => { | ||
| connectDisconnectQueueRef.current = []; | ||
|
|
||
| const now = new Date(); | ||
| roomChangedTimesRef.current.push(now); | ||
| checkRoomThreshold(now); | ||
| }, [room, checkRoomThreshold]); | ||
|
|
||
| const connectDisconnectEnqueueTimes = useRef<Array<Date>>([]); | ||
| const checkConnectDisconnectThreshold = useCallback((now: Date) => { | ||
| let connectDisconnectsInThreshold = 0; | ||
| connectDisconnectEnqueueTimes.current = connectDisconnectEnqueueTimes.current.filter((i) => { | ||
| const isWithinThreshold = | ||
| now.getTime() - i.getTime() < CONNECT_DISCONNECT_WARNING_THRESHOLD_MS; | ||
| if (isWithinThreshold) { | ||
| connectDisconnectsInThreshold += 1; | ||
| } | ||
| return isWithinThreshold; | ||
| }); | ||
|
|
||
| if (connectDisconnectsInThreshold > CONNECT_DISCONNECT_WARNING_THRESHOLD_QUANTITY) { | ||
| log.warn( | ||
| `useSequentialRoomConnectDisconnect: room connect / disconnect occurring in rapid sequence (over ${CONNECT_DISCONNECT_WARNING_THRESHOLD_QUANTITY}x in ${CONNECT_DISCONNECT_WARNING_THRESHOLD_MS}ms). This is not recommended and may be the sign of a bug like a useEffect dependency changing every render.`, | ||
| ); | ||
| } | ||
| }, []); | ||
|
|
||
| const connect = useCallback( | ||
| async (...args: Parameters<typeof Room.prototype.connect>) => { | ||
| return new Promise((resolve, reject) => { | ||
| if (!room) { | ||
| throw new Error('Called connect(), but room was unset'); | ||
| } | ||
| const now = new Date(); | ||
| checkConnectDisconnectThreshold(now); | ||
| connectDisconnectQueueRef.current.push({ type: 'connect', room, args, resolve, reject }); | ||
| connectDisconnectEnqueueTimes.current.push(now); | ||
| processConnectsAndDisconnects(); | ||
| }); | ||
| }, | ||
| [room, checkConnectDisconnectThreshold, processConnectsAndDisconnects], | ||
| ); | ||
|
|
||
| const disconnect = useCallback( | ||
| async (...args: Parameters<typeof Room.prototype.disconnect>) => { | ||
| return new Promise((resolve, reject) => { | ||
| if (!room) { | ||
| throw new Error('Called discconnect(), but room was unset'); | ||
| } | ||
| const now = new Date(); | ||
| checkConnectDisconnectThreshold(now); | ||
| connectDisconnectQueueRef.current.push({ type: 'disconnect', room, args, resolve, reject }); | ||
| connectDisconnectEnqueueTimes.current.push(now); | ||
| processConnectsAndDisconnects(); | ||
| }); | ||
| }, | ||
| [room, checkConnectDisconnectThreshold, processConnectsAndDisconnects], | ||
| ); | ||
|
|
||
| return { | ||
| connect: room ? connect : null, | ||
| disconnect: room ? disconnect : null, | ||
| } as UseSequentialRoomConnectDisconnectResults<R>; | ||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.