refactor: store currently playing song info in global state

gmus-web/src/actions/actions.ts

@@ -1,3 +1,4 @@
+import { Song } from '../types';
 import { Member, MusicPlayer } from '../types/state';
 import { ActionErrorOccurred } from './error';
 import { ActionLocal, ActionRemote, ActionTypeLocal, ActionTypeRemote } from './types';
@@ -51,6 +52,13 @@ export const playPaused = (): ActionPlayPaused => ({
   payload: undefined,
 });
 
+export type ActionSongInfoFetched = ActionLocal<ActionTypeLocal.SongInfoFetched, Song | null>;
+
+export const songInfoFetched = (song: Song | null): ActionSongInfoFetched => ({
+  type: ActionTypeLocal.SongInfoFetched,
+  payload: song,
+});
+
 export type LocalAction =
   | LoggedOut
   | ActionErrorOccurred
@@ -58,6 +66,7 @@ export type LocalAction =
   | ActionStateSetLocal
   | ActionSeeked
   | ActionPlayPaused
-  | ActionMasterSet;
+  | ActionMasterSet
+  | ActionSongInfoFetched;
 
 export type AnyAction = LocalAction | RemoteAction;

gmus-web/src/actions/types.ts

@@ -13,6 +13,7 @@ export enum ActionTypeLocal {
   Seeked = '@@local/SEEKED',
   MasterSet = '@@local/MASTER_SET',
   PlayPaused = '@@local/PLAY_PAUSED',
+  SongInfoFetched = '@@local/SONG_INFO_FETCHED',
 }
 
 export interface Action<T extends string = string, P = unknown> {

gmus-web/src/components/app.tsx

@@ -24,12 +24,11 @@ const UI = uiProviders[uiProvider];
 export const App: React.FC<Props> = ({ socket }) => {
   useKeepalive(socket);
   useMaster();
+  useCurrentlyPlayingSongInfo();
 
   const state = useContext(StateContext);
   const dispatch = useContext(DispatchContext);
 
-  const currentSong = useCurrentlyPlayingSongInfo(state.player.songId);
-
   const onTimeUpdate = useCallback(
     (currentTime: number): void => {
       dispatch(stateSet({ currentTime }));
@@ -74,7 +73,7 @@ export const App: React.FC<Props> = ({ socket }) => {
         <Suspense fallback={<LoadingWrapper />}>
           <UI
             isMaster={isMaster(state)}
-            currentSong={currentSong}
+            currentSong={state.songInfo}
             nextSong={nextSong}
             prevSong={prevSong}
           />

gmus-web/src/hooks/status.spec.tsx

@@ -1,45 +1,118 @@
-import { render, waitFor } from '@testing-library/react';
+import { render, RenderResult, waitFor } from '@testing-library/react';
 import nock from 'nock';
 import React from 'react';
+import { songInfoFetched } from '../actions';
+import { DispatchContext, StateContext } from '../context/state';
+import { GlobalState, initialState } from '../reducer';
 import { Song } from '../types';
 
 import { useCurrentlyPlayingSongInfo } from './status';
 
 describe(useCurrentlyPlayingSongInfo.name, () => {
-  const TestComponent: React.FC<{ songId: number | null }> = ({ songId }) => {
-    const songInfo = useCurrentlyPlayingSongInfo(songId);
-    return <div data-testid="info">{JSON.stringify(songInfo)}</div>;
+  const TestComponent: React.FC = () => {
+    useCurrentlyPlayingSongInfo();
+    return null;
+  };
+
+  const dispatch = jest.fn();
+
+  const setup = (state: GlobalState): RenderResult =>
+    render(
+      <StateContext.Provider value={state}>
+        <DispatchContext.Provider value={dispatch}>
+          <TestComponent />
+        </DispatchContext.Provider>
+      </StateContext.Provider>,
+    );
+
+  const testSong: Song = {
+    id: 1765,
+    track: 12,
+    title: 'My song',
+    artist: 'My artist',
+    album: 'My album',
+    time: 218,
   };
 
   describe('when there is no song ID', () => {
-    it('should return null', () => {
-      expect.assertions(1);
-      const { getByTestId } = render(<TestComponent songId={null} />);
-      expect(JSON.parse(getByTestId('info').innerHTML)).toBeNull();
+    const stateNoId: GlobalState = {
+      ...initialState,
+      player: {
+        ...initialState.player,
+        songId: null,
+      },
+    };
+
+    describe('when there is no song info in state', () => {
+      const stateNoIdNoInfo: GlobalState = {
+        ...stateNoId,
+        songInfo: null,
+      };
+
+      it('should not do anything', () => {
+        expect.assertions(1);
+        setup(stateNoIdNoInfo);
+        expect(dispatch).not.toHaveBeenCalled();
+      });
+    });
+
+    describe('when there is song info in state', () => {
+      const stateNoIdWithInfo: GlobalState = {
+        ...stateNoId,
+        songInfo: testSong,
+      };
+
+      it('should dispatch an action to clear the current info', () => {
+        expect.assertions(2);
+        setup(stateNoIdWithInfo);
+        expect(dispatch).toHaveBeenCalledTimes(1);
+        expect(dispatch).toHaveBeenCalledWith(songInfoFetched(null));
+      });
     });
   });
 
   describe('when there is a song ID in state', () => {
-    const testSong: Song = {
-      id: 1765,
-      track: 12,
-      title: 'My song',
-      artist: 'My artist',
-      album: 'My album',
-      time: 218,
+    const stateWithSongId: GlobalState = {
+      ...initialState,
+      player: {
+        ...initialState.player,
+        songId: testSong.id,
+      },
     };
 
-    beforeEach(() => {
-      nock('http://my-api.url:1234')
-        .get('/song-info?id=1765')
-        .reply(200, testSong, { 'Access-Control-Allow-Origin': '*' });
+    describe('when the song info is already fetched for the playing song ID', () => {
+      const stateFetched: GlobalState = {
+        ...stateWithSongId,
+        songInfo: testSong,
+      };
+
+      it('should not do anything', () => {
+        expect.assertions(1);
+        setup(stateFetched);
+        expect(dispatch).not.toHaveBeenCalled();
+      });
     });
 
-    it('should return the song info from the API', async () => {
-      expect.assertions(2);
-      const { getByTestId } = render(<TestComponent songId={1765} />);
-      await waitFor(() => {
-        expect(JSON.parse(getByTestId('info').innerHTML)).toStrictEqual(testSong);
+    describe('when the song info is stale', () => {
+      const stateStale: GlobalState = {
+        ...stateWithSongId,
+        songInfo: { ...testSong, id: testSong.id + 1 },
+      };
+
+      beforeEach(() => {
+        nock('http://my-api.url:1234')
+          .get('/song-info?id=1765')
+          .reply(200, testSong, { 'Access-Control-Allow-Origin': '*' });
+      });
+
+      it('should fetch the info for the updated song ID, and update the state', async () => {
+        expect.assertions(3);
+        setup(stateStale);
+        await waitFor(() => {
+          expect(dispatch).toHaveBeenCalledTimes(1);
+        });
+
+        expect(dispatch).toHaveBeenCalledWith(songInfoFetched(testSong));
       });
     });
   });

gmus-web/src/hooks/status.ts

@@ -1,22 +1,40 @@
 import { AxiosInstance, AxiosResponse } from 'axios';
-import { useCallback, useEffect } from 'react';
+import { useCallback, useContext, useEffect } from 'react';
+import { songInfoFetched } from '../actions';
+import { DispatchContext, StateContext } from '../context/state';
 
 import { Song } from '../types';
 import { getApiUrl } from '../utils/url';
 import { useRequestCallback } from './request';
 
-export function useCurrentlyPlayingSongInfo(songId: number | null): Song | null {
+export function useCurrentlyPlayingSongInfo(): void {
+  const state = useContext(StateContext);
+  const dispatch = useContext(DispatchContext);
+
   const sendRequest = useCallback(
     (axios: AxiosInstance, id: number): Promise<AxiosResponse<Song>> =>
       axios.get(`${getApiUrl()}/song-info?id=${id}`),
     [],
   );
 
-  const [onFetch, response] = useRequestCallback<number, Song>({ sendRequest });
+  const [onFetch, response, , cancelRequest] = useRequestCallback<number, Song>({ sendRequest });
+
+  useEffect(() => {
+    if (state.player.songId) {
+      if (state.player.songId === state.songInfo?.id) {
+        cancelRequest.current?.();
+      } else {
+        onFetch(state.player.songId);
+      }
+    } else if (state.songInfo?.id) {
+      cancelRequest.current?.();
+      dispatch(songInfoFetched(null));
+    }
+  }, [dispatch, state.player.songId, state.songInfo?.id, onFetch, cancelRequest]);
+
   useEffect(() => {
-    if (songId) {
-      onFetch(songId);
+    if (response?.id === state.player.songId) {
+      dispatch(songInfoFetched(response));
     }
-  }, [onFetch, songId]);
-  return response;
+  }, [dispatch, response, state.player.songId]);
 }

gmus-web/src/reducer/reducer.spec.ts

@@ -8,8 +8,10 @@ import {
   nameSet,
   playPaused,
   seeked,
+  songInfoFetched,
   stateSet,
 } from '../actions';
+import { Song } from '../types';
 import { MusicPlayer } from '../types/state';
 import { globalReducer, initialState, nullPlayer } from './reducer';
 import { GlobalState } from './types';
@@ -496,4 +498,23 @@ describe(globalReducer.name, () => {
       });
     });
   });
+
+  describe(ActionTypeLocal.SongInfoFetched, () => {
+    const song: Song = {
+      id: 123,
+      track: 17,
+      title: 'Some song',
+      artist: 'Some artist',
+      album: 'Some album',
+      time: 214,
+    };
+
+    const action = songInfoFetched(song);
+
+    it('should set the song info in state', () => {
+      expect.assertions(1);
+      const result = globalReducer(initialState, action);
+      expect(result.songInfo).toStrictEqual<Song>(song);
+    });
+  });
 });

gmus-web/src/reducer/reducer.ts

@@ -19,6 +19,7 @@ export const nullPlayer: MusicPlayer = {
 
 export const initialState: GlobalState = {
   initialised: false,
+  songInfo: null,
   player: nullPlayer,
   clientList: [],
   myClientName: '',
@@ -105,6 +106,9 @@ export function globalReducer(state: GlobalState, action: AnyAction): GlobalStat
       }
       return { ...state, player: { ...state.player, playing: !state.player.playing } };
 
+    case ActionTypeLocal.SongInfoFetched:
+      return { ...state, songInfo: action.payload };
+
     default:
       return state;
   }

gmus-web/src/reducer/types.ts

@@ -1,8 +1,10 @@
+import { Song } from '../types';
 import { Member, MusicPlayer } from '../types/state';
 
 export type GlobalState = {
   initialised: boolean;
   player: MusicPlayer;
+  songInfo: Song | null;
   clientList: Member[];
   myClientName: string;
 };