Types no longer inferred correctly

[jamesots-tq]

In version 3.24.0 and earlier, I can call useQuery like this, without needing to specify the generic types:

export function useGetThing(
  id: string | undefined,
  options?: UseQueryOptions<Thing | undefined>
): QueryObserverResult<Thing | undefined> {
  return useQuery(
    ['key', text],
    () => (id === undefined ? undefined : api.getThing(id)),
    options
  );
}

I often do this so that I can have a call to useGetThing(id, {enabled: Boolean(id)}), and have it only call the api when the id is not undefined - usually as it's the result of a previous query. I can't call api.getThing with an undefined id, so I have the check in the query function and have it return undefined when the query is not enabled. (Of course, this doesn't get called when the query is disabled, but it keeps TypeScript happy).

However, since 3.24.1, I have to include the type in the useQuery call: useQuery<Thing | undefined>(...), otherwise it complains that 'Thing | undefined is not assignable to Thing'.

(I imagine there may be better ways to write my useGetThing function, but nevertheless, this still appears to be a regression).

[TkDodo]

hm, weird. it boils down to options. If those are removed, it works fine. I did some stuff around there so it seems to be a regression, sorry for that.

I imagine there may be better ways to write my useGetThing function

In the meantime: what I'm usually doing is rejecting the promise instead of including undefined as a valid return value. I guess since you disable the query, it's a this should never happen scenario anyways, so this should work:

export function useGetThing(
  id: string | undefined,
  options?: UseQueryOptions<Thing | undefined>
): QueryObserverResult<Thing | undefined> {
  return useQuery(
    ['key', text],
    () => (id === undefined ? Promise.reject(new Error("no id provided")) : api.getThing(id)),
    options
  );
}

[TkDodo]

I think because we've made options more strict now, if you pass options in, but then use the (key, fn, options) syntax, you'll get the error now, and that is correct. options allow all options, so you could also pass in queryKey or even queryFn to your custom hook. But those fields don't do anything if you use that syntax, so we've removed them. If you omit them as well, it will work:

options?: Omit<UseQueryOptions<Thing | undefined>, "queryKey" | "queryFn">

it will also make it impossible for users of useGetThing to pass in an option that doesn't do anything, which is generally a good thing.

alternatively, you can use the object syntax:

export function useGetThing(
  id: string | undefined,
  options?: UseQueryOptions<Thing | undefined>
) {
  return useQuery<Thing | undefined>({
    queryKey: ["key", id],
    queryFn: () => (id === undefined ? undefined : api.getThing(id))),
    ...options
  });
}

and then you can decide if you spread options first or last, and depending on that, queryKey / queryFn will do something or not. But I think it's generally not so nice if a function allows for parameters that then have no effect at all...

[jamesots-tq]

I hadn't thought of returning a rejected promise — that works really nicely. I actually spotted a couple of places where someone had got the types wrong before, but the new types pick it up, so I'm happy with this now :)

[dave-at-season]

is the object syntax preferred to use going forward over the key/fn/options syntax?

It's pretty hard to tell why all my custom hooks aren't compiling after updating without finding this thread. And omitting queryKey and queryFn everywhere seems like it might cause some confusion for future maintainers without this context

[TkDodo]

is the object syntax preferred to use going forward over the key/fn/options syntax?

no, it's not.

The problem has always been there, but it has become more apparent since we're using the QueryKey generic internally on the options. This was introduced with #2622 and has nothing to do with Omit, and the Omit won't help you here.

The Problem

QueryOptions has 4 generics, and if you only provide one of them, the other 3 are falling back to the default generic:

options?: UseQueryOptions<Thing>

is actually:

options?: UseQueryOptions<Thing, unknown, Thing, QueryKey>

This will a) make the select option not work, because the 3rd generic is the result of select, so with that, you can't use it. And b) it makes your queryKey be of type QueryKey, which is string|unknown[].

If you now try to take this type and put it onto a query that has a hard-coded key of:

queryKey: ["key", id],

then what TS infers for this key is not assignable to QueryKey and you'll get an error.

Solution

If you have a hardcoded query key, you have to let the options know that:

options?: UseQueryOptions<Thing, unknown, Thing, Array<string>>

However, now you can still pass a different queryKey at runtime:

useThing("1", { queryKey: ["is", "this", "what", "you", "want"] })

very likely, you don't want consumers of your custom hook to pass in a query key. That is where the Omit comes in - to forbid it. You don't have to do that. But Omitting the queryKey won't make the Generic go away, so you still have to provide it.

---

All in all, it is quite hard to make an abstraction over useQuery that allows all options to be passed in that works for all cases. You'd likely have to mimic what useQuery does internally. To make select work, you need at least one more generic flowing through, because select can transform your data into anything. It is usually better to limit the amount of options that users can pass in to avoid those problems.

To emphasize: options?: UseQueryOptions<Thing> was always technically incorrect, and it just worked coincidentally because the QueryKey wasn't needed before internally. It is used now so those errors show up.