From 10ec421dd4e0da987e69a3dd7f4f696f9c5878e0 Mon Sep 17 00:00:00 2001 From: Renaud Chaput Date: Thu, 23 May 2024 11:50:13 +0200 Subject: [PATCH] Proposal: a modern & typed way of writing Redux actions doing API requests (#30270) --- .../mastodon/actions/account_notes.ts | 21 +- .../mastodon/actions/interactions.js | 86 +------- .../mastodon/actions/interactions_typed.ts | 30 +++ app/javascript/mastodon/api.ts | 16 +- app/javascript/mastodon/api/accounts.ts | 7 + app/javascript/mastodon/api/interactions.ts | 10 + .../mastodon/containers/status_container.jsx | 4 +- .../containers/account_note_container.js | 2 +- .../containers/notification_container.js | 4 +- .../picture_in_picture/components/footer.jsx | 4 +- .../containers/detailed_status_container.js | 4 +- .../mastodon/features/status/index.jsx | 4 +- app/javascript/mastodon/reducers/statuses.js | 28 +-- .../mastodon/store/typed_functions.ts | 186 ++++++++++++++++++ 14 files changed, 281 insertions(+), 125 deletions(-) create mode 100644 app/javascript/mastodon/actions/interactions_typed.ts create mode 100644 app/javascript/mastodon/api/accounts.ts create mode 100644 app/javascript/mastodon/api/interactions.ts diff --git a/app/javascript/mastodon/actions/account_notes.ts b/app/javascript/mastodon/actions/account_notes.ts index acd9ecf410..bf4f93dca9 100644 --- a/app/javascript/mastodon/actions/account_notes.ts +++ b/app/javascript/mastodon/actions/account_notes.ts @@ -1,18 +1,9 @@ -import type { ApiRelationshipJSON } from 'mastodon/api_types/relationships'; -import { createAppAsyncThunk } from 'mastodon/store/typed_functions'; +import { apiSubmitAccountNote } from 'mastodon/api/accounts'; +import { createDataLoadingThunk } from 'mastodon/store/typed_functions'; -import api from '../api'; - -export const submitAccountNote = createAppAsyncThunk( +export const submitAccountNote = createDataLoadingThunk( 'account_note/submit', - async (args: { id: string; value: string }) => { - const response = await api().post( - `/api/v1/accounts/${args.id}/note`, - { - comment: args.value, - }, - ); - - return { relationship: response.data }; - }, + (accountId: string, note: string) => apiSubmitAccountNote(accountId, note), + (relationship) => ({ relationship }), + { skipLoading: true }, ); diff --git a/app/javascript/mastodon/actions/interactions.js b/app/javascript/mastodon/actions/interactions.js index fe7c911b61..57f2459c01 100644 --- a/app/javascript/mastodon/actions/interactions.js +++ b/app/javascript/mastodon/actions/interactions.js @@ -3,10 +3,6 @@ import api, { getLinks } from '../api'; import { fetchRelationships } from './accounts'; import { importFetchedAccounts, importFetchedStatus } from './importer'; -export const REBLOG_REQUEST = 'REBLOG_REQUEST'; -export const REBLOG_SUCCESS = 'REBLOG_SUCCESS'; -export const REBLOG_FAIL = 'REBLOG_FAIL'; - export const REBLOGS_EXPAND_REQUEST = 'REBLOGS_EXPAND_REQUEST'; export const REBLOGS_EXPAND_SUCCESS = 'REBLOGS_EXPAND_SUCCESS'; export const REBLOGS_EXPAND_FAIL = 'REBLOGS_EXPAND_FAIL'; @@ -15,10 +11,6 @@ export const FAVOURITE_REQUEST = 'FAVOURITE_REQUEST'; export const FAVOURITE_SUCCESS = 'FAVOURITE_SUCCESS'; export const FAVOURITE_FAIL = 'FAVOURITE_FAIL'; -export const UNREBLOG_REQUEST = 'UNREBLOG_REQUEST'; -export const UNREBLOG_SUCCESS = 'UNREBLOG_SUCCESS'; -export const UNREBLOG_FAIL = 'UNREBLOG_FAIL'; - export const UNFAVOURITE_REQUEST = 'UNFAVOURITE_REQUEST'; export const UNFAVOURITE_SUCCESS = 'UNFAVOURITE_SUCCESS'; export const UNFAVOURITE_FAIL = 'UNFAVOURITE_FAIL'; @@ -51,83 +43,7 @@ export const UNBOOKMARK_REQUEST = 'UNBOOKMARKED_REQUEST'; export const UNBOOKMARK_SUCCESS = 'UNBOOKMARKED_SUCCESS'; export const UNBOOKMARK_FAIL = 'UNBOOKMARKED_FAIL'; -export function reblog(status, visibility) { - return function (dispatch) { - dispatch(reblogRequest(status)); - - api().post(`/api/v1/statuses/${status.get('id')}/reblog`, { visibility }).then(function (response) { - // The reblog API method returns a new status wrapped around the original. In this case we are only - // interested in how the original is modified, hence passing it skipping the wrapper - dispatch(importFetchedStatus(response.data.reblog)); - dispatch(reblogSuccess(status)); - }).catch(function (error) { - dispatch(reblogFail(status, error)); - }); - }; -} - -export function unreblog(status) { - return (dispatch) => { - dispatch(unreblogRequest(status)); - - api().post(`/api/v1/statuses/${status.get('id')}/unreblog`).then(response => { - dispatch(importFetchedStatus(response.data)); - dispatch(unreblogSuccess(status)); - }).catch(error => { - dispatch(unreblogFail(status, error)); - }); - }; -} - -export function reblogRequest(status) { - return { - type: REBLOG_REQUEST, - status: status, - skipLoading: true, - }; -} - -export function reblogSuccess(status) { - return { - type: REBLOG_SUCCESS, - status: status, - skipLoading: true, - }; -} - -export function reblogFail(status, error) { - return { - type: REBLOG_FAIL, - status: status, - error: error, - skipLoading: true, - }; -} - -export function unreblogRequest(status) { - return { - type: UNREBLOG_REQUEST, - status: status, - skipLoading: true, - }; -} - -export function unreblogSuccess(status) { - return { - type: UNREBLOG_SUCCESS, - status: status, - skipLoading: true, - }; -} - -export function unreblogFail(status, error) { - return { - type: UNREBLOG_FAIL, - status: status, - error: error, - skipLoading: true, - }; -} +export * from "./interactions_typed"; export function favourite(status) { return function (dispatch) { diff --git a/app/javascript/mastodon/actions/interactions_typed.ts b/app/javascript/mastodon/actions/interactions_typed.ts new file mode 100644 index 0000000000..5180806087 --- /dev/null +++ b/app/javascript/mastodon/actions/interactions_typed.ts @@ -0,0 +1,30 @@ +import { apiReblog, apiUnreblog } from 'mastodon/api/interactions'; +import type { StatusVisibility } from 'mastodon/models/status'; +import { createDataLoadingThunk } from 'mastodon/store/typed_functions'; + +import { importFetchedStatus } from './importer'; + +export const reblog = createDataLoadingThunk( + 'status/reblog', + (statusId: string, visibility: StatusVisibility) => + apiReblog(statusId, visibility), + (data, { dispatch, discardLoadData }) => { + // The reblog API method returns a new status wrapped around the original. In this case we are only + // interested in how the original is modified, hence passing it skipping the wrapper + dispatch(importFetchedStatus(data.reblog)); + + // The payload is not used in any actions + return discardLoadData; + }, +); + +export const unreblog = createDataLoadingThunk( + 'status/unreblog', + (statusId: string) => apiUnreblog(statusId), + (data, { dispatch, discardLoadData }) => { + dispatch(importFetchedStatus(data)); + + // The payload is not used in any actions + return discardLoadData; + }, +); diff --git a/app/javascript/mastodon/api.ts b/app/javascript/mastodon/api.ts index 2ccf178f00..4e5ccef08c 100644 --- a/app/javascript/mastodon/api.ts +++ b/app/javascript/mastodon/api.ts @@ -1,4 +1,4 @@ -import type { AxiosResponse, RawAxiosRequestHeaders } from 'axios'; +import type { AxiosResponse, Method, RawAxiosRequestHeaders } from 'axios'; import axios from 'axios'; import LinkHeader from 'http-link-header'; @@ -58,3 +58,17 @@ export default function api(withAuthorization = true) { ], }); } + +export async function apiRequest( + method: Method, + url: string, + params?: unknown, +) { + const { data } = await api().request({ + method, + url, + params, + }); + + return data; +} diff --git a/app/javascript/mastodon/api/accounts.ts b/app/javascript/mastodon/api/accounts.ts new file mode 100644 index 0000000000..51b1f4f8de --- /dev/null +++ b/app/javascript/mastodon/api/accounts.ts @@ -0,0 +1,7 @@ +import { apiRequest } from 'mastodon/api'; +import type { ApiRelationshipJSON } from 'mastodon/api_types/relationships'; + +export const apiSubmitAccountNote = (id: string, value: string) => + apiRequest('post', `/api/v1/accounts/${id}/note`, { + comment: value, + }); diff --git a/app/javascript/mastodon/api/interactions.ts b/app/javascript/mastodon/api/interactions.ts new file mode 100644 index 0000000000..4c466a1b46 --- /dev/null +++ b/app/javascript/mastodon/api/interactions.ts @@ -0,0 +1,10 @@ +import { apiRequest } from 'mastodon/api'; +import type { Status, StatusVisibility } from 'mastodon/models/status'; + +export const apiReblog = (statusId: string, visibility: StatusVisibility) => + apiRequest<{ reblog: Status }>('post', `v1/statuses/${statusId}/reblog`, { + visibility, + }); + +export const apiUnreblog = (statusId: string) => + apiRequest('post', `v1/statuses/${statusId}/unreblog`); diff --git a/app/javascript/mastodon/containers/status_container.jsx b/app/javascript/mastodon/containers/status_container.jsx index c6842e8df8..0174e5a02c 100644 --- a/app/javascript/mastodon/containers/status_container.jsx +++ b/app/javascript/mastodon/containers/status_container.jsx @@ -96,9 +96,9 @@ const mapDispatchToProps = (dispatch, { intl, contextType }) => ({ onModalReblog (status, privacy) { if (status.get('reblogged')) { - dispatch(unreblog(status)); + dispatch(unreblog(status.id)); } else { - dispatch(reblog(status, privacy)); + dispatch(reblog(status.id, privacy)); } }, diff --git a/app/javascript/mastodon/features/account/containers/account_note_container.js b/app/javascript/mastodon/features/account/containers/account_note_container.js index 20304a4524..9fbe0671c0 100644 --- a/app/javascript/mastodon/features/account/containers/account_note_container.js +++ b/app/javascript/mastodon/features/account/containers/account_note_container.js @@ -11,7 +11,7 @@ const mapStateToProps = (state, { account }) => ({ const mapDispatchToProps = (dispatch, { account }) => ({ onSave (value) { - dispatch(submitAccountNote({ id: account.get('id'), value})); + dispatch(submitAccountNote(account.get('id'), value)); }, }); diff --git a/app/javascript/mastodon/features/notifications/containers/notification_container.js b/app/javascript/mastodon/features/notifications/containers/notification_container.js index de450cd1ab..d829cb833e 100644 --- a/app/javascript/mastodon/features/notifications/containers/notification_container.js +++ b/app/javascript/mastodon/features/notifications/containers/notification_container.js @@ -39,12 +39,12 @@ const mapDispatchToProps = dispatch => ({ }, onModalReblog (status, privacy) { - dispatch(reblog(status, privacy)); + dispatch(reblog(status.id, privacy)); }, onReblog (status, e) { if (status.get('reblogged')) { - dispatch(unreblog(status)); + dispatch(unreblog(status.id)); } else { if (e.shiftKey || !boostModal) { this.onModalReblog(status); diff --git a/app/javascript/mastodon/features/picture_in_picture/components/footer.jsx b/app/javascript/mastodon/features/picture_in_picture/components/footer.jsx index d6b1b5fa81..1c142f3c10 100644 --- a/app/javascript/mastodon/features/picture_in_picture/components/footer.jsx +++ b/app/javascript/mastodon/features/picture_in_picture/components/footer.jsx @@ -123,7 +123,7 @@ class Footer extends ImmutablePureComponent { _performReblog = (status, privacy) => { const { dispatch } = this.props; - dispatch(reblog(status, privacy)); + dispatch(reblog(status.id, privacy)); }; handleReblogClick = e => { @@ -132,7 +132,7 @@ class Footer extends ImmutablePureComponent { if (signedIn) { if (status.get('reblogged')) { - dispatch(unreblog(status)); + dispatch(unreblog(status.id)); } else if ((e && e.shiftKey) || !boostModal) { this._performReblog(status); } else { diff --git a/app/javascript/mastodon/features/status/containers/detailed_status_container.js b/app/javascript/mastodon/features/status/containers/detailed_status_container.js index 1c650f544f..91bc700e98 100644 --- a/app/javascript/mastodon/features/status/containers/detailed_status_container.js +++ b/app/javascript/mastodon/features/status/containers/detailed_status_container.js @@ -74,12 +74,12 @@ const mapDispatchToProps = (dispatch, { intl }) => ({ }, onModalReblog (status, privacy) { - dispatch(reblog(status, privacy)); + dispatch(reblog(status.id, privacy)); }, onReblog (status, e) { if (status.get('reblogged')) { - dispatch(unreblog(status)); + dispatch(unreblog(status.id)); } else { if (e.shiftKey || !boostModal) { this.onModalReblog(status); diff --git a/app/javascript/mastodon/features/status/index.jsx b/app/javascript/mastodon/features/status/index.jsx index 3a9bf524f8..48f045a4af 100644 --- a/app/javascript/mastodon/features/status/index.jsx +++ b/app/javascript/mastodon/features/status/index.jsx @@ -299,7 +299,7 @@ class Status extends ImmutablePureComponent { }; handleModalReblog = (status, privacy) => { - this.props.dispatch(reblog(status, privacy)); + this.props.dispatch(reblog(status.id, privacy)); }; handleReblogClick = (status, e) => { @@ -308,7 +308,7 @@ class Status extends ImmutablePureComponent { if (signedIn) { if (status.get('reblogged')) { - dispatch(unreblog(status)); + dispatch(unreblog(status.id)); } else { if ((e && e.shiftKey) || !boostModal) { this.handleModalReblog(status); diff --git a/app/javascript/mastodon/reducers/statuses.js b/app/javascript/mastodon/reducers/statuses.js index 683fe848f7..1da1c9cf2f 100644 --- a/app/javascript/mastodon/reducers/statuses.js +++ b/app/javascript/mastodon/reducers/statuses.js @@ -3,10 +3,6 @@ import { Map as ImmutableMap, fromJS } from 'immutable'; import { STATUS_IMPORT, STATUSES_IMPORT } from '../actions/importer'; import { normalizeStatusTranslation } from '../actions/importer/normalizer'; import { - REBLOG_REQUEST, - REBLOG_FAIL, - UNREBLOG_REQUEST, - UNREBLOG_FAIL, FAVOURITE_REQUEST, FAVOURITE_FAIL, UNFAVOURITE_REQUEST, @@ -16,6 +12,10 @@ import { UNBOOKMARK_REQUEST, UNBOOKMARK_FAIL, } from '../actions/interactions'; +import { + reblog, + unreblog, +} from '../actions/interactions_typed'; import { STATUS_MUTE_SUCCESS, STATUS_UNMUTE_SUCCESS, @@ -65,6 +65,7 @@ const statusTranslateUndo = (state, id) => { const initialState = ImmutableMap(); +/** @type {import('@reduxjs/toolkit').Reducer} */ export default function statuses(state = initialState, action) { switch(action.type) { case STATUS_FETCH_REQUEST: @@ -91,14 +92,6 @@ export default function statuses(state = initialState, action) { return state.get(action.status.get('id')) === undefined ? state : state.setIn([action.status.get('id'), 'bookmarked'], false); case UNBOOKMARK_FAIL: return state.get(action.status.get('id')) === undefined ? state : state.setIn([action.status.get('id'), 'bookmarked'], true); - case REBLOG_REQUEST: - return state.setIn([action.status.get('id'), 'reblogged'], true); - case REBLOG_FAIL: - return state.get(action.status.get('id')) === undefined ? state : state.setIn([action.status.get('id'), 'reblogged'], false); - case UNREBLOG_REQUEST: - return state.setIn([action.status.get('id'), 'reblogged'], false); - case UNREBLOG_FAIL: - return state.get(action.status.get('id')) === undefined ? state : state.setIn([action.status.get('id'), 'reblogged'], true); case STATUS_MUTE_SUCCESS: return state.setIn([action.id, 'muted'], true); case STATUS_UNMUTE_SUCCESS: @@ -128,6 +121,15 @@ export default function statuses(state = initialState, action) { case STATUS_TRANSLATE_UNDO: return statusTranslateUndo(state, action.id); default: - return state; + if(reblog.pending.match(action)) + return state.setIn([action.meta.params.statusId, 'reblogged'], true); + else if(reblog.rejected.match(action)) + return state.get(action.meta.params.statusId) === undefined ? state : state.setIn([action.meta.params.statusId, 'reblogged'], false); + else if(unreblog.pending.match(action)) + return state.setIn([action.meta.params.statusId, 'reblogged'], false); + else if(unreblog.rejected.match(action)) + return state.get(action.meta.params.statusId) === undefined ? state : state.setIn([action.meta.params.statusId, 'reblogged'], true); + else + return state; } } diff --git a/app/javascript/mastodon/store/typed_functions.ts b/app/javascript/mastodon/store/typed_functions.ts index b66d7545c5..4b07a55610 100644 --- a/app/javascript/mastodon/store/typed_functions.ts +++ b/app/javascript/mastodon/store/typed_functions.ts @@ -2,6 +2,8 @@ import { createAsyncThunk } from '@reduxjs/toolkit'; // eslint-disable-next-line @typescript-eslint/no-restricted-imports import { useDispatch, useSelector } from 'react-redux'; +import type { BaseThunkAPI } from '@reduxjs/toolkit/dist/createAsyncThunk'; + import type { AppDispatch, RootState } from './store'; export const useAppDispatch = useDispatch.withTypes(); @@ -13,8 +15,192 @@ export interface AsyncThunkRejectValue { error?: unknown; } +interface AppMeta { + skipLoading?: boolean; +} + export const createAppAsyncThunk = createAsyncThunk.withTypes<{ state: RootState; dispatch: AppDispatch; rejectValue: AsyncThunkRejectValue; }>(); + +type AppThunkApi = Pick< + BaseThunkAPI< + RootState, + unknown, + AppDispatch, + AsyncThunkRejectValue, + AppMeta, + AppMeta + >, + 'getState' | 'dispatch' +>; + +interface AppThunkOptions { + skipLoading?: boolean; +} + +const createBaseAsyncThunk = createAsyncThunk.withTypes<{ + state: RootState; + dispatch: AppDispatch; + rejectValue: AsyncThunkRejectValue; + fulfilledMeta: AppMeta; + rejectedMeta: AppMeta; +}>(); + +export function createThunk( + name: string, + creator: (arg: Arg, api: AppThunkApi) => Returned | Promise, + options: AppThunkOptions = {}, +) { + return createBaseAsyncThunk( + name, + async ( + arg: Arg, + { getState, dispatch, fulfillWithValue, rejectWithValue }, + ) => { + try { + const result = await creator(arg, { dispatch, getState }); + + return fulfillWithValue(result, { + skipLoading: options.skipLoading, + }); + } catch (error) { + return rejectWithValue({ error }, { skipLoading: true }); + } + }, + { + getPendingMeta() { + if (options.skipLoading) return { skipLoading: true }; + return {}; + }, + }, + ); +} + +const discardLoadDataInPayload = Symbol('discardLoadDataInPayload'); +type DiscardLoadData = typeof discardLoadDataInPayload; + +type OnData = ( + data: LoadDataResult, + api: AppThunkApi & { + discardLoadData: DiscardLoadData; + }, +) => ReturnedData | DiscardLoadData | Promise; + +// Overload when there is no `onData` method, the payload is the `onData` result +export function createDataLoadingThunk< + LoadDataResult, + Args extends readonly unknown[], +>( + name: string, + loadData: (...args: Args) => Promise, + thunkOptions?: AppThunkOptions, +): ReturnType>; + +// Overload when the `onData` method returns discardLoadDataInPayload, then the payload is empty +export function createDataLoadingThunk< + LoadDataResult, + Args extends readonly unknown[], +>( + name: string, + loadData: (...args: Args) => Promise, + onDataOrThunkOptions?: + | AppThunkOptions + | OnData, + thunkOptions?: AppThunkOptions, +): ReturnType>; + +// Overload when the `onData` method returns nothing, then the mayload is the `onData` result +export function createDataLoadingThunk< + LoadDataResult, + Args extends readonly unknown[], +>( + name: string, + loadData: (...args: Args) => Promise, + onDataOrThunkOptions?: AppThunkOptions | OnData, + thunkOptions?: AppThunkOptions, +): ReturnType>; + +// Overload when there is an `onData` method returning something +export function createDataLoadingThunk< + LoadDataResult, + Args extends readonly unknown[], + Returned, +>( + name: string, + loadData: (...args: Args) => Promise, + onDataOrThunkOptions?: AppThunkOptions | OnData, + thunkOptions?: AppThunkOptions, +): ReturnType>; + +/** + * This function creates a Redux Thunk that handles loading data asynchronously (usually from the API), dispatching `pending`, `fullfilled` and `rejected` actions. + * + * You can run a callback on the `onData` results to either dispatch side effects or modify the payload. + * + * It is a wrapper around RTK's [`createAsyncThunk`](https://redux-toolkit.js.org/api/createAsyncThunk) + * @param name Prefix for the actions types + * @param loadData Function that loads the data. It's arguments will become the thunk's arguments + * @param onDataOrThunkOptions + * Callback called on the results from `loadData`. + * + * First argument will be the return from `loadData`. + * + * Second argument is an object with: `dispatch`, `getState` and `discardLoadData`. + * It can return: + * - `undefined` (or no explicit return), meaning that the `onData` results will be the payload + * - `discardLoadData` to discard the `onData` results and return an empty payload + * - anything else, which will be the payload + * + * You can also omit this parameter and pass `thunkOptions` directly + * @param maybeThunkOptions + * Additional Mastodon specific options for the thunk. Currently supports: + * - `skipLoading` to avoid showing the loading bar when the request is in progress + * @returns The created thunk + */ +export function createDataLoadingThunk< + LoadDataResult, + Args extends readonly unknown[], + Returned, +>( + name: string, + loadData: (...args: Args) => Promise, + onDataOrThunkOptions?: AppThunkOptions | OnData, + maybeThunkOptions?: AppThunkOptions, +) { + let onData: OnData | undefined; + let thunkOptions: AppThunkOptions | undefined; + + if (typeof onDataOrThunkOptions === 'function') onData = onDataOrThunkOptions; + else if (typeof onDataOrThunkOptions === 'object') + thunkOptions = onDataOrThunkOptions; + + if (maybeThunkOptions) { + thunkOptions = maybeThunkOptions; + } + + return createThunk( + name, + async (arg, { getState, dispatch }) => { + const data = await loadData(...arg); + + if (!onData) return data as Returned; + + const result = await onData(data, { + dispatch, + getState, + discardLoadData: discardLoadDataInPayload, + }); + + // if there is no return in `onData`, we return the `onData` result + if (typeof result === 'undefined') return data as Returned; + // the user explicitely asked to discard the payload + else if (result === discardLoadDataInPayload) + return undefined as Returned; + else return result; + }, + thunkOptions, + ); +}