Merge pull request #12 from SevenOutman/fix/unplayable-loop

Avoid infinite auto-skipping when all songs are unplayable

Author: SevenOutman

.codesandbox/ci.json

@@ -0,0 +1,4 @@
+{
+  "node": "16",
+  "sandboxes": ["aplayer-react-demo-7kxnb0"]
+}

.github/workflows/ci.yml

@@ -26,6 +26,12 @@ jobs:
         with:
           node-version: ${{ matrix.node-version }}
 
+      - uses: pnpm/action-setup@v2
+        name: Install pnpm
+        id: pnpm-install
+        with:
+          version: 7
+
       - name: Cache ~/.pnpm-store
         uses: actions/cache@v2
         env:
@@ -38,9 +44,6 @@ jobs:
             ${{ runner.os }}-${{ matrix.node-version }}-test-
             ${{ runner.os }}-
 
-      - name: Install pnpm
-        run: npm i -g pnpm
-
       - name: Install deps
         run: pnpm i
 
@@ -62,6 +65,13 @@ jobs:
       - uses: actions/setup-node@v3
         with:
           node-version: 18.x
+
+      - uses: pnpm/action-setup@v2
+        name: Install pnpm
+        id: pnpm-install
+        with:
+          version: 7
+
       - name: Cache ~/.pnpm-store
         uses: actions/cache@v2
         env:
@@ -73,7 +83,6 @@ jobs:
             ${{ runner.os }}-${{ matrix.node-version }}-release-${{ env.cache-name }}-
             ${{ runner.os }}-${{ matrix.node-version }}-release-
             ${{ runner.os }}-
-      - run: npm i -g pnpm
       - run: pnpm i
       - run: pnpm dlx semantic-release
         env:

src/components/player.tsx

@@ -1,4 +1,4 @@
-import { useEffect, useRef, useState } from "react";
+import { useCallback, useEffect, useMemo, useRef, useState } from "react";
 import cx from "clsx";
 
 import { ReactComponent as IconPlay } from "../assets/play.svg";
@@ -11,6 +11,8 @@ import { defaultThemeColor } from "../constants";
 import { PlaylistLoop, PlaylistOrder, usePlaylist } from "../hooks/usePlaylist";
 import { Lyrics } from "./lyrics";
 import { useThemeColor } from "../hooks/useThemeColor";
