Skip to content

fix(collab): rename useRoomActivity param provider_url → providerUrl #1485

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.

Sign up for GitHub

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

Closed
leecalcote wants to merge 1 commit into from
+11 −11
Closed

fix(collab): rename useRoomActivity param provider_url → providerUrl #1485

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions src/hooks/useRoomActivity.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ interface UserProfile {
}

interface CollaborationConfigParams {
provider_url: string;
providerUrl: string;
getUserProfile: () => Promise<{ data: UserProfile }>;
getUserAccessToken: () => Promise<{ data: string }>;
}
Expand Down Expand Up @@ -47,7 +47,7 @@ interface UserMapping {
}

interface UseRoomActivityParams {
provider_url?: string;
providerUrl?: string;
getUserProfile?: () => Promise<{ data: UserProfile }>;
getUserAccessToken?: () => Promise<{ data: string }>;
}
Comment on lines 49 to 53
Expand Down Expand Up @@ -100,7 +100,7 @@ const getSignalingUrlFromProviderUrl = (providerUrl: string): string => {
* Gets collaboration configuration for WebRTC
*/
export const getCollaborationConfig = async ({
provider_url,
providerUrl,
getUserProfile,
getUserAccessToken
}: CollaborationConfigParams): Promise<CollaborationConfig> => {
Expand All @@ -116,7 +116,7 @@ export const getCollaborationConfig = async ({
};

return {
signalingUrl: [getSignalingUrlFromProviderUrl(provider_url)],
signalingUrl: [getSignalingUrlFromProviderUrl(providerUrl)],
user: userProfile,
Comment on lines 102 to 120
authToken: accessToken,
refreshAuthToken: refreshToken,
Expand All @@ -135,18 +135,18 @@ export const getCollaborationConfig = async ({
const subscribeToRoomActivity = async (
wsRef: React.MutableRefObject<WebSocket | null>,
onUserMapChange: (userMap: UserMapping) => void,
provider_url: string,
providerUrl: string,
getUserProfile: () => Promise<{ data: UserProfile }>,
getUserAccessToken: () => Promise<{ data: string }>
): Promise<void> => {
if (!provider_url || !getUserProfile || !getUserAccessToken) {
if (!providerUrl || !getUserProfile || !getUserAccessToken) {
console.warn('Missing required parameters for subscription');
return;
}

try {
const config = await getCollaborationConfig({
provider_url,
providerUrl,
getUserProfile,
getUserAccessToken
});
Expand Down Expand Up @@ -187,7 +187,7 @@ const subscribeToRoomActivity = async (
*/
export const useRoomActivity = (
{
provider_url,
providerUrl,
getUserProfile,
getUserAccessToken
}: UseRoomActivityParams = {} as UseRoomActivityParams
Expand All @@ -197,11 +197,11 @@ export const useRoomActivity = (

useEffect(() => {
// -> all parameters check
if (provider_url && getUserProfile && getUserAccessToken) {
if (providerUrl && getUserProfile && getUserAccessToken) {
subscribeToRoomActivity(
wsRef,
setAllRoomsUserMapping,
provider_url,
providerUrl,
getUserProfile,
getUserAccessToken
);
Expand All @@ -215,7 +215,7 @@ export const useRoomActivity = (
ws.close();
}
};
}, [provider_url, getUserProfile, getUserAccessToken]);
}, [providerUrl, getUserProfile, getUserAccessToken]);
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The useEffect dependency array now includes providerUrl, getUserProfile, and getUserAccessToken. While this rename is correct, including these functions in the dependency array can lead to frequent WebSocket reconnections if the caller does not memoize them (e.g., using useCallback). Furthermore, there is a race condition in the cleanup logic (lines 210-217). The ws variable is captured from wsRef.current synchronously when the effect runs, but subscribeToRoomActivity is asynchronous and updates the ref later. This means the cleanup function may capture null and fail to close the newly opened WebSocket when the effect re-runs or the component unmounts. Consider refactoring the cleanup to access wsRef.current directly inside the returned function.


return [allRoomsUserMapping, wsRef];
};
Expand Down
Loading