regression(ui-voip): Switching between alternate voice call views breaks widget visibility

packages/ui-voip/src/providers/useAvailableViewTracker.spec.ts

@@ -0,0 +1,42 @@
+import { act, renderHook } from '@testing-library/react';
+import { StrictMode } from 'react';
+
+import useAvailableViewTracker from './useAvailableViewTracker';
+
+describe('useAvailableViewTracker', () => {
+	it('should track registered views under StrictMode, which calls state updaters twice', () => {
+		const { result } = renderHook(() => useAvailableViewTracker(), { wrapper: StrictMode });
+
+		act(() => result.current.registerView('popout'));
+		expect(result.current.currentViews.has('popout')).toBe(true);
+
+		act(() => result.current.unregisterView('popout'));
+		expect(result.current.currentViews.has('popout')).toBe(false);
+	});
+
+	it('should hide the widget while the room view is registered', () => {
+		const { result } = renderHook(() => useAvailableViewTracker(), { wrapper: StrictMode });
+
+		act(() => result.current.registerView('widget'));
+		expect(result.current.currentViews.has('widget')).toBe(true);
+
+		act(() => result.current.registerView('room'));
+		expect(result.current.currentViews.has('widget')).toBe(false);
+
+		act(() => result.current.unregisterView('room'));
+		expect(result.current.currentViews.has('widget')).toBe(true);
+	});
+
+	it('should return a new set only when the registered views change', () => {
+		const { result } = renderHook(() => useAvailableViewTracker(), { wrapper: StrictMode });
+
+		act(() => result.current.registerView('popout'));
+		const views = result.current.currentViews;
+
+		act(() => result.current.registerView('popout'));
+		expect(result.current.currentViews).toBe(views);
+
+		act(() => result.current.registerView('room'));
+		expect(result.current.currentViews).not.toBe(views);
+	});
+});

packages/ui-voip/src/providers/useAvailableViewTracker.ts

@@ -13,37 +13,31 @@ const filter = (view: AvailableViews, _index: number, array: AvailableViews[]) =
 	}
 };
 
+const withFilteredViews = (currentViews: Set<AvailableViews>) => ({
+	currentViews,
+	filteredViews: new Set(Array.from(currentViews).filter(filter)),
+});
+
 const useAvailableViewTracker = () => {
-	// keep in mind views.currentViews is a stable set, so please if you are going to use it in a useEffect, make sure to create a new set from it, otherwise you will not be able to track changes in the set.
 	const [views, setViews] = useState<{
 		currentViews: Set<AvailableViews>;
 		filteredViews: Set<AvailableViews>;
-	}>({
-		currentViews: new Set<AvailableViews>(),
-		filteredViews: new Set<AvailableViews>(),
-	});
+	}>(() => withFilteredViews(new Set<AvailableViews>()));
 
 	const registerView = useCallback((view: AvailableViews) => {
-		setViews((prev) => {
-			if (prev.currentViews.has(view)) return prev;
-
-			prev.currentViews.add(view);
-			return {
-				currentViews: prev.currentViews,
-				filteredViews: new Set(Array.from(prev.currentViews).filter(filter)),
-			};
-		});
+		// the updater must not touch prev: StrictMode calls it twice, and a mutated set would make the second call bail out
+		setViews((prev) => (prev.currentViews.has(view) ? prev : withFilteredViews(new Set(prev.currentViews).add(view))));
 	}, []);
 
 	const unregisterView = useCallback((view: AvailableViews) => {
 		setViews((prev) => {
-			if (!prev.currentViews.has(view)) return prev;
+			if (!prev.currentViews.has(view)) {
+				return prev;
+			}
 
-			prev.currentViews.delete(view);
-			return {
-				currentViews: prev.currentViews,
-				filteredViews: new Set(Array.from(prev.currentViews).filter(filter)),
-			};
+			const currentViews = new Set(prev.currentViews);
+			currentViews.delete(view);
+			return withFilteredViews(currentViews);
 		});
 	}, []);