Skip to content

refactor(ui-voip): Simplify useAvailableViewTracker logic and revert currentViews type from Set to Array #41687

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
gabriellsh wants to merge 1 commit into from
Closed

refactor(ui-voip): Simplify useAvailableViewTracker logic and revert currentViews type from Set to Array #41687

Show file tree
Hide file tree
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
4 changes: 2 additions & 2 deletions packages/ui-voip/src/context/MediaCallInstanceContext.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export type MediaCallInstanceContextValue = {
audioElement: RefObject<HTMLAudioElement | null> | undefined;
openRoomId: string | undefined;

currentViews: Set<AvailableViews>;
currentViews: Array<AvailableViews>;
registerView: RegisterView;
unregisterView: UnregisterView;

Expand All @@ -37,7 +37,7 @@ export const defaultContextValue = {
openRoomId: undefined,
setOpenRoomId: () => undefined,
getAutocompleteOptions: () => Promise.resolve([]),
currentViews: new Set<AvailableViews>(),
currentViews: [],
registerView: () => undefined,
unregisterView: () => undefined,
openWidget: () => undefined,
Expand Down
40 changes: 20 additions & 20 deletions packages/ui-voip/src/providers/useAvailableViewTracker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useCallback, useRef, useState } from 'react';
import { useCallback, useMemo, useState } from 'react';

import type { AvailableViews } from '../context/MediaCallInstanceContext';

Expand All @@ -13,35 +13,35 @@ const filter = (view: AvailableViews, _index: number, array: AvailableViews[]) =
}
};

const getViewsSetStateAction =
(filteredViews: AvailableViews[]) =>
(prev: Set<AvailableViews>): Set<AvailableViews> => {
if (filteredViews.length === prev.size && filteredViews.every((view) => prev.has(view))) {
return prev;
}
return new Set(filteredViews);
};

const useAvailableViewTracker = () => {
const viewsRef = useRef<Set<AvailableViews>>(new Set<AvailableViews>());
const [currentViews, setCurrentViews] = useState<Set<AvailableViews>>(new Set<AvailableViews>());
const [views, setViews] = useState<Set<AvailableViews>>(new Set<AvailableViews>());

const registerView = useCallback((view: AvailableViews) => {
if (viewsRef.current.has(view)) return;
setViews((prev) => {
if (prev.has(view)) {
return prev;
}

prev.add(view);

viewsRef.current.add(view);
const filteredViews = [...viewsRef.current].filter(filter);
setCurrentViews(getViewsSetStateAction(filteredViews));
return new Set(prev);
});
}, []);

const unregisterView = useCallback((view: AvailableViews) => {
if (!viewsRef.current.has(view)) return;
setViews((prev) => {
if (!prev.has(view)) {
return prev;
}

viewsRef.current.delete(view);
const filteredViews = [...viewsRef.current].filter(filter);
setCurrentViews(getViewsSetStateAction(filteredViews));
prev.delete(view);

return new Set(prev);
});
}, []);
Comment on lines 19 to 41

@coderabbitai coderabbitai Bot Aug 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "## candidate file"
wc -l packages/ui-voip/src/providers/useAvailableViewTracker.ts 2>/dev/null || true
cat -n packages/ui-voip/src/providers/useAvailableViewTracker.ts 2>/dev/null || true

echo
echo "## package React version references"
rg -n '"react"|"`@types/react`"|react@' -S package.json packages 2>/dev/null | head -200 || true

Repository: RocketChat/Rocket.Chat

Length of output: 3998

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "## React 19.2.7 runtime/pure updater behavior probe"
node - <<'JS'
function cloneBeforeAdd(prev, view) {
  if (prev.has(view)) return prev;
  return new Set(prev).add(view);
}

function mutateThenClone(prev, view) {
  if (prev.has(view)) return prev;
  prev.add(view);
  return new Set(prev);
}

function cloneBeforeDelete(prev, view) {
  if (prev.has(view)) return new Set(prev).delete(view) ? prev : prev;
  return prev;
}

function mutateThenCloneDelete(prev, view) {
  if (!prev.has(view)) return prev;
  prev.delete(view);
  return new Set(prev);
}

function withStrictMode(updater, initialState, events) {
  const states = [initialState];
  for (const [v] of events) {
    const replay = updater(states[states.length - 1], v);
    const actual = update ? updater(updater(states[states.length - 1], v), v) : updater(replay, v);
    states.push(actual);
  }
  return states;
}

const view = 'widget';
const init = new Set();
console.log('cloneBeforeAdd', JSON.stringify(Array.from(withStrictMode(cloneBeforeAdd, init, [[1], [2]]).map(s => Array.from(s)))));
console.log('mutateThenClone', JSON.stringify(Array.from(mutateThenClone, init, [[1], [2]]).map(s => Array.from(s))));
JS

Repository: RocketChat/Rocket.Chat

Length of output: 888

🌐 Web query:

React useState reducer/updater should be pure input can be mutated by future renders documentation

💡 Result:

In React, updater functions passed to state setters (such as those from useState or useReducer) must be pure [1][2][3]. They should take the pending state as their only argument and return the next state without modifying any pre-existing variables or objects [1][4][3]. You must not mutate the state inside an updater function [3]. Because React may call your updater function multiple times—specifically in development mode (Strict Mode) to help detect impurities—any mutation will result in observable, incorrect behavior, such as state being updated more times than intended [1][4][3]. The fundamental rules for updater functions are: 1. Purity: They must be pure, meaning they produce the same output for the same input and have no side effects [1][5]. 2. Immutability: Treat state as immutable [6][7]. Instead of mutating the existing state object or array (e.g., using.push or direct property assignment), always return a new object or array that represents the updated state [6][3][7]. 3. No Side Effects: Updater functions should only calculate and return the new state; they should not trigger side effects (such as making network requests, modifying the DOM, or setting other state variables) [2][5]. React uses these functions to calculate the next state during the render phase [2]. If an updater function is impure, the extra calls in Strict Mode will reveal the mistake by causing bugs, such as duplicated additions to an array, which would not happen if the function simply returned a new, unmutated copy of the data [1][3].

Citations:

Clone the previous state set before changing it.

registerView and unregisterView mutate prev, which React updater functions should treat as immutable. Clone before add or delete so the updater returns a different set object only when the view actually changed.

Proposed fix 
 			if (prev.has(view)) {
 				return prev;
 			}
 
-			prev.add(view);
-			return new Set(prev);
+			const next = new Set(prev);
+			next.add(view);
+			return next;
 		});
 	}, []);
