feat: global state setter actions using functions of previous state

gmus-web/src/actions/actions.ts

@@ -1,3 +1,4 @@
+import { SetStateAction } from 'react';
 import { Song } from '../types';
 import { Member, MusicPlayer } from '../types/state';
 import { ActionErrorOccurred } from './error';
@@ -23,12 +24,14 @@ export const nameSet = (name: string): ActionNameSet => ({
 
 export type ActionStateSetLocal = ActionLocal<
   ActionTypeLocal.StateSet,
-  Omit<Partial<MusicPlayer>, 'seekTime'>
+  SetStateAction<Omit<Partial<MusicPlayer>, 'seekTime'>>
 >;
 
-export const stateSet = (state: Partial<MusicPlayer> = {}): ActionStateSetLocal => ({
+export const stateSet = (
+  payload: SetStateAction<Partial<MusicPlayer>> = {},
+): ActionStateSetLocal => ({
   type: ActionTypeLocal.StateSet,
-  payload: state,
+  payload,
 });
 
 export type ActionSeeked = ActionLocal<ActionTypeLocal.Seeked, number>;

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

@@ -13,13 +13,18 @@ import {
   songInfoFetched,
   stateSet,
 } from '../actions';
-import { GlobalState, initialState } from '../reducer';
+import { globalReducer, GlobalState, initialState } from '../reducer';
 import { Song } from '../types';
 import { MusicPlayer } from '../types/state';
 import { globalEffects } from './effects';
 
 describe(globalEffects.name, () => {
   describe(ActionTypeLocal.StateSet, () => {
+    const state: GlobalState = {
+      ...initialState,
+      myClientName: 'my-client-name',
+    };
+
     it('should create a remote state set action', () => {
       expect.assertions(1);
 
@@ -28,24 +33,19 @@ describe(globalEffects.name, () => {
         playing: false,
         currentTime: 83,
         seekTime: 87,
-        master: 'my-client',
+        master: 'my-client-name',
         activeClients: [],
         queue: [],
         shuffleMode: false,
       };
 
-      const prevState: GlobalState = {
-        ...initialState,
-        myClientName: 'my-client-name',
-      };
-
       const action = stateSet(localPlayer);
 
-      const result = globalEffects(prevState, action);
+      const result = globalEffects(globalReducer(state, action), action);
 
       expect(result).toStrictEqual<ActionStateSetRemote>({
         type: ActionTypeRemote.StateSet,
-        payload: localPlayer,
+        payload: { ...localPlayer, seekTime: 83 },
       });
     });
   });

gmus-web/src/effects/effects.ts

@@ -39,24 +39,28 @@ function pushToQueue(state: GlobalState, action: ActionQueuePushed): RemoteActio
   };
 }
 
-export function globalEffects(prevState: GlobalState, action: LocalAction): RemoteAction | null {
+function sendStateUpdateToServer(state: GlobalState): RemoteAction | null {
+  if (!state.player.master) {
+    return null;
+  }
+  return {
+    type: ActionTypeRemote.StateSet,
+    payload: state.player,
+  };
+}
+
+export function globalEffects(state: GlobalState, action: LocalAction): RemoteAction | null {
   switch (action.type) {
     case ActionTypeLocal.StateSet:
-      if (!prevState.player.master && !action.payload.master) {
-        return null;
-      }
-      return {
-        type: ActionTypeRemote.StateSet,
-        payload: { ...prevState.player, ...action.payload },
-      };
+      return sendStateUpdateToServer(state);
 
     case ActionTypeLocal.Seeked:
-      if (!prevState.player.master) {
+      if (!state.player.master) {
         return null;
       }
       return {
         type: ActionTypeRemote.StateSet,
-        payload: { ...prevState.player, seekTime: action.payload },
+        payload: { ...state.player, seekTime: action.payload },
       };
 
     case ActionTypeLocal.MasterSet:
@@ -64,8 +68,8 @@ export function globalEffects(prevState: GlobalState, action: LocalAction): Remo
         return {
           type: ActionTypeRemote.StateSet,
           payload: {
-            ...prevState.player,
-            seekTime: prevState.player.currentTime,
+            ...state.player,
+            seekTime: state.player.currentTime,
             master: action.payload,
           },
         };
@@ -74,10 +78,10 @@ export function globalEffects(prevState: GlobalState, action: LocalAction): Remo
       return {
         type: ActionTypeRemote.StateSet,
         payload: {
-          ...prevState.player,
+          ...state.player,
           playing: false,
           seekTime: -1,
-          master: prevState.myClientName,
+          master: state.myClientName,
         },
       };
 
@@ -85,10 +89,10 @@ export function globalEffects(prevState: GlobalState, action: LocalAction): Remo
       return {
         type: ActionTypeRemote.StateSet,
         payload: {
-          ...prevState.player,
-          activeClients: prevState.player.activeClients.includes(action.payload)
-            ? prevState.player.activeClients.filter((client) => client !== action.payload)
-            : [...prevState.player.activeClients, action.payload],
+          ...state.player,
+          activeClients: state.player.activeClients.includes(action.payload)
+            ? state.player.activeClients.filter((client) => client !== action.payload)
+            : [...state.player.activeClients, action.payload],
         },
       };
 
@@ -96,19 +100,19 @@ export function globalEffects(prevState: GlobalState, action: LocalAction): Remo
       return {
         type: ActionTypeRemote.StateSet,
         payload: {
-          ...prevState.player,
-          playing: !prevState.player.playing,
+          ...state.player,
+          playing: !state.player.playing,
         },
       };
 
     case ActionTypeLocal.SongInfoFetched:
-      if (isMaster(prevState) || !action.payload.replace || !prevState.player.master) {
+      if (isMaster(state) || !action.payload.replace || !state.player.master) {
         return null;
       }
       return {
         type: ActionTypeRemote.StateSet,
         payload: {
-          ...prevState.player,
+          ...state.player,
           songId: action.payload.song?.id ?? null,
           playing: !!action.payload.song,
           currentTime: 0,
@@ -117,37 +121,37 @@ export function globalEffects(prevState: GlobalState, action: LocalAction): Remo
       };
 
     case ActionTypeLocal.QueuePushed:
-      return pushToQueue(prevState, action);
+      return pushToQueue(state, action);
     case ActionTypeLocal.QueueShifted:
-      if (!prevState.player.master) {
+      if (!state.player.master) {
         return null;
       }
       return {
         type: ActionTypeRemote.StateSet,
         payload: {
-          ...prevState.player,
-          queue: prevState.player.queue.slice(1),
-          playing: !!prevState.player.queue[0],
-          songId: prevState.player.queue[0],
+          ...state.player,
+          queue: state.player.queue.slice(1),
+          playing: !!state.player.queue[0],
+          songId: state.player.queue[0],
           currentTime: 0,
           seekTime: 0,
         },
       };
     case ActionTypeLocal.QueueRemoved:
-      if (!prevState.player.master) {
+      if (!state.player.master) {
         return null;
       }
       return {
         type: ActionTypeRemote.StateSet,
         payload: {
-          ...prevState.player,
-          queue: prevState.player.queue.filter((id) => id !== action.payload),
+          ...state.player,
+          queue: state.player.queue.filter((id) => id !== action.payload),
         },
       };
     case ActionTypeLocal.QueueOrdered:
       return {
         type: ActionTypeRemote.StateSet,
-        payload: { ...prevState.player, queue: reorderQueue(prevState.player.queue, action) },
+        payload: { ...state.player, queue: reorderQueue(state.player.queue, action) },
       };
 
     default:

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

@@ -300,6 +300,27 @@ describe(globalReducer.name, () => {
           });
         });
       });
+
+      describe('when the state update is a function', () => {
+        const actionFn = stateSet((last) => ({
+          ...last,
+          currentTime: (last.currentTime ?? 0) + 4,
+        }));
+
+        it('should set the state from the given function', () => {
+          expect.assertions(1);
+          const result = globalReducer(stateMaster, actionFn);
+
+          expect(result.player).toStrictEqual<MusicPlayer>({
+            ...nullPlayer,
+            master: 'some-master-client',
+            seekTime: -1,
+            songId: null,
+            playing: false,
+            currentTime: 31 + 4,
+          });
+        });
+      });
     });
 
     describe('when the client is a slave', () => {

gmus-web/src/reducer/reducer.ts

@@ -6,7 +6,12 @@ import {
   ActionTypeRemote,
   AnyAction,
 } from '../actions';
-import { isFromOurselves, isMaster, willBeMaster } from '../selectors';
+import {
+  getNextPlayerStateFromAction,
+  isFromOurselves,
+  isMaster,
+  willBeMaster,
+} from '../selectors';
 import { MusicPlayer } from '../types/state';
 import { GlobalState } from './types';
 
@@ -42,8 +47,10 @@ function onRemoteStateSet(state: GlobalState, action: ActionStateSetRemote): Glo
 }
 
 function onLocalStateSet(state: GlobalState, action: ActionStateSetLocal): GlobalState {
-  const nextPlayer: MusicPlayer = { ...state.player, ...action.payload };
-
+  const nextPlayer = getNextPlayerStateFromAction(state.player, action.payload);
+  if (!nextPlayer) {
+    return state;
+  }
   if (isMaster(state)) {
     return { ...state, player: nextPlayer };
   }

gmus-web/src/selectors.spec.ts

@@ -1,5 +1,5 @@
 import { ActionTypeRemote, stateSet } from './actions';
-import { GlobalState, initialState } from './reducer';
+import { GlobalState, initialState, nullPlayer } from './reducer';
 import { isActiveClient, isFromOurselves, isMaster, willBeMaster } from './selectors';
 
 describe('isMaster', () => {
@@ -139,12 +139,16 @@ describe('isFromOurselves', () => {
 });
 
 describe('willBeMaster', () => {
-  describe('when the action will cause the current client to be master', () => {
+  describe.each`
+    type                | action
+    ${'object-based'}   | ${stateSet({ master: 'a-slave-client' })}
+    ${'function-based'} | ${stateSet(() => ({ master: 'a-slave-client' }))}
+  `('when the $type action will cause the current client to be master', ({ action }) => {
     it('should return true', () => {
       expect.assertions(1);
-      expect(
-        willBeMaster({ myClientName: 'a-slave-client' }, stateSet({ master: 'a-slave-client' })),
-      ).toBe(true);
+      expect(willBeMaster({ player: nullPlayer, myClientName: 'a-slave-client' }, action)).toBe(
+        true,
+      );
     });
   });
 

gmus-web/src/selectors.ts

@@ -1,5 +1,19 @@
 import { ActionRemote, ActionStateSetLocal, ActionStateSetRemote } from './actions';
 import { GlobalState } from './reducer/types';
+import { MusicPlayer } from './types';
+
+export function getNextPlayerStateFromAction(
+  player: MusicPlayer | undefined,
+  payload: ActionStateSetLocal['payload'] | null,
+): MusicPlayer | null {
+  if (!(payload && player)) {
+    return null;
+  }
+  if (typeof payload === 'function') {
+    return { ...player, ...payload(player) };
+  }
+  return { ...player, ...payload };
+}
 
 export const isMaster = (state: Pick<GlobalState, 'player' | 'myClientName'>): boolean =>
   state.player.master === state.myClientName;
@@ -15,6 +29,13 @@ export const isFromOurselves = (
 export const willBeMaster = (
   state: Partial<GlobalState> & Pick<GlobalState, 'myClientName'>,
   action: ActionStateSetLocal | ActionStateSetRemote,
-): boolean => state.myClientName === action.payload?.master;
+): boolean => {
+  const actionHasMaster =
+    typeof action.payload === 'function' ? !!action.payload({}).master : !!action.payload?.master;
+  return (
+    actionHasMaster &&
+    state.myClientName === getNextPlayerStateFromAction(state.player, action.payload)?.master
+  );
+};
 
 export const getSongId = (state: Pick<GlobalState, 'player'>): number | null => state.player.songId;