Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typescript incorrectly narrows error based on isLoading #7105

Closed
lgenzelis opened this issue Mar 13, 2024 · 2 comments · Fixed by #7114
Closed

Typescript incorrectly narrows error based on isLoading #7105

lgenzelis opened this issue Mar 13, 2024 · 2 comments · Fixed by #7114

Comments

@lgenzelis
Copy link
Contributor

lgenzelis commented Mar 13, 2024

Describe the bug

If I do

const { error, isLoading } = useQuery(...);
if (isLoading) {
  ....

error is assumed to be null by typescript. That's wrong: if it's the first time the query is used, then yes, that's right. But if:

  1. query retries until we have isLoading === false and error !== null
  2. the component using the query is unmounted
  3. it gets remounted
  4. the query will be automatically retried (resulting in isLoading === true again), but error will be initialized with the previous error, so we have error !== null while also isLoading === true.

So either the types or the implementation are wrong. I'm hoping for the types being wrong, because I like this behavior 😅

In case it helps: the same implementation behavior is present with isFetching (so, you can have isFetching === true while also having error !== null), but the type narrowing doesn't kick in that case.

Your minimal, reproducible example

Ts Playground

Steps to reproduce

Since I'm hoping for this to be a bug in the types and not the implementation, I'm providing a Ts Playground as MWE. But if you folks tell me this is an implementation bug, I can create a codesandbox instead.

Expected behavior

Type narrowing match implementation

How often does this bug happen?

Every time

Screenshots or Videos

No response

Platform

Tanstack Query adapter

None

TanStack Query version

5.25.0

TypeScript version

5.3.3

Additional context

No response

@TkDodo
Copy link
Collaborator

TkDodo commented Mar 14, 2024

the query will be automatically retried (resulting in isLoading === true again), but error will be initialized with the previous error, so we have error !== null while also isLoading === true.

can you show this in a runtime sandbox, because if isLoading is true, data should be undefined and error should be null. This can be seen in the code here:

...(state.data === undefined && {
error: null,
status: 'pending',
}),

if a fetch comes in and we have no data, error state will be reset. So I don't see a runtime scenario where isLoading (derived from isPending) is true and isError is defined. If so, we have a runtime issue, not a typing issue :)

@lgenzelis
Copy link
Contributor Author

Alright, I created a sandbox mwe.

To see it:

  • open the browser console and reload the sandbox
  • wait for the error. You should see
===== rendering ====
>> error oh no
>> isLoading false
  • click on the Toggle button, which unmounts the component, and then click it again to remount it
  • You now will see the bug. The first render will show:
===== rendering ====
>> error oh no
>> isLoading true

and then it'll be almost instantly followed by a re-render where error is correctly set to null.

Btw, for context: why is this an issue? Because I'm resorting to useEffect to show an error toast if the query fails, and because of this issue the error toast is shown as soon as the component remounts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants