move state update out of effects

.pre-commit-config.yaml

@@ -24,7 +24,7 @@ repos:
       - id: flake8
         alias: python
         files: ^backend/
-        args: ["--max-complexity=10", "--max-line-length=79"]
+        args: ["--jobs=1", "--max-complexity=10", "--max-line-length=79"]
   - repo: https://github.com/codespell-project/codespell
     rev: v2.4.2
     hooks:

frontend/src/components/DownloadListItem.tsx

@@ -11,7 +11,7 @@ import VideoThumbnail from './VideoThumbail';
 
 type DownloadListItemProps = {
   download: Download;
-  setRefresh: (status: boolean) => void;
+  setRefresh: () => void;
 };
 
 const DownloadListItem = ({ download, setRefresh }: DownloadListItemProps) => {
@@ -76,7 +76,7 @@ const DownloadListItem = ({ download, setRefresh }: DownloadListItemProps) => {
                   label="Forget"
                   onClick={async () => {
                     await deleteDownloadById(download.youtube_id);
-                    setRefresh(true);
+                    setRefresh();
                   }}
                 />
               </div>
@@ -86,7 +86,7 @@ const DownloadListItem = ({ download, setRefresh }: DownloadListItemProps) => {
                   label="Add to queue"
                   onClick={async () => {
                     await updateDownloadQueueStatusById(download.youtube_id, 'pending');
-                    setRefresh(true);
+                    setRefresh();
                   }}
                 />
               </div>
@@ -100,7 +100,7 @@ const DownloadListItem = ({ download, setRefresh }: DownloadListItemProps) => {
                   onClick={async () => {
                     await updateDownloadQueueStatusById(download.youtube_id, 'ignore');
 
-                    setRefresh(true);
+                    setRefresh();
                   }}
                 />
               </div>
@@ -114,7 +114,7 @@ const DownloadListItem = ({ download, setRefresh }: DownloadListItemProps) => {
 
                       await updateDownloadQueueStatusById(download.youtube_id, 'priority');
 
-                      setRefresh(true);
+                      setRefresh();
                     }}
                   />
                 </div>
@@ -129,7 +129,7 @@ const DownloadListItem = ({ download, setRefresh }: DownloadListItemProps) => {
                 className="danger-button"
                 onClick={async () => {
                   await deleteDownloadById(download.youtube_id);
-                  setRefresh(true);
+                  setRefresh();
                 }}
               />
             </div>

frontend/src/components/MembershipAppsettings.tsx

@@ -37,13 +37,6 @@ export default function MembershipAppsettings({ show_help_text }: { show_help_te
   const [isLoadingSync, setIsLoadingSync] = useState(false);
   const [subSyncMessage, setSubSyncMessage] = useState('');
 
-  const fetchMembershipToken = async () => {
-    const apiTokenResponse = await APIClient<ApiTokenResponse>(
-      '/api/appsettings/membership/token/',
-    );
-    setMembershipApiToken(apiTokenResponse.data?.token || null);
-  };
-
   const deleteMembershipToken = async () => {
     await APIClient('/api/appsettings/membership/token/', { method: 'DELETE' });
     setMembershipApiToken(null);
@@ -64,6 +57,12 @@ export default function MembershipAppsettings({ show_help_text }: { show_help_te
   };
 
   useEffect(() => {
+    const fetchMembershipToken = async () => {
+      const apiTokenResponse = await APIClient<ApiTokenResponse>(
+        '/api/appsettings/membership/token/',
+      );
+      setMembershipApiToken(apiTokenResponse.data?.token || null);
+    };
     fetchMembershipToken();
   }, []);
 

frontend/src/components/VideoPlayer.tsx

@@ -167,20 +167,6 @@ const VideoPlayer = ({
   const [showInfoDialog, setShowInfoDialog] = useState(false);
   const [infoDialogContent, setInfoDialogContent] = useState('');
   const [isTheaterMode, setIsTheaterMode] = useState(false);
-  const [theaterModeKeyPressed, setTheaterModeKeyPressed] = useState(false);
-
-  const questionmarkPressed = useKeyPress('?');
-  const mutePressed = useKeyPress('m');
-  const fullscreenPressed = useKeyPress('f');
-  const subtitlesPressed = useKeyPress('c');
-  const increasePlaybackSpeedPressed = useKeyPress('>');
-  const decreasePlaybackSpeedPressed = useKeyPress('<');
-  const resetPlaybackSpeedPressed = useKeyPress('=');
-  const arrowRightPressed = useKeyPress('ArrowRight');
-  const arrowLeftPressed = useKeyPress('ArrowLeft');
-  const pPausedPressed = useKeyPress('p');
-  const theaterModePressed = useKeyPress('t');
-  const escapePressed = useKeyPress('Escape');
 
   const videoId = video.youtube_id;
   const videoUrl = video.media_url;
@@ -205,6 +191,152 @@ const VideoPlayer = ({
     }, 500);
   };
 
+  useKeyPress('m', () => {
+    setIsMuted(current => !current);
+  });
+
+  useKeyPress('p', () => {
+    if (videoRef.current?.paused) {
+      videoRef.current.play();
+    } else {
+      videoRef.current?.pause();
+    }
+  });
+
+  useKeyPress('>', () => {
+    const newSpeed = playbackSpeedIndex + 1;
+
+    if (videoRef.current && VIDEO_PLAYBACK_SPEEDS[newSpeed]) {
+      const speed = VIDEO_PLAYBACK_SPEEDS[newSpeed];
+      videoRef.current.playbackRate = speed;
+
+      setPlaybackSpeedIndex(newSpeed);
+      infoDialog(`${speed}x`);
+    }
+  });
+
+  useKeyPress('<', () => {
+    const newSpeedIndex = playbackSpeedIndex - 1;
+
+    if (videoRef.current && VIDEO_PLAYBACK_SPEEDS[newSpeedIndex]) {
+      const speed = VIDEO_PLAYBACK_SPEEDS[newSpeedIndex];
+      videoRef.current.playbackRate = speed;
+
+      setPlaybackSpeedIndex(newSpeedIndex);
+      infoDialog(`${speed}x`);
+    }
+  });
+
+  useKeyPress('=', () => {
+    const newSpeedIndex = 3;
+
+    if (videoRef.current && VIDEO_PLAYBACK_SPEEDS[newSpeedIndex]) {
+      const speed = VIDEO_PLAYBACK_SPEEDS[newSpeedIndex];
+      videoRef.current.playbackRate = speed;
+
+      setPlaybackSpeedIndex(newSpeedIndex);
+      infoDialog(`${speed}x`);
+    }
+  });
+
+  useKeyPress('f', () => {
+    if (videoRef.current && videoRef.current.requestFullscreen && !document.fullscreenElement) {
+      videoRef.current.requestFullscreen().catch(e => {
+        console.error(e);
+        infoDialog('Unable to enter fullscreen');
+      });
+    } else {
+      document.exitFullscreen().catch(e => {
+        console.error(e);
+        infoDialog('Unable to exit fullscreen');
+      });
+    }
+  });
+
+  useKeyPress('c', () => {
+    if (!videoRef.current) {
+      return;
+    }
+
+    const tracks = [...videoRef.current.textTracks];
+
+    if (tracks.length === 0) {
+      return;
+    }
+
+    const lastIndex = tracks.findIndex(x => x.mode === 'showing');
+    const active = tracks[lastIndex];
+
+    if (!active && lastSubtitleTack !== 0) {
+      tracks[lastSubtitleTack - 1].mode = 'showing';
+    } else if (active) {
+      active.mode = 'hidden';
+
+      setLastSubtitleTack(lastIndex + 1);
+    }
+  });
+
+  useKeyPress('ArrowLeft', () => {
+    const currentCurrentTime = videoRef.current?.currentTime;
+
+    if (currentCurrentTime !== undefined && videoRef.current) {
+      infoDialog('- 5 seconds');
+      videoRef.current.currentTime = currentCurrentTime - 5;
+    }
+  });
+
+  useKeyPress('ArrowRight', () => {
+    const currentCurrentTime = videoRef.current?.currentTime;
+
+    if (currentCurrentTime !== undefined && videoRef.current) {
+      infoDialog('+ 5 seconds');
+      videoRef.current.currentTime = currentCurrentTime + 5;
+    }
+  });
+
+  useKeyPress('?', () => {
+    setShowHelpDialog(current => {
+      const next = !current;
+
+      if (next) {
+        setTimeout(() => {
+          setShowHelpDialog(false);
+        }, 3000);
+      }
+
+      return next;
+    });
+  });
+
+  useKeyPress('t', () => {
+    if (embed) {
+      return;
+    }
+
+    setIsTheaterMode(current => {
+      const next = !current;
+      infoDialog(next ? 'Theater mode' : 'Normal mode');
+
+      return next;
+    });
+  });
+
+  useKeyPress('Escape', () => {
+    if (embed) {
+      return;
+    }
+
+    setIsTheaterMode(current => {
+      if (!current) {
+        return current;
+      }
+
+      infoDialog('Normal mode');
+
+      return false;
+    });
+  });
+
   const handleVideoEnd =
     (
       youtubeId: string,
@@ -238,174 +370,6 @@ const VideoPlayer = ({
       onVideoEnd?.();
     };
 
-  useEffect(() => {
-    if (mutePressed) {
-      setIsMuted(!isMuted);
-    }
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [mutePressed]);
-
-  useEffect(() => {
-    if (pPausedPressed) {
-      if (videoRef.current?.paused) {
-        videoRef.current.play();
-      } else {
-        videoRef.current?.pause();
-      }
-    }
-  }, [pPausedPressed]);
-
-  useEffect(() => {
-    if (increasePlaybackSpeedPressed) {
-      const newSpeed = playbackSpeedIndex + 1;
-
-      if (videoRef.current && VIDEO_PLAYBACK_SPEEDS[newSpeed]) {
-        const speed = VIDEO_PLAYBACK_SPEEDS[newSpeed];
-        videoRef.current.playbackRate = speed;
-
-        setPlaybackSpeedIndex(newSpeed);
-        infoDialog(`${speed}x`);
-      }
-    }
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [increasePlaybackSpeedPressed]);
-
-  useEffect(() => {
-    if (decreasePlaybackSpeedPressed) {
-      const newSpeedIndex = playbackSpeedIndex - 1;
-
-      if (videoRef.current && VIDEO_PLAYBACK_SPEEDS[newSpeedIndex]) {
-        const speed = VIDEO_PLAYBACK_SPEEDS[newSpeedIndex];
-        videoRef.current.playbackRate = speed;
-
-        setPlaybackSpeedIndex(newSpeedIndex);
-        infoDialog(`${speed}x`);
-      }
-    }
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [decreasePlaybackSpeedPressed]);
-
-  useEffect(() => {
-    if (resetPlaybackSpeedPressed) {
-      const newSpeedIndex = 3;
-
-      if (videoRef.current && VIDEO_PLAYBACK_SPEEDS[newSpeedIndex]) {
-        const speed = VIDEO_PLAYBACK_SPEEDS[newSpeedIndex];
-        videoRef.current.playbackRate = speed;
-
-        setPlaybackSpeedIndex(newSpeedIndex);
-        infoDialog(`${speed}x`);
-      }
-    }
-  }, [resetPlaybackSpeedPressed]);
-
-  useEffect(() => {
-    if (fullscreenPressed) {
-      if (videoRef.current && videoRef.current.requestFullscreen && !document.fullscreenElement) {
-        videoRef.current.requestFullscreen().catch(e => {
-          console.error(e);
-          infoDialog('Unable to enter fullscreen');
-        });
-      } else {
-        document.exitFullscreen().catch(e => {
-          console.error(e);
-          infoDialog('Unable to exit fullscreen');
-        });
-      }
-    }
-  }, [fullscreenPressed]);
-
-  useEffect(() => {
-    if (subtitlesPressed) {
-      if (videoRef.current) {
-        const tracks = [...videoRef.current.textTracks];
-
-        if (tracks.length === 0) {
-          return;
-        }
-
-        const lastIndex = tracks.findIndex(x => x.mode === 'showing');
-        const active = tracks[lastIndex];
-
-        if (!active && lastSubtitleTack !== 0) {
-          tracks[lastSubtitleTack - 1].mode = 'showing';
-        } else {
-          if (active) {
-            active.mode = 'hidden';
-
-            setLastSubtitleTack(lastIndex + 1);
-          }
-        }
-      }
-    }
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [subtitlesPressed]);
-
-  useEffect(() => {
-    if (arrowLeftPressed || arrowRightPressed) {
-      let timeStep = 5;
-
-      if (arrowLeftPressed) {
-        infoDialog('- 5 seconds');
-        timeStep *= -1;
-      }
-
-      if (arrowRightPressed) {
-        infoDialog('+ 5 seconds');
-      }
-
-      const currentCurrentTime = videoRef.current?.currentTime;
-
-      if (currentCurrentTime !== undefined && videoRef.current) {
-        videoRef.current.currentTime = currentCurrentTime + timeStep;
-      }
-    }
-  }, [arrowLeftPressed, arrowRightPressed]);
-
-  useEffect(() => {
-    if (questionmarkPressed) {
-      if (!showHelpDialog) {
-        setTimeout(() => {
-          setShowHelpDialog(false);
-        }, 3000);
-      }
-
-      setShowHelpDialog(!showHelpDialog);
-    }
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [questionmarkPressed]);
-
-  useEffect(() => {
-    if (embed) {
-      return;
-    }
-
-    if (theaterModePressed && !theaterModeKeyPressed) {
-      setTheaterModeKeyPressed(true);
-
-      const newTheaterMode = !isTheaterMode;
-      setIsTheaterMode(newTheaterMode);
-
-      infoDialog(newTheaterMode ? 'Theater mode' : 'Normal mode');
-    } else if (!theaterModePressed) {
-      setTheaterModeKeyPressed(false);
-    }
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [theaterModePressed, isTheaterMode, theaterModeKeyPressed]);
-
-  useEffect(() => {
-    if (embed) {
-      return;
-    }
-
-    if (escapePressed && isTheaterMode) {
-      setIsTheaterMode(false);
-
-      infoDialog('Normal mode');
-    }
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [escapePressed, isTheaterMode]);
-
   return (
     <>
       <div

frontend/src/functions/useKeypressHook.tsx

@@ -1,8 +1,15 @@
-import { useEffect, useState } from 'react';
+import { useEffect, useEffectEvent, useRef, useState } from 'react';
 
 // source: https://thibault.sh/react-hooks/use-key-press
-export function useKeyPress(targetKey: string) {
+export function useKeyPress(targetKey: string, onKeyDown?: () => void, onKeyUp?: () => void) {
   const [isKeyPressed, setIsKeyPressed] = useState(false);
+  const isKeyPressedRef = useRef(false);
+  const handleKeyDownEvent = useEffectEvent(() => {
+    onKeyDown?.();
+  });
+  const handleKeyUpEvent = useEffectEvent(() => {
+    onKeyUp?.();
+  });
 
   useEffect(() => {
     const handleKeyDown = (event: KeyboardEvent) => {
@@ -13,13 +20,21 @@ export function useKeyPress(targetKey: string) {
         !event.altKey &&
         !event.altKey
       ) {
+        if (isKeyPressedRef.current) {
+          return;
+        }
+
+        isKeyPressedRef.current = true;
         setIsKeyPressed(true);
+        handleKeyDownEvent();
       }
     };
 
     const handleKeyUp = (event: KeyboardEvent) => {
       if (event.key === targetKey) {
+        isKeyPressedRef.current = false;
         setIsKeyPressed(false);
+        handleKeyUpEvent();
       }
     };
 

frontend/src/pages/Base.tsx

@@ -1,8 +1,8 @@
-import { Outlet, useLoaderData, useLocation, useSearchParams } from 'react-router-dom';
+import { Outlet, useLoaderData, useSearchParams } from 'react-router-dom';
 import Footer from '../components/Footer';
 import Colours from '../configuration/colours/Colours';
 import { UserConfigType } from '../api/actions/updateUserConfig';
-import { useEffect, useState } from 'react';
+import { useCallback, useEffect } from 'react';
 import Navigation from '../components/Navigation';
 import { useAuthStore } from '../stores/AuthDataStore';
 import { useUserConfigStore } from '../stores/UserConfigStore';
@@ -43,12 +43,8 @@ const Base = () => {
   const { setAppSettingsConfig } = useAppSettingsStore();
 
   const { userConfig, userAccount, appSettings, auth } = useLoaderData() as BaseLoaderData;
-
-  const location = useLocation();
-
   const currentPageFromUrl = Number(searchParams.get('page'));
-
+  const currentPage = Number.isNaN(currentPageFromUrl) ? 0 : currentPageFromUrl;
-  const [currentPage, setCurrentPage] = useState(currentPageFromUrl);
 
   useEffect(() => {
     setAuth(auth);
@@ -59,40 +55,20 @@ const Base = () => {
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, []);
 
-  useEffect(() => {
+  const setCurrentPage = useCallback(
-    if (currentPageFromUrl !== currentPage) {
+    (page: number) => {
-      setCurrentPage(0);
-    }
-
-    // This should only be executed when location.pathname changes.
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [location.pathname]);
-
-  useEffect(() => {
-    if (currentPageFromUrl !== currentPage) {
-      setCurrentPage(currentPageFromUrl);
-    }
-
-    // This should only be executed when currentPageFromUrl changes.
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [currentPageFromUrl]);
-
-  useEffect(() => {
-    if (currentPageFromUrl !== currentPage) {
       setSearchParams(params => {
-        if (currentPage == 0) {
+        if (page === 0) {
           params.delete('page');
         } else {
-          params.set('page', currentPage.toString());
+          params.set('page', page.toString());
         }
 
         return params;
       });
-    }
+    },
-
+    [setSearchParams],
-    // This should only be executed when currentPage changes.
+  );
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [currentPage]);
 
   return (
     <>

frontend/src/pages/Download.tsx

@@ -60,7 +60,7 @@ const Download = () => {
   const vidTypeFilterFromUrl = searchParams.get('vid-type');
   const errorFilterFromUrl = searchParams.get('error');
 
-  const [refresh, setRefresh] = useState(false);
+  const [refreshNonce, setRefreshNonce] = useState(0);
   const [showHiddenForm, setShowHiddenForm] = useState(false);
   const [addAsAutoStart, setAddAsAutoStart] = useState(false);
   const [addAsFlat, setAddAsFlat] = useState(false);
@@ -113,34 +113,31 @@ const Download = () => {
     }
   };
 
+  const refreshDownloadQueue = () => {
+    setRefreshNonce(current => current + 1);
+  };
+
   useEffect(() => {
     (async () => {
-      if (refresh) {
+      const videosResponse = await loadDownloadQueue(
-        const videosResponse = await loadDownloadQueue(
+        currentPage,
-          currentPage,
+        channelFilterFromUrl,
-          channelFilterFromUrl,
+        vidTypeFilterFromUrl,
-          vidTypeFilterFromUrl,
+        errorFilterFromUrl,
-          errorFilterFromUrl,
+        showIgnored,
-          showIgnored,
+        searchInput,
-          searchInput,
+      );
-        );
+      const { data: channelResponseData } = videosResponse ?? {};
-        const { data: channelResponseData } = videosResponse ?? {};
+      const videoCount = channelResponseData?.paginate?.total_hits;
-        const videoCount = channelResponseData?.paginate?.total_hits;
 
-        if (videoCount && lastVideoCount !== videoCount) {
+      if (videoCount && lastVideoCount !== videoCount) {
-          setLastVideoCount(videoCount);
+        setLastVideoCount(videoCount);
-        }
-
-        setDownloadResponse(videosResponse);
-        setRefresh(false);
       }
+
+      setDownloadResponse(videosResponse);
     })();
 
     // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [refresh]);
-
-  useEffect(() => {
-    setRefresh(true);
   }, [
     channelFilterFromUrl,
     vidTypeFilterFromUrl,
@@ -148,6 +145,7 @@ const Download = () => {
     currentPage,
     showIgnored,
     searchInput,
+    refreshNonce,
   ]);
 
   useEffect(() => {
@@ -166,7 +164,7 @@ const Download = () => {
       errorFilterFromUrl,
       status,
     );
-    setRefresh(true);
+    refreshDownloadQueue();
   };
 
   return (
@@ -184,7 +182,7 @@ const Download = () => {
             if (!isDone) {
               setRescanPending(false);
               setDownloadPending(false);
-              setRefresh(true);
+              refreshDownloadQueue();
             }
           }}
         />
@@ -308,7 +306,7 @@ const Download = () => {
                           force: addAsForce,
                         });
                         setDownloadQueueText('');
-                        setRefresh(true);
+                        refreshDownloadQueue();
                         setShowHiddenForm(false);
                       }
                     }}
@@ -329,7 +327,7 @@ const Download = () => {
                   const newParams = new URLSearchParams();
                   newParams.set('ignored', String(!showIgnored));
                   setSearchParams(newParams);
-                  setRefresh(true);
+                  refreshDownloadQueue();
                 }}
                 type="checkbox"
                 checked={showIgnored}
@@ -539,7 +537,7 @@ const Download = () => {
                         channelFilterFromUrl,
                         vidTypeFilterFromUrl,
                       );
-                      setRefresh(true);
+                      refreshDownloadQueue();
                       setShowDeleteConfirm(false);
                     }}
                   >
@@ -563,7 +561,7 @@ const Download = () => {
                 <Fragment
                   key={`${download.channel_id}_${download.timestamp}_${download.youtube_id}`}
                 >
-                  <DownloadListItem download={download} setRefresh={setRefresh} />
+                  <DownloadListItem download={download} setRefresh={refreshDownloadQueue} />
                 </Fragment>
               );
             })}

frontend/src/pages/Video.tsx

@@ -108,7 +108,6 @@ const Video = () => {
   const { appSettingsConfig } = useAppSettingsStore();
   const { userConfig } = useUserConfigStore();
 
-  const [videoEnded, setVideoEnded] = useState(false);
   const [seekToTimestamp, setSeekToTimestamp] = useState<number>();
   const [playlistAutoplay, setPlaylistAutoplay] = useState(
     localStorage.getItem('playlistAutoplay') === 'true',
@@ -179,24 +178,6 @@ const Video = () => {
     localStorage.setItem('playlistIdForAutoplay', playlistIdForAutoplay || '');
   }, [playlistAutoplay, playlistIdForAutoplay]);
 
-  useEffect(() => {
-    if (videoEnded && playlistAutoplay) {
-      const playlist = videoPlaylistNavResponseData?.find(playlist => {
-        return playlist.playlist_meta.playlist_id === playlistIdForAutoplay;
-      });
-
-      if (playlist) {
-        const nextYoutubeId = playlist.playlist_next?.youtube_id;
-
-        if (nextYoutubeId) {
-          setVideoEnded(false);
-          navigate(Routes.Video(nextYoutubeId));
-        }
-      }
-    }
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [videoEnded, playlistAutoplay]);
-
   const errorMessage = videoResponseError?.error;
 
   if (errorMessage) {
@@ -235,7 +216,18 @@ const Video = () => {
           setRefreshVideoList(true);
         }}
         onVideoEnd={() => {
-          setVideoEnded(true);
+          if (!playlistAutoplay) {
+            return;
+          }
+
+          const playlist = videoPlaylistNavResponseData?.find(playlist => {
+            return playlist.playlist_meta.playlist_id === playlistIdForAutoplay;
+          });
+          const nextYoutubeId = playlist?.playlist_next?.youtube_id;
+
+          if (nextYoutubeId) {
+            navigate(Routes.Video(nextYoutubeId));
+          }
         }}
       />