From 78c26bc3f2d23a4d17b950e14f7859f0e424a603 Mon Sep 17 00:00:00 2001 From: Etan Joseph Heyman Date: Sat, 28 Feb 2026 04:10:16 +0200 Subject: [PATCH] nightshift: fix stale onEnded closure in useAudioPreloader The onEnded callback was captured in the useEffect closure but not included in the dependency array. If the caller updated onEnded between renders, the stale version from the initial render would fire instead. Use a ref pattern so the ended handler always calls the latest callback without needing to re-run the effect (which would reload all audio). Co-Authored-By: Claude Opus 4.6 --- src/hooks/useAudioPreloader.ts | 212 ++++++++++++++++++--------------- 1 file changed, 113 insertions(+), 99 deletions(-) diff --git a/src/hooks/useAudioPreloader.ts b/src/hooks/useAudioPreloader.ts index ff40d4d..ce8a95a 100644 --- a/src/hooks/useAudioPreloader.ts +++ b/src/hooks/useAudioPreloader.ts @@ -1,175 +1,189 @@ -import { useEffect, useState, useRef, useCallback } from 'react' +import { useEffect, useState, useRef, useCallback } from "react"; interface AudioSnippet { - lineNumber: number - audioUrl: string + lineNumber: number; + audioUrl: string; } interface PreloadState { - loaded: number - total: number - ready: boolean + loaded: number; + total: number; + ready: boolean; } interface UseAudioPreloaderReturn extends PreloadState { - play: (lineNumber: number) => void - stop: () => void - pause: () => void - resume: () => void - isPlaying: boolean - setPlaybackRate: (rate: number) => void - setLoop: (loop: boolean) => void + play: (lineNumber: number) => void; + stop: () => void; + pause: () => void; + resume: () => void; + isPlaying: boolean; + setPlaybackRate: (rate: number) => void; + setLoop: (loop: boolean) => void; } export function useAudioPreloader( snippets: AudioSnippet[], - onEnded?: (lineNumber: number) => void + onEnded?: (lineNumber: number) => void, ): UseAudioPreloaderReturn { const [state, setState] = useState({ loaded: 0, total: snippets.length, ready: false, - }) - const [isPlaying, setIsPlaying] = useState(false) - const audioMapRef = useRef>(new Map()) - const currentlyPlayingRef = useRef(null) - const playbackRateRef = useRef(1) - const loopRef = useRef(false) + }); + const [isPlaying, setIsPlaying] = useState(false); + const audioMapRef = useRef>(new Map()); + const currentlyPlayingRef = useRef(null); + const playbackRateRef = useRef(1); + const loopRef = useRef(false); + + // Keep onEnded in a ref so the audio 'ended' handler always calls the latest callback + // without needing to re-run the effect (which would reload all audio elements) + const onEndedRef = useRef(onEnded); + onEndedRef.current = onEnded; useEffect(() => { if (snippets.length === 0) { - setState({ loaded: 0, total: 0, ready: true }) - return + setState({ loaded: 0, total: 0, ready: true }); + return; } - let loadedCount = 0 - const total = snippets.length + let loadedCount = 0; + const total = snippets.length; // Reset state for new snippets - setState({ loaded: 0, total, ready: false }) - audioMapRef.current.clear() + setState({ loaded: 0, total, ready: false }); + audioMapRef.current.clear(); snippets.forEach(({ lineNumber, audioUrl }) => { - if (!audioUrl) return + if (!audioUrl) return; - const audio = new Audio() - audio.preload = 'auto' - let handled = false // Prevent double counting from canplay + canplaythrough + const audio = new Audio(); + audio.preload = "auto"; + let handled = false; // Prevent double counting from canplay + canplaythrough const handleCanPlay = () => { - if (handled) return - handled = true - loadedCount++ - audioMapRef.current.set(lineNumber, audio) + if (handled) return; + handled = true; + loadedCount++; + audioMapRef.current.set(lineNumber, audio); setState({ loaded: loadedCount, total, ready: loadedCount === total, - }) - } + }); + }; const handleError = () => { - if (handled) return - handled = true + if (handled) return; + handled = true; // Still increment loaded count to not block ready state - loadedCount++ - console.warn(`Failed to load audio for line ${lineNumber}: ${audioUrl}`) + loadedCount++; + console.warn( + `Failed to load audio for line ${lineNumber}: ${audioUrl}`, + ); setState({ loaded: loadedCount, total, ready: loadedCount === total, - }) - } + }); + }; const handleEnded = () => { - setIsPlaying(false) - if (onEnded) { - onEnded(lineNumber) - } - } - - audio.addEventListener('canplaythrough', handleCanPlay, { once: true }) - audio.addEventListener('canplay', handleCanPlay, { once: true }) // Fallback for cached audio - audio.addEventListener('error', handleError, { once: true }) - audio.addEventListener('ended', handleEnded) - audio.src = audioUrl - audio.load() // Explicitly trigger loading - }) + setIsPlaying(false); + onEndedRef.current?.(lineNumber); + }; + + audio.addEventListener("canplaythrough", handleCanPlay, { once: true }); + audio.addEventListener("canplay", handleCanPlay, { once: true }); // Fallback for cached audio + audio.addEventListener("error", handleError, { once: true }); + audio.addEventListener("ended", handleEnded); + audio.src = audioUrl; + audio.load(); // Explicitly trigger loading + }); return () => { // Cleanup all audio elements on unmount - audioMapRef.current.forEach(audio => { - audio.pause() - audio.removeEventListener('ended', () => {}) - audio.src = '' - }) - audioMapRef.current.clear() - currentlyPlayingRef.current = null - } - }, [snippets]) + audioMapRef.current.forEach((audio) => { + audio.pause(); + audio.removeEventListener("ended", () => {}); + audio.src = ""; + }); + audioMapRef.current.clear(); + currentlyPlayingRef.current = null; + }; + }, [snippets]); const play = useCallback((lineNumber: number) => { // Stop any currently playing audio if (currentlyPlayingRef.current) { - currentlyPlayingRef.current.pause() - currentlyPlayingRef.current.currentTime = 0 + currentlyPlayingRef.current.pause(); + currentlyPlayingRef.current.currentTime = 0; } - const audio = audioMapRef.current.get(lineNumber) + const audio = audioMapRef.current.get(lineNumber); if (audio) { - audio.currentTime = 0 - audio.playbackRate = playbackRateRef.current - audio.loop = loopRef.current + audio.currentTime = 0; + audio.playbackRate = playbackRateRef.current; + audio.loop = loopRef.current; audio.play().catch((err) => { - console.warn(`Failed to play audio for line ${lineNumber}:`, err) - setIsPlaying(false) - }) - currentlyPlayingRef.current = audio - setIsPlaying(true) + console.warn(`Failed to play audio for line ${lineNumber}:`, err); + setIsPlaying(false); + }); + currentlyPlayingRef.current = audio; + setIsPlaying(true); } - }, []) + }, []); const stop = useCallback(() => { - audioMapRef.current.forEach(audio => { - audio.pause() - audio.currentTime = 0 - }) - currentlyPlayingRef.current = null - setIsPlaying(false) - }, []) + audioMapRef.current.forEach((audio) => { + audio.pause(); + audio.currentTime = 0; + }); + currentlyPlayingRef.current = null; + setIsPlaying(false); + }, []); const pause = useCallback(() => { if (currentlyPlayingRef.current) { - currentlyPlayingRef.current.pause() - setIsPlaying(false) + currentlyPlayingRef.current.pause(); + setIsPlaying(false); } - }, []) + }, []); const resume = useCallback(() => { if (currentlyPlayingRef.current) { currentlyPlayingRef.current.play().catch((err) => { - console.warn('Failed to resume audio:', err) - setIsPlaying(false) - }) - setIsPlaying(true) + console.warn("Failed to resume audio:", err); + setIsPlaying(false); + }); + setIsPlaying(true); } - }, []) + }, []); const setPlaybackRate = useCallback((rate: number) => { - playbackRateRef.current = rate + playbackRateRef.current = rate; // Update rate on currently playing audio immediately if (currentlyPlayingRef.current) { - currentlyPlayingRef.current.playbackRate = rate + currentlyPlayingRef.current.playbackRate = rate; } - }, []) + }, []); const setLoop = useCallback((loop: boolean) => { - loopRef.current = loop + loopRef.current = loop; // Update loop on currently playing audio immediately if (currentlyPlayingRef.current) { - currentlyPlayingRef.current.loop = loop + currentlyPlayingRef.current.loop = loop; } - }, []) - - return { ...state, play, stop, pause, resume, isPlaying, setPlaybackRate, setLoop } + }, []); + + return { + ...state, + play, + stop, + pause, + resume, + isPlaying, + setPlaybackRate, + setLoop, + }; }