Proposal: a modern & typed way of writing Redux actions doing API requests

[renchap]

At the moment, writing Redux actions that perform API requests requires a lot of boilerplate and is not easily typed.

This PR proposes a new way of writing those, using the new createDataLoadingThunk() function.

It is based on RTK's createAsyncThunk:

• You give it a base name for the action. Here I used an object/action pattern as it makes things more explicit.
• Then you give it a data loading function. apiRequest has been created to allow easy definition of data loading functions from the Mastodon API
• The API-loading functions are now defined in the api/ directory, separate from the actions, and are fully typed (arguments + what the API returns). They no longer depend on Redux, which makes them easier to use (no need to pass getState). I intend to remove the one api(getState) function in another PR, and avoid loading the access token into the Redux state
• You can provide an optional onData attribute to createDataLoadingThunk, allowing you to both execute code after the data is fetched (for example see reblog() in this PR) and transform the data that is returned to the reducer (see submitAccountNote() which puts the data in an object)
• If there is no explicit return in onData (or it returns undefined), then the API result is the payload. This allows to do side-effects while preserving the payload
• If you are not interested in the API results, you can return discardApiResults which will make an empty payload. This can be used when you do not care about what the API returns, and will avoid serialising large actions with the whole result
• You can also provide specific options for middlewares, like skipLoading
• Like with createAsyncThunk, 3 actions are dispatched during the lifetime of the thunk:
  • myThunk.pending when the thunk is first invoked
  • myThunk.fulfilled when the thunk returns correctly
  • myThunk.rejected when an error happens in the thunk
• You can access the parameters for a thunk using action.meta.params (typed correctly), the result with action.payload when fulfilled, or the error with action.error when rejected
• I needed to create the custom createThunk wrapper to inject the middleware meta parameters into the action when fulfilled/rejected, because this is not supported (yet?) by RTK. I intend to open a feature request to get it done upstream
• To have a look at what is involved for converting 2 existing actions to this new way, you can have a look at the 3rd commit which only includes rewrites of reblog() and unreblog()

I need to test this more, and try implementing other actions to see if it can handle all the cases, but I am optimistic that it will make the code better and without much boilerplate.

[renchap]

Updated with a (much complex) signature for createDataLoading thunk but it should cover all the use cases.

I also added some JSDoc to make it easier to understand what is does.

[ThisIsMissEm]

A few small suggestions, but LGTMO

[ClearlyClaire]

Having proper typing and avoiding all the async lifecycle boilerplate are huge pluses, but I'm slightly worried that splitting API requests and actions will make things more difficult to grasp, especially when those are in different functions.

[renchap]

You could use inline loadData functions and not have those separated. My reasoning for staging to split those is to slowly build an (internal, for now?) Typescript Mastodon API library which provides you with 100% typed functions for all API calls.

It also allows to more easily tie together the frontend and the backend, where:

• a serializer has (loosely) a corresponding file in api_types/
• a controller has a corresponding file in api/

If you change one, then you change the other, and they are isolated such that they are easy to understand and update.

In the longer term, I foresee us wanting to use a library to automatically generate the API functions & types, for example from an OpenAPI spec, for example using https://openapi-ts.pages.dev or https://orval.dev

[ClearlyClaire]

That sounds sensible.

However, re-reading the changes, I have also a minor gripe with the fact that this split obscures the action's signature behind another layer of indirection.

Otherwise this looks good to me.

[renchap]

However, re-reading the changes, I have also a minor gripe with the fact that this split obscures the action's signature behind another layer of indirection.

I see what you mean. If you are using a language server in your editor then those are displayed, but they are not explicit and you need to go look at the parameters from the API request function.

This is better if the loadData function is inlined:

export const reblog = createDataLoadingThunk(
  'status/reblog',
  (statusId: string, visibility: StatusVisibility) =>
    apiRequest<{ reblog: Status }>('post', `v1/statuses/${statusId}/reblog`, {
      visibility,
    }),
  (data, { dispatch, discardLoadData }) => {
    dispatch(importFetchedStatus(data.reblog));
    return discardLoadData;
  },
);

I am not against always inlining it, it is probably a good idea anyway to not tie the action parameters and the API call parameters, and this probably will need an intermediate step if we switch to an OpenAPI generator.

Another alternative is to embrace this redirection layer completely and make everything explicit:

export const reblog = createDataLoadingThunk(
  'status/reblog',
  (statusId: string, visibility: StatusVisibility) => apiReblog(statusId, visibility),
  (data, { dispatch, discardLoadData }) => {
    dispatch(importFetchedStatus(data.reblog));
    return discardLoadData;
  },
);

What do you think?

[ClearlyClaire]

I think both approaches are fine and more readable than the current commit. I don't have a strong opinion on whether to inline the API request or not.

I think in the absence of automatically-generated typed functions from API spec, I prefer them inlined, but this can be split if you think that will make moving to automatically-generated typed functions easier.

[renchap]

Updated with explicit arguments for the new actions

[github-actions]

This pull request has merge conflicts that must be resolved before it can be merged.

[github-actions]

This pull request has resolved merge conflicts and is ready for review.

[ClearlyClaire]

Looks good to me overall!

I'm not sure how to make it more readable, but both the removal of async lifecycle boilerplate and the proper typing of the actions are huge improvements.