coinbase / build-onchain-apps

Accelerate your onchain creativity with the Build Onchain Apps Template. ⛵️
https://buildonchainapps.xyz
MIT License
712 stars 119 forks source link

Feature Request: Use react-query and/or wagmi instead of custom fetch hooks for datafetching #356

Closed Yuripetusko closed 8 months ago

Yuripetusko commented 9 months ago

Describe the solution you'd like

react-query is already a dependency and it is also part of wagmi hooks. It handles caching very well, you can pass expiration, cache time to each hook and also use custom storage if needed.

I noticed 3 hooks that use custom solutions where I beleive react-query or wagmi would work better

Describe alternatives you've considered.

useCollectionMetadata should use react query to use all the benefits of react-query caching and have consistent api with other hooks. Both useEnsName and ensAvatar can use wagmi's hooks directly. Alternatively a custom react-query hook similar to:

/**
 * A hook to get ENS avatar
 * @param ensAddressOrName - either EVM address or an ens name like bitfalls.eth
 * @param chainId - defaults to 1 for ethereum as generally ENS is there only
 * @param skip - used to conditionally skip the hook call
 */
export const useGetEnsIdentity = ({
  ensAddressOrName,
  chainId = ENS_CHAIN_ID,
  skip = false,
}: {
  ensAddressOrName?: string;
  chainId?: number;
  skip?: boolean;
}) => {
  return useQuery<string | null>({
    queryKey: ['useGetEnsAvatar', ensAddressOrName, chainId],
    queryFn: async () => {
      const ensName =
        ensAddressOrName && getIsValidEthereumAddress(ensAddressOrName)
          ? await getEnsName(wagmiConfig, { address: ensAddressOrName, chainId })
          : ensAddressOrName;
      const ensResolver = ensName
        ? await getEnsResolver(wagmiConfig, { chainId, name: ensName })
        : null;

      const publicClient = getPublicClient(wagmiConfig, {
        chainId,
      });

      if (!ensResolver || !ensName) {
        return null;
      }

      const avatar = await publicClient.getEnsAvatar({ name: normalize(ensName) });

      return avatar;
    },
    gcTime: CACHE_TIME,
    enabled: !skip,
    refetchOnWindowFocus: false,
  });
};
Yuripetusko commented 9 months ago

Let me know if there was no specific reason to use custom hook for that, then I can create a PR with a change in https://github.com/coinbase/onchainkit

Zizzamia commented 9 months ago

Really appreciate this question, and it's a conversation I am open to keep debate on.

A few thoughts:

I noticed 3 hooks that use custom solutions where I beleive react-query or wagmi would work better

Initially, I thought using wagmi would be the best approach. However, I discovered that it introduces some edge cases. For instance, including mainnet alongside L2s just to support certain Hooks compromises the clean separation of concerns and chain access that I prefer. Moreover, with OnchainKit, we plan to extend several Hooks to cater to a broader range of Identity needs. Therefore, forking and refining is an early decision based on our opinions.

At this stage of OnchainKit's development, it may not be entirely clear how it differs from Wagmi. Our objective is not to replace Wagmi but to supplement it with utilities that enhance or complement its offerings in some way.

Some may suggest, "Just add that to Wagmi." However, considering the importance of maintaining a clean separation of concerns, I believe in having more libraries that specialize in specific functionalities and excel at them. Just as Viem splitting from Wagmi was a wise move, our approach involves exploring unique ways to contribute to the ecosystem.

Apologies for this long thread, but I wanted to ensure you were fully informed, as your understanding is important to me. And I would love more of these convo going forward.

@Yuripetusko thoughts?

Yuripetusko commented 9 months ago

react-query to me seems a canon that is not need it for some of the utility we need for OnchainKit. I agree folks can use it on their own decision, but OnchainKit goal is to stay the most lean as possible and perfect a few use cases.

I would usually agree, pulling an extra big dependency seems overkill, but react-query now became basically an industry standard in writing any kind of data fetching hooks, be it graphql, http/fetch or anything else really. Moreover this promotes a common coding convention, for example the return props (isLoading, isPending, isError, etc), and it has very elaborate caching system built in. Basically there's no point trying to build something custom, because you will end up just writing the same code that react-query already wrote and half of the react industry is using. I think it would make sense to go as dependency-less as possible if this part of onchain kit was more like viem, so framework agnostic, but when it comes to react, I think react-query makes the most sense.

I agree though on the part where maiinet need to be added to wagmi config just for ens support, which is not great, but I would still use react-query here simillary to my code snippet above, just to give devs a familiar api

Some may suggest, "Just add that to Wagmi." However, considering the importance of maintaining a clean separation of concerns, I believe in having more libraries that specialize in specific functionalities and excel at them. Just as Viem splitting from Wagmi was a wise move, our approach involves exploring unique ways to contribute to the ecosystem.

I agree on this. And I would actually suggest you to start separating onchainkit early before the codebase grows more using monorepo approach, into plain js (ts) package and similar to viem, and then react implementation/hooks of the core functions. Because react might lose relevance or to not exclude some great devs that are trying to use svelte/vue. I am attempting something like this with our repo https://github.com/rmrk-team/rmrk-js, still very early though, I was mainly focusing on project structure for now, but the idea is similar

Btw, regarding separation of concerns and viem/wagmi, I saw this interesting mention in latest viem docs under miscelleneous section, a third party library that extends vagmi's public client. I found the idea quite interesting https://www.reversemirage.com/

Zizzamia commented 9 months ago

You make a lot of great points around @tanstack/react-query.

Let me sleep on this for a couple of days and decided how to move forward.