Await a dispatch thunk before executing some code is working but compiler complains

Question

So I was tasked to fix some buttons so that there is a loading spinner present when they are clicked. The problem is that my solution is not ideal since intellij complained that I have unessesarry await. I would love to have your collective experience so I know which direction is the right one ( and if possible some explanation, since i found it also in other places in the source code)

In the ImportExport.tsx i have the following , where the setIsExportAktionAktive is what i use to show that the button is loading

  const handleOnExport = useCallback(
    async (fpNumber?: number) => {
      setIsExportAktionAktive(true)
      const statusCode2Error: Record<string, string> = {
        '403': I18n.t(.....blabla),
        '423': I18n.t(.....blabla)
      }
      try {
       // this line is the problem intellij complains , if i remove it ,it does not wait for the export to finish and gets setIsExportAktionAktive(false) almost immediately !
       await dispatch(export(fpNumber, statusCode2Error, handleImportExportError))
      }catch (error){
        console.error(error)
      } finally {
        setIsExportAktionAktive(false)
      }

    },
    [I18n, dispatch, handleImportExportError]
  )

In the ImportExportSlice.ts i have the following

export const export=
  (
    fpNumber?: number,
    errorMsg?: Record<string, string>,
    callback?: (status: string, errorMsg: Record<string, string>) => void
  ): AppThunk =>
  async (dispatch) => {
    await (fpNummer ? importExportApi.export(fpNumber) : importExportApi.export())
      .catch((error) => {
        const statusCode: string = error.response.status.toString()
        dispatch(setExportErrorStatusCode(statusCode))
        callback?.(statusCode, errorMsg)
      })
      .finally(() => dispatch(importExportProtokollLoad()))
  }

AppThunk is defined like this

export type AppThunk = ThunkAction<void, RootState, null, Action<string>>

I could put it in the slice in the State, but i dont think/ dont know if thats better. Also async await in a useCallback is bad. Is there a way for me to actually return a Promise so that i can chain a then ? I am pretty sure i am going to have a hard time in the code review with my current solution (most likely also if i put it in the state ) Any thoughts ?

Answer 1

It does certainly appear to be useless since there is nothing to do after the call. Even the thunk itself appears to not need to be declared async and await anything because export doesn't return anything nor does it have any logic after the await. It also mixes async/await with a Promise chain which is a bit of a Javascript anti-pattern.

Update export to return an actual Promise instead of implicitly returning one, either by returning the Promise chain or converting to async/await.

• Return the Promise chain:

  export const export = (
    fpNumber?: number,
    errorMsg?: Record<string, string>,
    callback?: (status: string, errorMsg: Record<string, string>) => void
  ): AppThunk => (dispatch) => {
    return (fpNummer
      ? importExportApi.export(fpNumber)
      : importExportApi.export()
    )
      .catch((error) => {
        const statusCode: string = error.response.status.toString();
        dispatch(setExportErrorStatusCode(statusCode));
        callback?.(statusCode, errorMsg);
      })
      .finally(() => dispatch(importExportProtokollLoad()));
  };
  

• Use async/await:

  export const export = (
    fpNumber?: number,
    errorMsg?: Record<string, string>,
    callback?: (status: string, errorMsg: Record<string, string>) => void
  ): AppThunk => async (dispatch) => {
    try {
      await (fpNummer
        ? importExportApi.export(fpNumber)
        : importExportApi.export()
      );
    } catch(error) {
      const statusCode: string = error.response.status.toString();
      dispatch(setExportErrorStatusCode(statusCode));
      callback?.(statusCode, errorMsg);
    } finally {
      dispatch(importExportProtokollLoad());
    }
  };
  

Now that export is properly returning an awaited Promise the handleOnExport callback should be able to await it, or here also the Promise can be returned to the caller.

const handleOnExport = useCallback(
  (fpNumber?: number) => {
    setIsExportAktionAktive(true)
    const statusCode2Error: Record<string, string> = {
      '403': I18n.t(.....blabla),
      '423': I18n.t(.....blabla)
    }
    try {
      return dispatch(export(
        fpNumber,
        statusCode2Error,
        handleImportExportError
      ));
    } catch (error) {
      console.error(error);
    } finally {
      setIsExportAktionAktive(false));
    }
  },
  [I18n, dispatch, handleImportExportError]
);

If you are curious about using Redux-Toolkit to write the thunk this is how it could be implemented and used:

import { createAsyncThunk } from '@reduxjs/toolkit';

export const export = createAsyncThunk(
  "export",
  async (
    { fpNumber, errorMsg }: {
      fpNumber?: number;
      errorMsg?: Record<string, string>;
    },
    { dispatch, rejectWithValue } // ThunkAPI
  ) => {
    try {
      return fpNummer
        ? importExportApi.export(fpNumber)
        : importExportApi.export();
    } catch(error) {
      const statusCode: string = error.response.status.toString();
      return rejectWithValue({ statusCode, errorMsg });
    } finally {
      dispatch(importExportProtokollLoad();
    }
  },
);

const handleOnExport = useCallback(
  (fpNumber?: number) => {
    setIsExportAktionAktive(true)
    const statusCode2Error: Record<string, string> = {
      '403': I18n.t(.....blabla),
      '423': I18n.t(.....blabla)
    }
    try {
      dispatch(export(
        fpNumber,
        statusCode2Error,
       
      )).unwrap();
    } catch (error) {
      console.error(error);
      handleImportExportError(error.statusCode, error.errorMsg);
    } finally {
      setIsExportAktionAktive(false));
    }
  },
  [I18n, dispatch, handleImportExportError]
);