Переглянути джерело

fix: correct effect dispatch order

    * fix: dispatch action with effect in next event loop
    * fix: don't set seekTime unnecessarily
    * fix: treat state as previous state in effect builders
    * feat: temporary branch deployment
Fela Maslen 4 роки тому
батько
коміт
f9d8d70418

+ 1 - 3
Jenkinsfile

@@ -59,9 +59,7 @@ node {
     }
 
     stage('Deploy') {
-      if (env.BRANCH_NAME == "master") {
-        sh 'LIBRARY_DIRECTORY=$GMUS_LIBRARY_DIRECTORY ./k8s/deploy.sh'
-      }
+      sh 'LIBRARY_DIRECTORY=$GMUS_LIBRARY_DIRECTORY ./k8s/deploy.sh'
     }
   }
 }

+ 8 - 12
gmus-web/src/components/app.tsx

@@ -58,25 +58,21 @@ export const App: React.FC<Props> = ({
     isDesktop,
   ]);
 
-  const shouldPlay = isActiveClient(state) && !!state.player.songId;
-  const wasPlaying = useRef<boolean>(false);
+  const shouldLoadAudio = isActiveClient(state) && !!state.player.songId;
+  const didLoadAudio = useRef<boolean>(false);
   const [seekTime, setSeekTime] = useState<number>(-1);
   useEffect(() => {
-    if (shouldPlay && !wasPlaying.current) {
-      wasPlaying.current = true;
+    if (shouldLoadAudio && !didLoadAudio.current) {
+      didLoadAudio.current = true;
       setSeekTime(state.player.seekTime === -1 ? state.player.currentTime : state.player.seekTime);
-    } else if (state.player.seekTime !== -1) {
-      setSeekTime(state.player.seekTime);
+    } else if (!shouldLoadAudio && didLoadAudio.current) {
+      didLoadAudio.current = false;
     }
-
-    if (!shouldPlay && wasPlaying.current) {
-      wasPlaying.current = false;
-    }
-  }, [shouldPlay, state.player.currentTime, state.player.seekTime]);
+  }, [shouldLoadAudio, state.player.currentTime, state.player.seekTime]);
 
   return (
     <>
-      {shouldPlay && (
+      {shouldLoadAudio && (
         <Player
           src={getSongUrl(state.player.songId as number)}
           playing={state.player.playing}

+ 4 - 5
gmus-web/src/effects/effects.spec.ts

@@ -13,9 +13,8 @@ import {
   songInfoFetched,
   stateSet,
 } from '../actions';
-import { globalReducer, GlobalState, initialState } from '../reducer';
+import { GlobalState, initialState } from '../reducer';
 import { Song } from '../types';
-import { MusicPlayer } from '../types/state';
 import { globalEffects } from './effects';
 
 describe(globalEffects.name, () => {
@@ -28,7 +27,7 @@ describe(globalEffects.name, () => {
     it('should create a remote state set action', () => {
       expect.assertions(1);
 
-      const localPlayer: MusicPlayer = {
+      const localPlayer = {
         songId: 123,
         playing: false,
         currentTime: 83,
@@ -41,11 +40,11 @@ describe(globalEffects.name, () => {
 
       const action = stateSet(localPlayer);
 
-      const result = globalEffects(globalReducer(state, action), action);
+      const result = globalEffects(state, action);
 
       expect(result).toStrictEqual<ActionStateSetRemote>({
         type: ActionTypeRemote.StateSet,
-        payload: { ...localPlayer, seekTime: 83 },
+        payload: { ...state.player, ...localPlayer },
       });
     });
   });

+ 10 - 5
gmus-web/src/effects/effects.ts

@@ -1,13 +1,14 @@
 import {
   ActionQueueOrdered,
   ActionQueuePushed,
+  ActionStateSetLocal,
   ActionTypeLocal,
   ActionTypeRemote,
   LocalAction,
   RemoteAction,
 } from '../actions';
 import { GlobalState } from '../reducer/types';
-import { isMaster } from '../selectors';
+import { getNextPlayerStateFromAction, isMaster } from '../selectors';
 
 const reverseInArray = <T>(array: T[], index: number): T[] => [
   ...array.slice(0, Math.max(0, index)),
@@ -39,20 +40,24 @@ function pushToQueue(state: GlobalState, action: ActionQueuePushed): RemoteActio
   };
 }
 
-function sendStateUpdateToServer(state: GlobalState): RemoteAction | null {
-  if (!state.player.master) {
+function sendStateUpdateToServer(
+  state: GlobalState,
+  action: ActionStateSetLocal,
+): RemoteAction | null {
+  const nextPlayer = getNextPlayerStateFromAction(state.player, action.payload);
+  if (!state.player.master && !nextPlayer?.master) {
     return null;
   }
   return {
     type: ActionTypeRemote.StateSet,
-    payload: state.player,
+    payload: nextPlayer,
   };
 }
 
 export function globalEffects(state: GlobalState, action: LocalAction): RemoteAction | null {
   switch (action.type) {
     case ActionTypeLocal.StateSet:
-      return sendStateUpdateToServer(state);
+      return sendStateUpdateToServer(state, action);
 
     case ActionTypeLocal.Seeked:
       if (!state.player.master) {

+ 14 - 9
gmus-web/src/hooks/socket.spec.tsx

@@ -57,7 +57,7 @@ describe(useDispatchWithEffects.name, () => {
 
   const state = ({ my: 'state' } as unknown) as GlobalState;
 
-  const dispatch: Dispatch<AnyAction> = jest.fn();
+  const dispatch = jest.fn();
 
   const socket = ({
     send: jest.fn(),
@@ -91,15 +91,17 @@ describe(useDispatchWithEffects.name, () => {
         globalEffectsSpy = jest.spyOn(effects, 'globalEffects').mockReturnValueOnce(null);
       });
 
-      it('should dispatch the action to the local store', () => {
-        expect.assertions(2);
+      it('should dispatch the action to the local store', async () => {
+        expect.hasAssertions();
         const { getByText } = render(<TestComponent />);
+        expect(dispatch).not.toHaveBeenCalled();
         act(() => {
           fireEvent.click(getByText('Dispatch!'));
         });
 
-        expect(dispatch).toHaveBeenCalledTimes(1);
-        expect(dispatch).toHaveBeenCalledWith(someAction);
+        await waitFor(() => {
+          expect(dispatch).toHaveBeenCalledWith(someAction);
+        });
       });
 
       it('should not send a message to the socket', () => {
@@ -118,15 +120,18 @@ describe(useDispatchWithEffects.name, () => {
         globalEffectsSpy = jest.spyOn(effects, 'globalEffects').mockReturnValueOnce(someEffect);
       });
 
-      it('should dispatch the action to the local store', () => {
-        expect.assertions(2);
+      it('should dispatch the action to the local store', async () => {
+        expect.hasAssertions();
+
         const { getByText } = render(<TestComponent />);
+        expect(dispatch).not.toHaveBeenCalled();
         act(() => {
           fireEvent.click(getByText('Dispatch!'));
         });
 
-        expect(dispatch).toHaveBeenCalledTimes(1);
-        expect(dispatch).toHaveBeenCalledWith(someAction);
+        await waitFor(() => {
+          expect(dispatch).toHaveBeenCalledWith(someAction);
+        });
       });
 
       it('should send a message to the socket', () => {

+ 3 - 1
gmus-web/src/hooks/socket.ts

@@ -43,7 +43,9 @@ export function useDispatchWithEffects(
         localStorage.removeItem(clientNameKey);
       } else {
         setLastAction(action);
-        dispatch(action);
+        setImmediate(() => {
+          dispatch(action);
+        });
       }
     },
     [dispatch, socket],