typeDefinition of updater function in setQueryData

[TkDodo]

Hi,

First of all: thanks for this 😎 library. I'm having a quick typescript question:

setQueryData takes an updater in the form of:

((oldData: T | undefined) => T)

The question is: if oldData is possibly undefined, how would be we able to construct T?

suppose we have a simple type:

type Person = {
    firstName: string,
    lastName: string
}

and we mutate that persons firstName, which we want to optimistically update (I've taken the optimistic update example as a base):

useMutation(
    firstName => axios.patch('/api/person', { firstName }),
    {
      onMutate: firstName => {
        queryCache.cancelQueries('person')

        const previousValue = queryCache.getQueryData('person')

        queryCache.setQueryData('person', old => ({
          ...old,
          firstName
        }))

        return previousValue
      }
  )

but if oldData is possibly undefined, the resulting object can never be of type Person - it might miss the lastName.

We are currently working around this by doing:

const previousValue = queryCache.getQueryData('person')

if (previousValue) {
    queryCache.setQueryData('person', {
        ...previousValue,
        firstName
    })
}

The question is: can old really be undefined ? If so, wouldn't this be a runtime error in the example here when accessing old.items ? Or is this maybe just an issue with the type definitions and old can actually never be undefined?

Thanks for clarifying

[sekhavati]

@TkDodo I had a similar issue occurring with setQueryData. Once I qualified the input parameter to the updater function the compiler was happy. In my case I was dealing with an array of users rather than a single user, ie:

type User = {
  firstName: string;
  lastName: string;
  age: number;
};

Broken

queryCache.setQueryData(QUERY_KEYS.FETCH_USERS, (oldUsers) => [
    ...oldUsers,
    newUser,
  ]);

Fixed

queryCache.setQueryData(QUERY_KEYS.FETCH_USERS, (oldUsers: Array<User>) => [
    ...oldUsers,
    newUser,
  ]);

Hope this helps

[TkDodo]

Hi @sekhavati

Yeah, it works like this with Arrays, but note that you could also provide the call-side generic:

queryCache.setQueryData<Array<User>><(QUERY_KEYS.FETCH_USERS, oldUsers => [
    ...oldUsers,
    newUser,
  ]);

The reason this works is exactly because you have an Array. Note that in either case, oldUsers is still possibly undefined (according to the types), but since you always create a new Array (and spreading undefined is possible), you have a function of this signature, which is fine:

(oldUsers: Array<User> | undefined>) => Array<User>

However, if you are trying to do the same thing with a single user, it still won't work, as you cannot just say oldUser is of type User, because that doesn't match the type for setQueryData. So this:

type User = {
    firstName: string
    lastName: string
}

queryCache.setQueryData('key', (oldUser: User) => ({
    ...oldUser,
    lastName: 'Smith',
}))

will still result in:

Error:(43, 32) TS2345: Argument of type '(oldUser: User) => { lastName: string; firstName: string; }' is not assignable to parameter of type '{ lastName: string; firstName: string; } | ((oldData: { lastName: string; firstName: string; } | undefined) => { lastName: string; firstName: string; })'.
  Type '(oldUser: User) => { lastName: string; firstName: string; }' is not assignable to type '(oldData: { lastName: string; firstName: string; } | undefined) => { lastName: string; firstName: string; }'.
    Types of parameters 'oldUser' and 'oldData' are incompatible.
      Type '{ lastName: string; firstName: string; } | undefined' is not assignable to type 'User'.
        Type 'undefined' is not assignable to type 'User'.

which is still the exact same issue

[sekhavati]

@TkDodo I see the same error as you now after enabling strict mode regardless of whether it's a single or array based example. I'm in the middle of migrating a JS project to TS and didn't have it on before.

I also agree that queryCache.setQueryData<User[]>(...) seems preferable and allows more type safety.

[tannerlinsley]

Status on this?

[TkDodo]

Hi Tanner,

The main question remains: Can the previousValue actually be undefined at runtime? I was unable to get that from looking at the source code.

if yes, that means that the types are correct, but that the example can fail here when accessing old.items and old is undefined, and then we have a hard time constructing a T when we get T | undefined in that function. It will be possible with arrays, but with objects, it's very hard (see my previous comment)

if no, then all we would need to do is "fix" the type definitions of setQueryData, which would be easy to do (happy to provide a PR if that would be correct).

Thanks for you help :)

[pokorson]

The main question remains: Can the previousValue actually be undefined at runtime? I was unable to get that from looking at the source code.

It could be undefined as you can provide not existing queryKey, right?

[TkDodo]

The main question remains: Can the previousValue actually be undefined at runtime? I was unable to get that from looking at the source code.

It could be undefined as you can provide not existing queryKey, right?

Okay, I didn't think of that. I don't think there is an easy way to make that type-safe (as in: it will be T if the key is valid and undefined otherwise, because we don't know what a valid key is)

The question is if that is a use-case we want to cover, and if it is worth to do undefined checks for the case that you mis-type a queryKey. Personally, I'd rather have a real runtime error if something like that happens 🤷 .

Let me re-phrase the question: Given a valid queryKey, can the previousValue be undefined? Say, on the first run, when you don't have a previousValue yet .. ?

[pokorson]

Let me re-phrase the question: Given a valid queryKey, can the previousValue be undefined? Say, on the first run, when you don't have a previousValue yet .. ?

This should be possible as well. If you don't set the initial value the default one will be undefined. If you will try reading query before queryFn completes you will get undefined. This might be also true when there's a request error or { manual: true }? To me the result type is incorrect as previousValue: undefined should just return undefined

[TkDodo]

To me the result type is incorrect as previousValue: undefined should just return undefined

ah right, so instead of:

((oldData: T | undefined) => T)

we should have:

((oldData: T | undefined) => T | undefined)

?

[pokorson]

yes, it makes more sense I think

[tannerlinsley]

🎉 This issue has been resolved in version 1.5.6 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[TkDodo]

@tannerlinsley I think I have to resurrect this issue for the v2 version:

https://github.com/tannerlinsley/react-query/blob/e200c99496b7ebea84da5a0df38242d80554a844/src/core/queryCache.ts#L491

The type of updater seems to have been reverted to:

Updater<TResult | undefined, TResult>

still, I think it should be:

Updater<TResult | undefined, TResult | undefined>

because I cannot produce TResult from TResult | undefined if I have an object

I can create a new issue if you want, or also just make another PR?

[boschni]

Currently the setQueryData expects data to be set, as it also sets the query to a success state. What would you expect to happen when you return undefined?

[TkDodo]

Well I would like it to stay undefined then? The issue is that you cannot, on type level, produce T out of undefined in all cases, especially not with objects.

to re-iterate from my example above:

type User = {
    firstName: string
    lastName: string
}

queryCache.setQueryData('key', oldUser => ({
    ...oldUser,
    lastName: 'Smith',
}))

this just doesn't work because oldUser is User | undefined, and I cannot summon a firstName out of thin air :)

So what I am doing at the moment is bypassing the functional updater

const oldUser = queryCache.getQueryData<User>('key') // oldUser is now `User | undefined`
if (oldUser) {
    queryCache.setQueryData('key', {
        ...oldUser,
        lastName: 'Smith',
    })
}

that works, but then what is the functional updater for? The optimistic update example is written in js, but this would actually not work with the current typings in TS.

if the functional updater allows undefined to be returned, we can do:

queryCache.setQueryData('key', oldUser => oldUser && {
    ...oldUser,
    lastName: 'Smith',
})

which means if we get undefined in, we just output undefined as well.

[boschni]

If undefined is a valid result, then you can do:

queryCache.setQueryData<User | undefined>('key', oldUser => oldUser && {
    ...oldUser,
    lastName: 'Smith',
})

[TkDodo]

right, I can include undefined in TResult. I am still a a bit confused though.

Looking at the code, updater is defined as:

https://github.com/tannerlinsley/react-query/blob/171628699cff8e6bbfa5d31bc3d1ccb11ff9e12f/src/core/query.ts#L192

but when we actually call the functional updater, the result is coerced to TResult | undefined:

https://github.com/tannerlinsley/react-query/blob/171628699cff8e6bbfa5d31bc3d1ccb11ff9e12f/src/core/query.ts#L198

but looking at the type definition of functionalUpdate shows that it returns TResult:

so why can't I return undefined from the functional updater then ?

[boschni]

It all depends on what you consider as a valid result for that query. If you expect undefined to be a valid value of data when the query status is success, then the result type for that query would be T | undefined. If you do not expect undefined to be a valid value, then you need to provide T when calling setQueryData.

[TkDodo]

I do generally not consider undefined valid for TResult. I understand that calling setData sets the query result to success, so it must get a TResult. Since the functional updater is basically fed with the value of getData, which can always be possibly undefined, this is tricky.

I think I'll just stick to the manual getData -> if not undefined -> setData(newValue) approach then. I was hoping that the functional updater would make this syntax a bit easier, but I get that it's not that easy (e.g. "bailing out" of the functional updater if undefined is returned, but undefined might be, as you correctly pointed out, a valid TResult as well, so this sounds like a no-go).

Thanks for the clarifications 👍

[gpbaculio]

@TkDodo this worked for me:

  if (previousMerchant)
          queryCache.setQueryData<MerchantType>(
            ["merchant", merchant_id],
            (old) =>
              ({
                ...old,
                is_favorite: !is_favorite,
              } as MerchantType)
          );

[TkDodo]

but I get that it's not that easy (e.g. "bailing out" of the functional updater if undefined is returned, but undefined might be, as you correctly pointed out, a valid TResult as well, so this sounds like a no-go).

1.5 years later and we've finally managed to do that in v4 🙌.. You can now return undefined from the functional updater and it will bail out and not create / update the cache entry. For that, we had to make undefined an illegal cache value for successful queries, but that's overall a good thing.

here is the PR for reference: #3271