Proposal: separate the data fetching from the hooks

[bpierre]

Hi! 👋

At the moment, most hooks in the library return a structured object first, then one or multiple functions similar to this one:

https://github.com/tmm/wagmi/blob/0649fa5f0cafefd86f921ad81a5b0c2222a2ba67/packages/react/src/hooks/accounts/useBalance.ts#L53-L112

These functions do two different things:

• Fetch data and return it.
• Update the hook state.

I think this approach could be improved by moving the fetching part into a separate function, which could then either be exported by the library or kept private.

Proposal Summary

We would end up with two distinct functions:

• A utility function that can be imported directly, completely independent and side effect free, that can be imported from the library (e.g. accountBalance()).
• A hook refresh function, that could even be part of the structured object (e.g. by adding a refetch function to the object like React Query does).

The API of accountBalance would look like this:

type Balance = {
  decimals: number
  formatted: string
  symbol: string
  value: BigNumber
}

type AccountBalanceFn = (

  // provider becomes a required parameter
  // since we don’t have any context.
  provider: BaseProvider,

  // so does addressOrName
  addressOrName: string,

  // The rest of the config stays unchanged apart from
  // removing the hook-specific properties.
  config?: {
    formatUnits?: Unit | number
    token?: string
  }
) => Promise<Balance>

The refetch function would be straightforward in comparison:

type RefetchFn = () => void

Usage

Using the utility function could look like this:

import { accountBalance } from 'wagmi'
import { useQuery } from 'react-query'
import { provider } from './my-provider'

function Balance() {
  const [{ error, data, refetch }] = useQuery(
    'balance_wagmi.eth',
    accountBalance(provider, 'wagmi.eth')
  )
  return (
    <div>
      <p>{error?.message ?? data ?? 'loading…'}</p>
      <button onClick={refetch}>update</button>
    </div>
  )
}

Of course, using the hook would be the preferred way, this is how it would look:

import { useBalance } from 'wagmi'

function Balance() {
  const [{ error, data, refetch }] = useBalance({ addressOrName: 'wagmi.eth' })
  return (
    <div>
      <p>{error?.message ?? data ?? 'loading…'}</p>
      <button onClick={refetch}>update</button>
    </div>
  )
}

The utility function could also be used in other contexts than React (e.g. Node, Vue, etc.):

import { accountBalance } from 'wagmi'
import { provider } from './my-provider'

async function main() {
  console.log(
    await accountBalance(provider, 'wagmi.eth')
  )
}

Pros

It would bring the following advantages:

1. Prevent erroneous states

At the moment users can fetch data using the hook or a function returned by this hook, which seem side-effect free. It might seem convenient since users can temporarily override the config defined in the hook itself, except that the function will also override the state of the hook, even if their configs differ.

function Balances() {
  const [{ error, data }, getBalance] = useBalance({ addressOrName: 'wagmi.eth' })

  const getWagmi2Balance = async () => {
    const balance = await getBalance({ addressOrName: 'wagmi2.eth' })

    // At this point, the `data` and `error` returned
    // by the hook above would be overrided by getBalance(),
    // even though it contains the result for wagmi2.eth and not wagmi.eth.

    if (balance.error) {
      alert(`Error! ${balance.error.message}`)
    } else {
      alert(balance)
    }
  }

  return (
    <div>
      <p>wagmi.eth balance: {error?.message ?? data ?? 'loading…'}</p>
      <button onClick={getBalance}>update</button>
      <button onClick={getWagmi2Balance}>show balance of wagmi2.eth</button>
    </div>
  )
}

Edit: here is an illustration of the issue: https://codesandbox.io/s/falling-butterfly-rruwp?file=/src/App.tsx

2. Use promises to their full extent

React renders don’t know how to deal with thrown errors, so it currently always resolves, even in case of error:

https://github.com/tmm/wagmi/blob/0649fa5f0cafefd86f921ad81a5b0c2222a2ba67/packages/react/src/hooks/accounts/useBalance.ts#L105-L109

The problem of this approach is that it doesn’t allow to use the promise properly: try / catch wouldn’t work, neither would plugging it to other tools like React Query. Having a separate utility would allow to return the data directly, and to return errors by throwing them.

3. Provide one way to do things for users

These changes would encourage users to use the hook by providing the options and using the returned result. As pointed in the first point, using a different config for getBalance() is not recommended as it will result in the hook returning an erroneous state, so users should always be able to call refetch() and use data to get the value instead.

4. Improve separation of concerns, provide more flexibility

The hook implementation would be more focused on the state itself, since the actual data fetching would be moved in accountBalance(). Managing async state in hooks can be tricky, and separating the data fetching implementation could help making it more clear.

accountBalance() would make be easier to maintain side-effect free once removed from the hook context, and if exported by the library, would provide flexible ways to use the library, at the cost of a more “advanced” API (since more parameters will be required to use these functions by providing them the context they need, like the provider).

A lot of what would remain in useBalance() would be easy to move to a generic hook internally, which could end up in creating hooks simple hooks like useBalance() in the following way:

type UseBalanceConfig = {
  addressOrName?: string
  formatUnits?: Unit | number
  token?: string
}

export const useBalance = createWagmiHook<UseBalanceConfig>(
  ({ config, provider }) => accountBalance(provider, config.account, { config })
)

createWagmiHook() would take care of everything else.

5. Tests

Utility functions like accountBalance() would be easy to test, while createWagmiHook() tests could extensively focus on testing the way it manages state changes.

Cons

Less flexibility when using the hook

It would remove the possibility to quickly use other parameters when using the hook. I would argue that using getBalance() with another config should be discouraged anyway, as it results in an erroneous state for the main hook. Using the utility function will be a possibility at the price of verbosity.

Something that could mitigate this issue would be to allow multiple entries to be returned by the library hooks, when possible:

import { useBalance } from 'wagmi'

const accounts = ['wagmi.eth', 'wagmi2.eth']

function Balance() {
  // here `data` would be an array, since an array of accounts was provided
  const { data, loading, error } = useBalance(
    accounts.map(account => ({ addressOrName: account })
  )
  return (
    <div>
      {error?.message
         ?? data?.map((balance, i) => (
           <p>{accounts[i]} balance: {balance}</p>
         ))
         ?? 'Loading…'}
    </div>
  )
}

Breaking changes in the API

It’s always bad to break things, but I would consider the functions returned by the hooks as part of the “advanced API” anyway.

Related

• Deterministic, finite states & caching #85: this proposal also takes inspiration from React Query data structure.
• Prevent state to be set on unmounted or outdated render contexts #135: this PR shows how subtle problems might appear from attempting to do these two things (refetching hook data + independent data fetching function) from a single function.

[bpierre]

I made a demo to illustrate the erroneous states issue: https://codesandbox.io/s/falling-butterfly-rruwp?file=/src/App.tsx

[tmm]

Started on this! #197