Comment by acemarke | Hacker News Reader

Comment by acemarke | Hacker News Reader

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...

I definitely have a lot of opinions about this :D. The main one is that RTK was fantastic (as are the docs!), and createEntityAdapter was definitely better than the equivalent that I wrote before finding it... A few more specific thoughts though:

- 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?

Thanks for the feedback, and glad to hear it's working well overall!

There's definitely been some prior requests for some additional functionality around `createEntityAdapter`. We tried to keep the original implementation fairly scoped. (The current code is actually a port of the `@ngrx/entity` package.) There were a couple suggestions around having the ability to specify additional sorting indices of some kind. I briefly tried to play with implementing that idea [0], but wasn't sure it was the right direction and haven't gotten back to it. We don't do anything about logic for creating new items or auto-generating IDs atm. But, we are definitely interested in any specific suggestions for API improvements or additional use cases that it would be good to cover.

If this "page state" is mostly coming from the server and is primarily being used as cached data, it might actually be a good candidate for our RTK Query data fetching and caching API [1] [2]. If on the other hand you need those for a lot of other calculations in the slices, then RTKQ probably isn't the right choice.

`createAsyncThunk` specifically implements the standard "dispatch actions based on an async request lifecycle" pattern that's always been taught for Redux usage [3]. That way, you only have to provide a "payload creator" function that makes the async request and returns the data, and it dispatches the actions for you. Typically you then dispatch the thunk itself from a component, like `dispatch(fetchMyData())`.

FWIW I and the other Redux maintainers generally hang out in the `#redux` channel in the Reactiflux Discord ( https://www.reactiflux.com ), and we're always happy to chat and answer questions. Please drop by when you get a chance - I'd love to discuss some of these topics in more detail!

[0] https://github.com/reduxjs/redux-toolkit/pull/948

[1] https://redux-toolkit.js.org/rtk-query/overview

[2] https://redux.js.org/tutorials/essentials/part-7-rtk-query-b...

[3] https://redux.js.org/tutorials/fundamentals/part-7-standard-...

[4] https://redux.js.org/tutorials/fundamentals/part-8-modern-re...

Reply on news.ycombinator.com