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

Комментарии • 39

  • @dealloc
    @dealloc Год назад +14

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

    • @windresta
      @windresta Год назад

      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.

  • @mahadevovnl
    @mahadevovnl Год назад +10

    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.

    • @morganjones4281
      @morganjones4281 Год назад

      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.

  • @Fuzbo_
    @Fuzbo_ Год назад +12

    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 😅

    • @andrew-burgess
      @andrew-burgess  Год назад

      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.

    • @greidinger-reis
      @greidinger-reis Год назад +3

      Beware that useQuery's onSuccess callback is getting removed in react query v5

  • @dinoscheidt
    @dinoscheidt Год назад +3

    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 ❤️

    • @andrew-burgess
      @andrew-burgess  Год назад

      Thanks for the rec! Definitely gonna check that out!

  • @Euquila
    @Euquila Год назад +1

    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 :)

  • @gosnooky
    @gosnooky 11 месяцев назад +1

    Love that NVim theme. What is the name?

  • @tthiagolino8
    @tthiagolino8 Год назад +1

    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
    }
    ```

  • @bradyfractal6653
    @bradyfractal6653 Год назад

    In Svelte we can just write typescript and it works.. I don't get how people tolerate all the bad abstractions in React.

  • @MaixPeriyon
    @MaixPeriyon 9 месяцев назад

    This isn’t necessarily a hook problem I just think it wasn’t well written but. I enjoy your videos

  • @feldinho
    @feldinho Год назад

    I feel a little dumb right now but I don’t get this example… 😕

    • @andrew-burgess
      @andrew-burgess  Год назад

      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.

  • @greidinger-reis
    @greidinger-reis Год назад

    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

  • @adamdrake39
    @adamdrake39 Год назад +1

    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! 😄

    • @andrew-burgess
      @andrew-burgess  Год назад +1

      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.

  • @Vantivify
    @Vantivify Год назад

    are u using some kind of extension for google developer tools ?

  • @88goeth
    @88goeth 3 месяца назад

    I totally agree with you, hooks are the worst thing that ever happened. I hate them... so now i hate React

  • @chrissalgaj4111
    @chrissalgaj4111 Год назад +1

    what color theme?

  • @Yollov
    @Yollov Год назад

    Oh no, dude😂

    • @andrew-burgess
      @andrew-burgess  Год назад +1

      😂 Man, I thought hooks were hard already, but turns out, they're even harder.

  • @smatsri
    @smatsri Год назад

    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

  •  Год назад +6

    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 🤷‍♂

    • @andrew-burgess
      @andrew-burgess  Год назад +2

      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!

    • @zzej
      @zzej Год назад +7

      @@andrew-burgess yes you can. with mutateAsync :)

    • @brendonovich
      @brendonovich Год назад +2

      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.

  • @youetznab
    @youetznab Год назад

    use vue

  • @mohansingh2916
    @mohansingh2916 Год назад +1

    What font are you using?

  • @AngelHdzMultimedia
    @AngelHdzMultimedia Год назад +1

    Kind Sir, do yourself a favor, and learn Vue 3 and Nuxt 3. You'll thank me later. 🔥🤝👀

    • @ky3ow
      @ky3ow Год назад +1

      i think its not matter of framework or even language, the logic itself is bad