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

RTKQ: refetch() can't be a promise ? #1939

Closed
vadeneev opened this issue Jan 19, 2022 · 10 comments
Closed

RTKQ: refetch() can't be a promise ? #1939

vadeneev opened this issue Jan 19, 2022 · 10 comments
Milestone

Comments

@vadeneev
Copy link

Could you help me to get why RTKQ useQuery's refetch is not a promise-like thing ? (what is motivation)
I mean - I can't await it e.t.c
same for lazyQuery and it's trigger

@phryneas
Copy link
Member

The lazyQuery trigger can be awaited in 1.7.
As for refetch, I'm not sure from the back of my mind - historically that was problematic in some situations when a fetch was already running, now it should be technically possible but never came up I think.

@jpcody
Copy link

jpcody commented Feb 9, 2022

Just adding a concrete use case here: we use either refetch or a lazy trigger passed in to a pull-to-refresh component, and it'd be really nice to await the results of either of those reliably to then mark isRefreshing to false once they're done.

@phryneas
Copy link
Member

phryneas commented Feb 9, 2022

@jpcody but you already have a isFetching property on the query hook result?

@jpcody
Copy link

jpcody commented Feb 9, 2022

Yes. which mostly fits my needs, but not always.

// Component A - listing all dogs
const { data, isFetching, isLoading, refetch } = useGetDogsQuery()
<PullToRefresh onRefresh={refetch} refreshing={isFetching && !isLodading} />

// Component B - showing a single dog
const fetchDog, { isFetching, isLoading } = useLazyGetDogByIdQuery()
const refetch = useCallback(() => fetchDog(id), [id])
<PullToRefresh onRefresh={refetch} refreshing={isFetching && !isLodading} />

Component A actually behaves as expected, but component B doesn't, which leads me to want an async refetch.

In component B, the lazy query only returns isLoading on the first component load, not any subsequent loads of different components that use the same query. So in my case (in React Native), if I load a component that lazy queries for dog 1, then a new component that lazy queries for dog 2, the second component's isLoading will always return false. So on subsequent loads, isRefreshing will always evaluate as true and the refreshing state will appear instead of the loading state.

That lazy query behavior is unexpected to me, and perhaps I'm doing something wrong on my end. But given that behavior, in my pull-to-refresh component, I'd like to simply await the value I pass in onRefresh, which could be either a refetch function or a lazy query.

@markerikson
Copy link
Collaborator

lazy query only returns isLoading on the first component load

Yes, isLoading and isFetching are different:

https://redux-toolkit.js.org/rtk-query/usage/queries#frequently-used-query-hook-return-values

@jpcody
Copy link

jpcody commented Feb 9, 2022

Sorry, this is difficult to communicate about. What's unexpected to me is that when a component is re-mounted, and thus the query is re-issued with a different argument and different data is fetched, isLoading does not fire again. So after I lazily fetch 1 dog in a component in the above example, then fetching a different dog via re-mounting the same component always reports isLoading as false.

@awdyson
Copy link

awdyson commented Apr 7, 2022

If nothing else, consistency in RTKQ's explicit calls would be nice.
The submit method in there is ingested by a smart RHF form that disables controls while that promise is alive.
I ended up needing to use a ContextProvider to continue that disabled state through the life of the refetch.
image
image
image

Obviously I figured out a workaround, but hopefully this is some extra proof of value.

@phryneas
Copy link
Member

phryneas commented Apr 7, 2022

@awdyson if nothing else, a respectful tone would be nice.

As already stated, there is no philosophical reason against doing this (it was just added in other places in later versions of RTK, but not there yet) - and thus also no reason for someone not to just whip up a PR if they need this feature. It's not a lot of code, in fact I've just done it - #2212.

Please go ahead and test the CodeSandbox build. I'm getting breakfast now.

@awdyson
Copy link

awdyson commented Apr 7, 2022

@phryneas I'm really sorry, and I'm grateful for the work being done here.
I spent a while trying to figure out how to concisely phrase a use-case that would help justify this ticket, and not enough time making sure I didn't come off like a jerk.

In good faith, I tried my code against that PR's commit hash.
Looks like the type info is correct, but the promise is resolving immediately.
image

I've got the branch pulled locally to see if I can figure out where to change that, but I often feel a little out-classed by open source libraries.
Will comment in the PR thread if I can find a solution.

@markerikson
Copy link
Collaborator

Merged #2212 into the v1.9 branch - should be available there via the PR preview build.

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

No branches or pull requests

5 participants