wevm / wagmi

Reactive primitives for Ethereum apps
https://wagmi.sh
MIT License
6k stars 1.07k forks source link

bug: <useQuery type error> The queryFn attribute ignored in UseQueryParameters but queryFn is actually passed in #3216

Closed unicape closed 1 year ago

unicape commented 1 year ago

Is there an existing issue for this?

Package Version

2.0.0.beta.3

Current Behavior

No response

Expected Behavior

No response

Steps To Reproduce

No response

Link to Minimal Reproducible Example (StackBlitz, CodeSandbox, GitHub repo etc.)

No response

Anything else?

No response

unicape commented 1 year ago
export function useBalance<
  config extends Config = ResolvedRegister['config'],
  selectData = GetBalanceData,
>(
  parameters: UseBalanceParameters<config, selectData> = {},
): UseBalanceReturnType<selectData> {
  const { address, query = {} } = parameters

  const config = useConfig(parameters)
  const chainId = useChainId()

  const queryOptions = getBalanceQueryOptions(config, {
    ...parameters,
    chainId: parameters.chainId ?? chainId,
  })
  const enabled = Boolean(address && (query.enabled ?? true))

  return useQuery({ ...queryOptions, ...query, enabled })
}

queryOptions.queryFn exists but there is no queryFn in the parameter type of useQuery

tmm commented 1 year ago

Good catch! Think we can get away with swapping queryOptions and query everywhere. That way queryFn and queryKey won't be able to be overridden at runtime.

- return useQuery({ ...queryOptions, ...query, enabled })
+ return useQuery({ ...query, ...queryOptions, enabled })
unicape commented 1 year ago

接得好!认为我们可以摆脱交换queryOptionsquery到处。这样queryFn,就queryKey无法在运行时被覆盖。

- return useQuery({ ...queryOptions, ...query, enabled })
+ return useQuery({ ...query, ...queryOptions, enabled })

@tmm I think there may be a misunderstanding

export type UseQueryParameters<
  queryFnData = unknown,
  error = DefaultError,
  data = queryFnData,
  queryKey extends QueryKey = QueryKey,
> = Evaluate<
  ExactPartial<
    Omit<
      UseQueryOptions<queryFnData, error, data, queryKey>,
      | 'initialData'
      | 'queryFn'
      | 'queryHash'
      | 'queryKey'
      | 'queryKeyHashFn'
      | 'throwOnError'
    >
  > & {
    // Fix `initialData` type
    initialData?: UseQueryOptions<
      queryFnData,
      error,
      data,
      queryKey
    >['initialData']
  }
>

In the type UseQueryParameters, Omit filters out queryFn.

const queryOptions = getBalanceQueryOptions(config, {
    ...parameters,
    chainId: parameters.chainId ?? chainId,
  })

The return value of getBalanceQueryOptions contains queryFn

tmm commented 1 year ago

That should be okay. The options are used internally and we remove queryFn from UseQueryParameters so it's not exposed to users of the hooks.

If you could provide an example of where this is an issue, it would be helpful.

unicape commented 1 year ago

那应该没问题。这些选项在内部使用,我们将其删除queryFnUseQueryParameters因此不会暴露给钩子的用户。

如果您能提供一个示例来说明问题所在,将会很有帮助。

I think this is wrong. It defeats the original intention of using typescript. And I encountered problems when developing the vue version of wagmi.

Because the value or attribute of queryKey in vue-query needs to be responsive data, I can only do this as follows

const { queryFn } = getBalanceQueryOptions(config)
const queryKey = computed(() => {
  const options = deepUnref({
    ...parameters,
    chainId,
  })
  return getBalanceQueryKey<config>(options)
})

const enabled = computed(() => {
  return Boolean(unref(address) && (query.enabled ?? true))
})

return useQuery({
  queryKey,
  queryFn,  // type error
  ...query,
  enabled
})

Do you have any better suggestions?

tmm commented 1 year ago

Interesting. This type error doesn't exist in packages/react/hooks/* and we do the same thing. Can you share a minimal reproduction using a TypeScript Playground?

unicape commented 1 year ago

Interesting. This type error doesn't exist in packages/react/hooks/* and we do the same thing. Can you share a minimal reproduction using a TypeScript Playground?

const { queryFn, queryKey } = getBalanceQueryOptions(config)

return useQuery({
  queryKey,
  queryFn,  // type error
  ...query,
  enabled
})

This is all you need to reproduce the problem

tmm commented 1 year ago

Got it. I see what you are saying now. Fixed https://github.com/wagmi-dev/wagmi/commit/c3766d0f70435919afb5ed11d94b6ae7ee751563#diff-7ec05795d8ce1410bed58ca1c1399d5a45406c7b3a2d9079915208fc5c070b04

github-actions[bot] commented 10 months ago

This issue has been locked since it has been closed for more than 14 days.

If you found a concrete bug or regression related to it, please open a new bug report with a reproduction against the latest wagmi version. If you have any other comments you can create a new discussion.