From 82cdd3d26af827ea01fced30ab2afba26a2ee6e7 Mon Sep 17 00:00:00 2001 From: Natalie Bridgers Date: Tue, 23 Jun 2026 20:41:10 -0500 Subject: [PATCH] refactor(player): stabilize callback identities, drop redundant HLSPlayer key surfaceError listed useWebRTC in its useCallback deps, so it got a new identity on every transport toggle. The video event-listener effect (play/pause/playing/loadedmetadata/error) listed surfaceError in its deps, so every toggle tore down and re-added all five listeners. Read useWebRTC and onError via refs instead; surfaceError becomes stable and the effect only runs on active / onPlaying changes. HLSPlayer's stats-polling interval had the same shape: onStatsChange is created fresh on every Player render (setStats), so the interval tore down and re-created each render. Same fix: onStatsChangeRef. Drop onStatsChange from the dep array. The sona key on HLSPlayer was forcing a full remount on src change, but the HLSPlayer effect already tears down and rebuilds hls.js on src change. The remount was redundant churn. Drop sona, the resetSona effect, and the key. --- js/web/src/components/player/hls-player.tsx | 9 ++++-- js/web/src/components/player/player.tsx | 34 ++++++++++----------- 2 files changed, 24 insertions(+), 19 deletions(-) diff --git a/js/web/src/components/player/hls-player.tsx b/js/web/src/components/player/hls-player.tsx index a770e0ea..2ed0f67b 100644 --- a/js/web/src/components/player/hls-player.tsx +++ b/js/web/src/components/player/hls-player.tsx @@ -54,6 +54,11 @@ export function HLSPlayer({ ref?: RefObject; }) { const hlsRef = useRef(null); + // Ref so the stats-polling interval (a child of the main effect) can + // call the latest onStatsChange without re-creating the interval on + // every parent render. + const onStatsChangeRef = useRef(onStatsChange); + onStatsChangeRef.current = onStatsChange; useImperativeHandle( ref ?? { current: null }, @@ -211,7 +216,7 @@ export function HLSPlayer({ ? Math.max(0, liveEdge - video.currentTime) : undefined; - onStatsChange?.({ + onStatsChangeRef.current?.({ width: currentLevelData?.width ?? video.videoWidth, height: currentLevelData?.height ?? video.videoHeight, viewportWidth: typeof window === "undefined" ? 0 : window.innerWidth, @@ -229,7 +234,7 @@ export function HLSPlayer({ }); }, 1000); return () => clearInterval(id); - }, [active, onStatsChange, videoRef]); + }, [active, videoRef]); return null; } diff --git a/js/web/src/components/player/player.tsx b/js/web/src/components/player/player.tsx index 9ecd9c36..657d330f 100644 --- a/js/web/src/components/player/player.tsx +++ b/js/web/src/components/player/player.tsx @@ -166,18 +166,23 @@ export function Player({ // Stable per-mount id for video playback const [sessionId] = useSonare(); - const surfaceError = useCallback( - (msg: string) => { - setError(msg); - onError?.(msg); - // WebRTC failed — fall back to HLS automatically. - if (useWebRTC) { - setUseWebRTC(false); - writeTransportPreference(false); - } - }, - [onError, useWebRTC], - ); + // Refs so the video event-listener effect below doesn't have to list + // these as deps — it would otherwise tear down and re-add its + // listeners on every transport toggle or parent re-render. + const useWebRTCRef = useRef(useWebRTC); + const onErrorRef = useRef(onError); + useWebRTCRef.current = useWebRTC; + onErrorRef.current = onError; + + const surfaceError = useCallback((msg: string) => { + setError(msg); + onErrorRef.current?.(msg); + // WebRTC failed — fall back to HLS automatically. + if (useWebRTCRef.current) { + setUseWebRTC(false); + writeTransportPreference(false); + } + }, []); const handleWebRTCChange = useCallback((value: boolean) => { setUseWebRTC(value); @@ -382,10 +387,6 @@ function PlayerBackend({ onStatsChange: (stats: PlayerStats) => void; ref: RefObject; }) { - const [sona, resetSona] = useSonare(); - useEffect(() => { - resetSona(); - }, [src]); if (useWebRTC || src.startsWith("webrtc://")) { return (