| Looks very nice! I see you're using Redux Toolkit and TypeScript, including some fairly heavy use of `createEntityAdapter`. Any feedback on using RTK? We're always interested in hearing how RTK is working for our users. A couple quick suggestions on the code itself: - I see a lot of custom hooks like `useAccountsPageState` [0], except that they're completely duplicating the `useSelector` signature. I think the right answer here is to start by defining the "pre-typed" `useAppSelector` hook we show in our TS usage docs [1]. Then, make sure your selectors have the correct `RootState` type. Finally, you can rewrite the hook as `const useAccountsPageState = () => useAppSelector(selectPageState)`. - It looks like you're importing `TopHatDispatch` into some other files. That has the risk of causing circular import problems, since it's importing from the store itself. You may want to rework functions like `updateSyncedCurrencies` [2] to be a thunk. (In fact, looking at the code, this looks like a very good candidate for RTK's `createAsyncThunk` API.) Also, it looks like this is going to end up dispatching a separate action for every item. It would probably be more efficient to fetch all the items first, then dispatch _one_ action with the combined fetched results. [0] https://github.com/Athenodoros/TopHat/blob/a916386edf/src/st... [1] https://redux.js.org/tutorials/typescript-quick-start#define... [2] https://github.com/Athenodoros/TopHat/blob/a916386edf/src/st... |
- createEntityAdapter was great, but one idea for an addition: I dislike having numbers as IDs (Will floating point precision bite me? Will I accidentally treat them as actual numbers? Do I need to cast them back for lookups or equality checks?), but I didn't get around to moving to strings, and preserving the sorting and ID generation with that in mind. Maybe RTK could bundle some utilities for managing this, like sorting functions and/or ascending ID generation to save others from my fate? I'll definitely start with these next time.
- I really liked the Slice API in RTK, but I felt like I wanted to run nested slices quite a lot - breaking up serialisable state (ie. user data) and page state, but then breaking down further into individual data types or pages respectively, so that pages could "own" their own state (and maybe get as far as something like a managed Mixin for a page state, like the recurring transaction table). I spent some time writing a way to compose Slices, but I found that it made things probably more complicated than it needed to be - even if you end up with a load of useAccountsPageState-style duplication which is maintained manually. (I'll admit this was a while ago, so my memory of why I decided against this is a little rusty...).
- I hadn't seen useAppDispatch and useAppSelector - I'll definitely swap over my copy of them in https://github.com/Athenodoros/TopHat/blob/main/src/state/sh... ...
- I definitely got that the expected pattern is createAsyncThunk (or useDispatch more broadly) rather than my separate functions using TopHatDispatch. I spent some time tooling about with migrating over, but I felt like it was an additional abstraction layer that I didn't really need. I eventually went with a fairly strict flow of "Types -> Storage Logic -> Store Definition -> Actions -> Components" which dealt with the circular imports. I'd be interested in other thoughts behind the best practice though - maybe they're more obvious with other people, or a larger project?