isDataEqual arguments are of type unknown

[morellodev]

Describe the bug
The isDataEqual function of the useQuery configuration object is currently defined as follows:

isDataEqual: (oldData: unknown, newData: unknown) => boolean;

while I expected oldData and newData to be of type TResult:

isDataEqual: (oldData: TResult, newData: TResult) => boolean;

Maybe there is a reason for those arguments to be of type unknown, but if isDataEqual is invoked only when the data has been correctly fetched, then they should be typed as TResult.

To Reproduce
Steps to reproduce the behavior:

1. Assign a custom comparator function for isDataEqual property of the useQuery hook
2. TypeScript says oldData and newData arguments are of type unknown, forcing the user to explicitly write type assertions:

useQuery(key, fetcher, {
    isDataEqual: (oldData, newData) => customDeepEqualityChecker(
        oldData as ApiResponseType, newData as ApiResponseType
    )
  }
);

Expected behavior
isDataEqual arguments are of type TResult.

Desktop (please complete the following information):

• OS: macOS Big Sur Beta
• Browser: Chrome
• Version: 86

[TkDodo]

TypeScript says oldData and newData arguments are of type unknown, forcing the user to explicitly write type assertions:

I think you can narrow the type like this as well to avoid the cast:

useQuery(key, fetcher, {
    isDataEqual: (oldData: ApiResponseType, newData: ApiResponseType) => customDeepEqualityChecker(
        oldData, newData
    )
  }
);

but yeah, it's effectively the same...

Maybe there is a reason for those arguments to be of type unknown, but if isDataEqual is invoked only when the data has been correctly fetched, then they should be typed as TResult.

internally, I think oldData is of type TResult | undefined, because on the first run, it will be undefined, so I think the correct signature would be:

isDataEqual?: (oldData: TResult | undefined, newData: TResult) => boolean

@boschni what do you think?

[morellodev]

The first time the data is fetched there is no reason to call isDataEqual, so there is no need for oldData to be typed as TResult | undefined.

[TkDodo]

@dennismorello I think you could call queryCache.setQueryData before creating an observer, or prefetching also comes to mind.

there is no reason to call isDataEqual

We could check if both prevData and data are undefined and then omit the call, but I think it’s also fine to handle that inside of isDataEqual.

[morellodev]

@TkDodo that is not the point of the issue 🤓 Don't get me wrong, but I expect isDataEqual to be called after each successful subsequent refetch to prevent the data property returned by useQuery from changing if the new data has the same value as the last result (by some logic).

That said, I would ask @tannerlinsley if

• am I right on what isDataEqual is meant for?
• if so, does it make sense to type its arguments as TResult?

[morellodev]

For instance, SWR accepts a compare function defined as follows:

Moreover, I see from the react-query documentation that there is not a default compare function for isDataEqual (it is up to the user to opt-in this feature).

[tannerlinsley]

Seeing how this is not technically a bug, it's now a discussion.

[boschni]

Changed the type in V3 to isDataEqual?: (oldData: TResult | undefined, newData: TResult) => boolean.

But the structural sharing will already make sure a component is not re-rendered when the data did not change. The isDataEqual option is only needed when dealing with non-JSON values.

[morellodev]

I know I can simply return the data field from the fetcher function, but I prefer to keep it simple.

[TkDodo]

In that case, I would only return what you need from the fetch-function:

const { data } = useQuery('key', async () => {
  const response = await axios.get(...)
  return response.data
}

that way, only the actual data will be in the cache and you don't need a custom comparison I believe.

[boschni]

Would a selector be useful in this case? https://react-query-beta.tanstack.com/guides/migrating-to-react-query-3#selectors

[morellodev]

This is absolutely cool! Thank you 🙏🏻

[TkDodo]

I know I can simply return the data field from the fetcher function, but I prefer to keep it simple.

right. I personally don't believe that a custom comparison function is simpler, but I see your point. The fetch-function might not be something you want to temper with, e.g. if it's a service that is also used by others.

v3 also has a select option where you can subscribe to parts of the data that is in the cache, maybe that's what you are looking for?

const { data } = useQuery('key', () => axios.get(...), { select: response => response.data })