-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Add possibility to pass a callback to enabled. #7566
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 034bb63:
|
Definitely something that would improve tanstack query a lot on react-native. So glad this PR exists! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not against this, I just think I tried this some time ago and it wasn't that trivial.
Usually, we'd want those functions to accept the query
itself, and not just be a function without an argument. I recently did this for staleTime
and put that into a shared utils:
query/packages/query-core/src/utils.ts
Lines 91 to 101 in 96a743e
export function resolveStaleTime< | |
TQueryFnData = unknown, | |
TError = DefaultError, | |
TData = TQueryFnData, | |
TQueryKey extends QueryKey = QueryKey, | |
>( | |
staleTime: undefined | StaleTime<TQueryFnData, TError, TData, TQueryKey>, | |
query: Query<TQueryFnData, TError, TData, TQueryKey>, | |
): number | undefined { | |
return typeof staleTime === 'function' ? staleTime(query) : staleTime | |
} |
can you try to refactor this into the same direction?
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 034bb63. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
… use of a resolveEnabled util function
can you also add updates to the api docs please ? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7566 +/- ##
===========================================
+ Coverage 43.95% 62.40% +18.44%
===========================================
Files 185 125 -60
Lines 7037 4495 -2542
Branches 1540 1239 -301
===========================================
- Hits 3093 2805 -288
+ Misses 3578 1459 -2119
+ Partials 366 231 -135 |
Yes, updated docs as well now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks really good 👏
ah, just noticed:
|
Fixed! Does it look good like that? |
eslint is failing - please have a look https://cloud.nx.app/runs/O9IbYCGg8Y?utm_source=pull-request&utm_medium=comment |
Thanks for letting me know! Pushed some lint fixes now. |
…TanStack/query#7566 @ `useApi*()` * now typing `queryKey` of `UseInfiniteQueryOptions` @ `useApiWithCursor()` @ api/index.ts * fix now showing user `displayName` when its name is `null`, introduced in 62e1a5e @ `<ReplyItem>` * fix hydration node mistmatch in `<AMenuItem key="list">` when switching from query with empty param to non-empty @ pages/posts.vue @ fe
We are using tanstack query in react native, and i have tried to implement a way to refetch all active queries in the focused screen.
In react native all screens remains mounted even when navigating to a new screen, and thus its easy to accidentally re-render out of focus screens, as well as accidentally refetching out of focus queries.
notifyOnChange props solved the problem with re-renders.
This helpes to be able to use invalidateQueries in for example pull to refresh when we dont want to have to specify exactly which queries should or should not be refetched on invalidation.
As it is today, we are forced to trigger re-renders when navigation to set enabled to true or false to be able to do this, but its costly to rerender screens on navigation and especially the ones going out of focus
Related discussion and PR
#5734
@paulobmarcos @TkDodo