Skip to content
This repository has been archived by the owner on Aug 6, 2021. It is now read-only.

(WIP) refactor: Redux toolkit + TypeScript #551

Open
wants to merge 28 commits into
base: master
Choose a base branch
from

Conversation

firstred
Copy link
Contributor

This original plan was a simple refactor to make state management as a whole more scalable, but eventually turned into a much bigger refactor. Therefore, I have pushed a WIP to check if this PR has any chance of getting merged.

bunqDesktop is, without a doubt, a brilliant piece of engineering, but with like many projects of this size, scalability is becoming an issue. I have tried to add extra reducers to Redux but found it difficult not to knock over other functionality. "Luckily," I have seen this problem numerous times before in my own projects. Some helpful tips and toolkits from the creators of Redux and React have helped me solve the problem.

With this pull request, I am trying to add the best parts to the project.

Here is what has helped me tremendously scale React and Redux:

  • Remove all side effects from React components, including async functions and thunks. Many thunks together tend to cause a so-called "callback hell."
  • Gradually apply more TypeScript. In this PR, I have converted most of the files and added type-checking to actions.
  • Divide the Redux store into slices with the help of Redux Toolkit. Every slice has its own actions and reducers. Accessing other slices is strictly forbidden. Only side-effects (thunks/sagas) and React components are allowed to access multiple slices of the store. It's an opinionated toolkit, but in the end, it helps to keep the store lean and clean.
  • Use Redux Toolkit for immutable updates. Internally Redux Toolkit uses Immer to create reducers that do not mutate the state directly. Working with immutable drafts is a lot easier and saves many lines of code.
  • Keep all side effects in sagas and deprecate thunks. I have tried to apply batch actions in thunks and a few components, but that wasn't very effective. With sagas, you create function generators which can be halted after every async operation (so even half-way during a side-effect is possible) and makes it a lot easier to orchestrate the many async processes in this app. For instance, you can use takeLeading on pages that load initial data. In case the user revisits this page while it is still loading, it will keep waiting for the initial process to finish, and omits any new requests to load while it's still working.
  • Batch actions for a performance boost. Redux saga can natively batch actions together in a single render pass: https://github.com/manaflair/redux-batch#usage-w-extra-middlewares
    I have tried to apply @manaflair/redux-batch, but it looks like Redux nowadays has native support for batched actions, by leveraging the new (unstable) unstable_batchedUpdate API: https://react-redux.js.org/api/batch. Together with Redux saga, we can make components dispatch multiple actions at once and let sagas orchestrate all the side-effects that are subscribed to these actions.
  • Apply TypeScript aliases. Writing ~components is a lot easier. No more guessing where ../../../MyComponent actually is. In tsconfig.json, you can set the paths. The webpacktools.js file helps the Babel module resolver convert from TypeScript to Babel.
  • Move non-React files out of the react folder. I have restructured everything that did not belong in the react folder in such a way that you can now easily see what you can expect in a folder. It is just a preference, though. If you don't like it, I can easily revert it. I just thought that it would be a lot clearer like this.

Let me know what you think! It's probably a bit overwhelming, but I expect that it's a great way to increase maintainability and improve performance.

@Crecket
Copy link
Member

Crecket commented Jan 21, 2020

Everything so far seems fine with me as I read through it quickly but I'll check things more thoroughly tomorrow 👍

@firstred
Copy link
Contributor Author

firstred commented Jan 21, 2020

Redux Toolkit is complaining about non-serializable objects being added to the state. Whereas with Redux, you can usually get away with doing that, you are not supposed to do that and Redux Toolkit (properly) crashes once you dispatch non-serializable actions. To avoid future problems, I have made several objects serializable.

I have:

  • Turned new Date(), into +new Date(), which casts the date to a number, the timestamp.
  • Added the IRuleCollection interface, which makes the default properties such as id,title, etc. public members of a RuleCollection. This way you can serialize the objects before dispatching them and see with TypeScript when you're dealing with just a plain object:
    Screenshot from 2020-01-21 11-21-29
  • Payment
  • RequestResponse
  • RequestInquiry
  • RequestInquiryBatch

@firstred
Copy link
Contributor Author

Another TypeScript improvement.

  • With Redux Toolkit I am now exporting the root state type, so you can easily type-check in mapStateToProps and mapDispatchToProps, and add those returned prop types to a components prop types, like so:
interface IProps { }

class RuleCollectionChecker extends React.Component<ReturnType<typeof mapStateToProps> & ReturnType<typeof mapDispatchToProps> & IProps> {
    // ...
}

const mapStateToProps = (state: ReduxState) => ({
    someProp: state.someProps,
});

const mapDispatchToProps = (dispatch: AppDispatch) => ({
    someDispatch: () => dispatch(someAction()),
});

@firstred
Copy link
Contributor Author

firstred commented Jan 21, 2020

I am currently trying to figure out the structure of the whole Redux state. You will notice that with the latest commit everything builds, but it is having a hard time listing payments, etc. That's because with Redux Toolkit I have to manually apply serialization before objects can be dispatched. It's currently using the fromPlainObject and toPlainObject methods of models for that, but I think toReduxObject and fromReduxObject will make it more obvious that we are serializing for Redux and not e.g. the API.

@firstred
Copy link
Contributor Author

Although we could apply the same serialization as the API, I don't think that we should. I have been able to drastically reduce CPU and memory usage by applying a manual serialization strategy for the Redux store. It looks like, together with async and batch dispatching, these are going to be the biggest performance improvements we can achieve in the short term, with little effort.

@firstred
Copy link
Contributor Author

Ahh and avoiding no serialization at all, obviously, is going to be a big improvement, too 😸

This seems to be the biggest performance hog:
Screenshot from 2020-01-21 20-01-03

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement batched actions for redux App sometimes feels slow and unresponsive
2 participants