+import { useNotice } from "../hooks/useNotice";
+import { useSetTimeout } from "../hooks/useSetTimeout";
 
 /**
  * @see https://aplayer.js.org/#/home?id=options
@@ -54,13 +56,36 @@ export function APlayer({
     getSongId: (song) => song.url,
   });
 
+  const [notice, showNotice] = useNotice();
+
+  const autoSkipTimeoutRef = useRef<
+    ReturnType<typeof setTimeout> | undefined
+  >();
+  const setTimeout = useSetTimeout();
+
+  const cancelAutoSkip = useCallback(() => {
+    if (autoSkipTimeoutRef.current) {
+      clearTimeout(autoSkipTimeoutRef.current);
+      autoSkipTimeoutRef.current = undefined;
+    }
+  }, []);
+
   const audioControl = useAudioControl({
     src: playlist.currentSong.url,
     initialVolume: volume,
     autoPlay,
-    onError() {
+    onError(e) {
+      const { error } = e.target as HTMLAudioElement;
+
+      if (error) {
+        showNotice(
+          "An audio error has occurred, player will skip forward in 2 seconds."
+        );
+      }
       if (playlist.hasNextSong) {
-        playlist.next();
+        autoSkipTimeoutRef.current = setTimeout(() => {
+          playlist.next();
+        }, 2000);
       }
     },
     onEnded() {
@@ -88,11 +113,29 @@ export function APlayer({
     }
   }, [playlist.currentSong, audioControl.playAudio]);
 
+  const handlePlayButtonClick = useCallback(() => {
+    cancelAutoSkip();
+    audioControl.togglePlay(playlist.currentSong.url);
+  }, [audioControl, cancelAutoSkip, playlist.currentSong.url]);
+
   const hasPlaylist = playlist.length > 1;
 
   const [isPlaylistOpen, setPlaylistOpen] = useState(() => hasPlaylist);
 
   const themeColor = useThemeColor(playlist.currentSong, theme);
+  const playlistAudioProp = useMemo(
+    () => (Array.isArray(audio) ? audio : [audio]),
+    [audio]
+  );
+
+  const { prioritize } = playlist;
+  const handlePlayAudioFromList = useCallback(
+    (audioInfo: AudioInfo) => {
+      cancelAutoSkip();
+      prioritize(audioInfo);
+    },
+    [cancelAutoSkip, prioritize]
+  );
 
   return (
     <div
@@ -114,7 +157,7 @@ export function APlayer({
               "aplayer-button",
               audioControl.isPlaying ? "aplayer-pause" : "aplayer-play"
             )}
-            onClick={() => audioControl.togglePlay(playlist.currentSong.url)}
+            onClick={handlePlayButtonClick}
           >
             {audioControl.isPlaying ? <IconPause /> : <IconPlay />}
           </div>
@@ -150,16 +193,18 @@ export function APlayer({
             onLoopChange={playlist.setLoop}
           />
         </div>
-        <div className="aplayer-notice"></div>
+        <div className="aplayer-notice" style={notice.style}>
+          {notice.text}
+        </div>
         <div className="aplayer-miniswitcher"></div>
       </div>
       {hasPlaylist ? (
         <Playlist
           themeColor={themeColor}
           open={isPlaylistOpen}
-          audio={Array.isArray(audio) ? audio : [audio]}
+          audio={playlistAudioProp}
           playingAudioUrl={playlist.currentSong.url}
-          onPlayAudio={(audioInfo) => playlist.prioritize(audioInfo)}
+          onPlayAudio={handlePlayAudioFromList}
         />
       ) : null}
     </div>

src/hooks/useNotice.test.ts

@@ -0,0 +1,40 @@
+import { expect, test, vi } from "vitest";
+import { renderHook, act } from "@testing-library/react";
+import { useNotice } from "./useNotice";
+
+test("Return empty text and opacity: 0 initially", () => {
+  const { result } = renderHook(() => useNotice());
+
+  expect(result.current[0]).toEqual({ text: "", style: { opacity: 0 } });
+});
+
+test("showNotice updates state to according text and opacity: 1", () => {
+  const { result } = renderHook(() => useNotice());
+
+  act(() => {
+    result.current[1]("A notice");
+  });
+
+  expect(result.current[0]).toEqual({
+    text: "A notice",
+    style: { opacity: 1 },
+  });
+});
+
+test("After 2000ms, opacity becomes 0 and text remains", () => {
+  vi.useFakeTimers();
+  const { result } = renderHook(() => useNotice());
+
+  act(() => {
+    result.current[1]("A notice");
+  });
+
+  act(() => {
+    vi.advanceTimersByTime(2000);
+  });
+
+  expect(result.current[0]).toEqual({
+    text: "A notice",
+    style: { opacity: 0 },
+  });
+});

src/hooks/useNotice.ts

@@ -0,0 +1,25 @@
+import { useCallback, useRef, useState } from "react";
+import { useSetTimeout } from "./useSetTimeout";
+
+export function useNotice() {
+  const timerRef = useRef<ReturnType<typeof setTimeout> | undefined>();
+  const [notice, setNotice] = useState({ text: "", style: { opacity: 0 } });
+
+  const setTimeout = useSetTimeout();
+
+  const showNotice = useCallback(
+    (text: string, duration = 2000) => {
+      if (timerRef.current) {
+        clearTimeout(timerRef.current);
+      }
+
+      setNotice({ text, style: { opacity: 1 } });
+      timerRef.current = setTimeout(() => {
+        setNotice({ text, style: { opacity: 0 } });
+      }, duration);
+    },
+    [setTimeout]
+  );
+
+  return [notice, showNotice] as const;
+}

src/hooks/useSetTimeout.test.ts

@@ -0,0 +1,37 @@
+import { vi, test, expect } from "vitest";
+import { renderHook, act } from "@testing-library/react";
+import { useSetTimeout } from "./useSetTimeout";
+
+test("Returns a function that schedule a callback after specific delay", () => {
+  vi.useFakeTimers();
+
+  const { result } = renderHook(() => useSetTimeout());
+  const callback = vi.fn();
+  act(() => {
+    result.current(callback, 1000);
+  });
+
+  act(() => {
+    vi.advanceTimersByTime(1000);
+  });
+
+  expect(callback).toHaveBeenCalledOnce();
+});
+
+test("Cancels the schedule when component unmounts", () => {
+  vi.useFakeTimers();
+
+  const { result, unmount } = renderHook(() => useSetTimeout());
+  const callback = vi.fn();
+  act(() => {
+    result.current(callback, 1000);
+  });
+
+  unmount();
+
+  act(() => {
+    vi.advanceTimersByTime(1000);
+  });
+
+  expect(callback).not.toHaveBeenCalled();
+});

src/hooks/useSetTimeout.ts

@@ -0,0 +1,24 @@
+import { useCallback, useEffect, useRef } from "react";
+
+export function useSetTimeout() {
+  const timeoutsRef = useRef<ReturnType<typeof setTimeout>[]>([]);
+
+  useEffect(() => {
+    const timeouts = timeoutsRef.current;
+    return () => {
+      for (const timeout of timeouts) {
+        clearTimeout(timeout);
+      }
+    };
+  }, []);
+
+  const safeSetTimeout = useCallback((callback: () => void, ms: number) => {
+    const timeout = setTimeout(callback, ms);
+
+    timeoutsRef.current.push(timeout);
+
+    return timeout;
+  }, []);
+
+  return safeSetTimeout;
+}