Wallet has 2 competing types of redux async middleware patterns

[petemill]

As of https://github.com/brave/brave-core/pull/10637, wallet UI is now using both the async-handled redux actions pattern, and the redux-thunk pattern. It's not clear when to use which, and this can / has lead to reduced readability and maintainability. We should pick one that meets our aims of a responsive UI and clean code / module separation, and stick with it.

As per https://github.com/brave/brave-core/pull/10637#issuecomment-960451549

I'm wondering about the logic of the split made in this PR, as it wasn't immediately understandable to me (and I'm seeing an anti-pattern attempt to creep in via subsequent PRs adding random async functions to lib.ts).

So if I'm getting this right:

• handlers.ts are functions which respond to existing dispatched actions and do things, e.g. fetch data, and dispatch further actions with the data.
• lib.ts are thunks which are themselves dispatch-able and can do things and dispatch further actions.

So first confusing things is naming - maybe lib.ts should be thunks.ts, otherwise devs will put "lib" things in here (see above example).

But the main confusion is that there are now two different philosophies in the same UI for redux async middleware: 1. our AsyncActionHandler and now 2. redux-thunk. They both do exactly the same thing but with slight difference:

• AsyncActionHandler takes a regular redux action already dispatched and performs side-effects at the same time as the regular redux reducer can handle the action (e.g. the reducer will set an 'inProgress` flag in the state, and the async handler will fetch the data).
• redux-thunk creates its own dispatch-able actions, which when called do the async calls and then dispatches further redux actions.

So now there are 2 ways for a developer to make an action that will do something async: either make an action in wallet_actions and handle it accordingly in handlers.ts or make a thunk and handle it in lib.ts. If there is a difference in when to use which then it needs to be clear and commented.

Since wallet is a large app, we should encourage 1 easy-to-follow pattern for developers in order to keep this complex and central part of the code as readable and maintainable as possible.

We started the pattern of using AsyncActionHandler across our code so that we can encourage developers to build responsiveness to their apps and show that things are happening in progress whilst still reducing boilerplate and cognitive load that redux has had complaints about.

If the intention was to clean up and have smaller modules, perhaps just splitting some of the handlers and actions by a category would do the job (e.g. keyring_{actions,handlers}, transaction_actions, account_actions, balance_actions)?

Or if the preferred pattern for this UI really is to use redux-thunk and dispatch more actions to relay start progress back to the reducer, then it's probably best to use redux-thunk instead of AsyncActionHandler.

[onyb]

We're incrementally moving the code to Redux Toolkit which should also address this issue once the migration is complete. Keeping this issue open until then by dropping the priority to P4.

[josheleonard]

lib.ts was removed in this PR: chore(wallet): remove lib context

We should still investigate the removal of the AsyncActionHandler, as it adds additional middleware to the redux store