@@
 			if (!prev.has(view)) {
 				return prev;
 			}
 
-			prev.delete(view);
-			return new Set(prev);
+			const next = new Set(prev);
+			next.delete(view);
+			return next;
 		});
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const registerView = useCallback((view: AvailableViews) => {
if (viewsRef.current.has(view)) return;
setViews((prev) => {
if (prev.has(view)) {
return prev;
}
prev.add(view);
viewsRef.current.add(view);
const filteredViews = [...viewsRef.current].filter(filter);
setCurrentViews(getViewsSetStateAction(filteredViews));
return new Set(prev);
});
}, []);
const unregisterView = useCallback((view: AvailableViews) => {
if (!viewsRef.current.has(view)) return;
setViews((prev) => {
if (!prev.has(view)) {
return prev;
}
viewsRef.current.delete(view);
const filteredViews = [...viewsRef.current].filter(filter);
setCurrentViews(getViewsSetStateAction(filteredViews));
prev.delete(view);
return new Set(prev);
});
}, []);
const registerView = useCallback((view: AvailableViews) => {
setViews((prev) => {
if (prev.has(view)) {
return prev;
}
const next = new Set(prev);
next.add(view);
return next;
});
}, []);
const unregisterView = useCallback((view: AvailableViews) => {
setViews((prev) => {
if (!prev.has(view)) {
return prev;
}
const next = new Set(prev);
next.delete(view);
return next;
});
}, []);
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/ui-voip/src/providers/useAvailableViewTracker.ts` around lines 19 -
41, Update the state updaters in registerView and unregisterView to clone prev
before calling add or delete, preserving the early returns when the view is
already registered or absent. Return the cloned set only when the view actually
changes.


const currentViews = useMemo(() => [...views].filter(filter), [views]);

return {
currentViews,
registerView,
Expand Down
2 changes: 1 addition & 1 deletion packages/ui-voip/src/views/MediaCallPopout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const MediaCallPopout = () => {

useEffect(() => {
queueMicrotask(() => {
if (currentViews.has('popout') && callId) {
if (currentViews.includes('popout') && callId) {
void openPopoutWindow(callId);
return;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/ui-voip/src/views/MediaCallRoomSection/MediaCallRoomSection.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ const MediaCallRoomSection = ({ showChat, onToggleChat, user, containerHeight }:
} = useMediaCallView();
const { currentViews } = useMediaCallInstance();

const isPopout = currentViews.has('popout');
const isPopout = currentViews.includes('popout');

const { muted, held, peerInfo, connectionState, startedAt } = sessionState;

Expand Down
2 changes: 1 addition & 1 deletion packages/ui-voip/src/views/MediaCallWidget/MediaCallWidget.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const MediaCallWidget = () => {

const widgetVisible = targetWidgetVisibility === 'open' || state !== 'none';

if (hidden || !currentViews.has('widget') || !widgetVisible) {
if (hidden || !currentViews.includes('widget') || !widgetVisible) {
return null;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/ui-voip/src/views/MediaCallWidget/OngoingCallWithScreen.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const OngoingCall = () => {
} = useMediaCallView();
const { muted, held, remoteMuted, remoteHeld, peerInfo, connectionState, startedAt } = sessionState;
const { currentViews } = useMediaCallInstance();
const isPopout = currentViews.has('popout');
const isPopout = currentViews.includes('popout');

const { localScreen, remoteScreen } = streams;

Expand Down
Loading