I kinda hate hooks ... a refactoring story
HTML-код
- Опубликовано: 4 окт 2024
- I'll be the first to say, I'm not entirely sure what this video is about ... kinda about react hooks, kinda about react-query, but, I hope, mainly about finding the right place for your business logic.
My Links
shaky.sh
shaky.sh/tools
Not a hooks problem but an architectural problem.
Firstly, there's no need to merge the two concepts into one. This kind of abstraction will likely bite you as your requirements change for either. If they share the same kind of data, then just make it a single thing-which you already did as "Product". Otherwise, separate their concerns and handle them differently.
Secondly, instead of storing a local state, you could use React Query's cache mechanism as local state. This also allows you to revalidate said state automatically if needed (or be disabled). This way you don't need explicit side effects to update state (leading to footguns).
State is the main factor to affect the view, and now you say that use caches and auto-refresh to manage it instead of actively trigger SetState... and now "Side Effects" are somehow "needed" to update the state.... wow, very weird logic of hookers.
What a weird hook, that "useGetOrCreateProduct". Why are there no parameters? Why are widgets and gadgets separate things, why not just have a product with a "productType" property? This hook is doing way too much (twice as large as it should be) and your code is unintuitive.
You're not wrong. I definitely don't think that this code was meant to be a model to follow, so much as the beginning of a discussion on how to make better code. As to "why not just have a product with a productType", you have to consider that this is data that is being stored and fetched from a database with a working history. Changing how data is stored and the properties it has in the database can be complex. A lot of the time it is easier to alter how you fetch the data instead of altering the structure of the data itself.
I personally would try to remove the useEffect here as an additional cleanup step.
useEffects are generally overused in cases where they are not needed, and in this case, can be avoided by leveraging the “onSuccess” option of the mutation hook (if you extend the definition to pass through the mutation options) or just in the second argument of the mutate call function call.
Aside from this though, I was recently looking at a similar issue, more on React Query where I was finding it very verbose to try to reuse the logic for the useQuery, prefetch, and invalidation, particularly around just passing around the types, which have to be provided to all the generics of React Query.
I’ve really been loving tRPC for this reason, because you define a procedure that makes the useQuery, prefetch, and invalidation ALL type-safe and with auto-complete. I just with this layer on top of React Query existed as-is 😅
Oooh, that sounds pretty sweet! Need to make time to dig into tRPC more.
And yeah, you’re right about onSuccess. It’s way cleaner.
Beware that useQuery's onSuccess callback is getting removed in react query v5
Yeah, this is pretty messy with overloaded responsibilities. Can strongly recommend “child first design” by watching “Definitive Guide to React Component Design and the key in avoiding tech dept” by Kevin Ghadyani incl. his “React Hooks the right way” videos. He is the author of OneForm and when I first worked through them, a bunch of light bulbs went off. Strongly recommend. Since then, made them part of engineering on-boarding. Best ❤️
Thanks for the rec! Definitely gonna check that out!
People are going to hate me for this, but I pump out features for my clients by writing BIG components and then do a cleanup step after a few weeks whereby I move stuff into other files (with the same filename with a "." and then useSomethingSomething). I also almost always have a Zustand store for anything mildly complex. So my files are like "bla.tsx", "bla.store.ts", "bla.helpers.tsx", "bla.useSomethingSomething.ts", "bla.constants.ts". So yea, lots of logic before the return is fine and I clean it up when I feel like it (move it into another file as a hook). I'm also a big fan of huge component renders. I'm only using additional components / nesting / composition when it's very clear that it's needed. I like being able to scroll and seeing everything in one place. I work alone though so that might have something to do with it :)
Love that NVim theme. What is the name?
I understood what the intent of the example was, but using a local state to display the data instead of the data returned directly from useQuery and using react query hooks just to modify a local state you lose most of the functionality of the library
Asynchronous state should only live inside react query and should not be synchronized with local state
It would make more sense to have a local state that only controls the id of the query and that the hook returns a ternary that returns the data returned from useQuery if there is one and has already been returned or, otherwise, execute the mutation that returns the data optimistically, this way you still enjoy all the reactivity of the use query hook when the data comes back from the server
```
function useGetOrCreateProduct(initialId: Widget['id'] | Gadget['id']){
const [id, setId] = useState(initialId)
const {data: widgetData} = useGetWidget(id) // it can be a hook that actually does an individual query or simply one that does a find on the return of useGetWidgets hook
const {data: gadgetData} = useGetGadget(id)
const {mutate: createWidget, data: newWidget} = usePostWidget() //add an invalidate to the mutation's onSuccess
const {mutate: createGadget, data: newGadget} = usePostGadget()
function getWidget() {
if(widgetData) return widgetData
createWidget(id)
return newWidget
}
function getGadget() {
if(gadgetData) return gadgetData
createGadget(id)
return newGadget
}
const product = isWidget(id) ? getWidget() : getGagdet()
return {
product,
getProduct: setProduct
}
```
In Svelte we can just write typescript and it works.. I don't get how people tolerate all the bad abstractions in React.
This isn’t necessarily a hook problem I just think it wasn’t well written but. I enjoy your videos
I feel a little dumb right now but I don’t get this example… 😕
Ah, sorry, that’s not what I want! Lemme know what’s confusing, happy to add details here. But also, there’s a follow up coming next week.
I would make two hooks, one for widgets and one for gadgets, they can receive either gadget/widget or undefined, so that way you can use them conditionally following react hooks rules
@Cush ?
I dont see this as a very realistic example. If you are working with a server that is generating widgets or gadgets or whatever, you would expect the server to generate them and also generate the ids. So you would never check the ids client side, its not the client responsibility. Also react-query gives you hooks already... getting too fancy with your hooks is just complicating matters and no wonder you rant about hooks if you are over using them. It's interesting to see how you CAN use react-query but for me this is just trying to get too fancy with hooks and you end up with long, complicated code that will be hard to read for a newbie. But I do like you videos! 😄
I appreciate it! Agree about the server generating the IDs … one of my main lessons from making this video is that I reduced my real-life scenario too far, and in ways that didn’t work if you don’t have the context that’s in my head.
are u using some kind of extension for google developer tools ?
I totally agree with you, hooks are the worst thing that ever happened. I hate them... so now i hate React
what color theme?
Tokyo Night
interesting, tokyo night on vim looks different,
Oh no, dude😂
😂 Man, I thought hooks were hard already, but turns out, they're even harder.
i dont thinks that hooks are meant to be used all the way down
i would use a context the return an inteface
something like IData {createOrUpdate: Widget | Gadget } => Promise
and implement it separated from react
I strongly prefer not destructuring the hook returns from both useQueries and useMutations. `postGadget.mutate()`, `postGadget.data` & `postGadget.reset()` look a lot clearer to me and it's less naming work. Easier when you're somewhere deep in the code to understand what you're reading too, rather than having lots of flat variable names.
Also, instead of using `data` from the hook, can't you just await it when calling the mutation? (I know you can in rtk-query, haven't used react-query that much).
Also, I'd just pass whatever mutation function you want into the hook instead of having to add clauses for all of the possible mutations you want. The product state is unnessecary, just return the data from the mutation hook. These are my two cents in regards to refactoring I suppose 🤷♂
I don’t think you can await in react-query, but love these other ideas! Especially passing a mutation function into the hook … gonna play with that for sure!
@@andrew-burgess yes you can. with mutateAsync :)
Love your point on destructuring, it's so much nicer to dot into the query object than to name a bunch of fields. It's kinda annoying that destructuring is what's shown in the RQ docs, and I often tell people on our team to not do it since that's what they're first introduced to.
use vue
or svelte!
What font are you using?
Mona Lisa
I recently switched from Hack to Mono Lisa!
Kind Sir, do yourself a favor, and learn Vue 3 and Nuxt 3. You'll thank me later. 🔥🤝👀
i think its not matter of framework or even language, the logic itself is bad