-
-
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
feat(react-query): allow useQuery and useQueries to unsubscribe from the query cache with an option #8348
base: main
Are you sure you want to change the base?
Conversation
…the query cache with an option
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 550b504. 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 2 targetsSent with 💌 from NxCloud. |
@hirbod could you help me re-vamp the React Native docs page if we ship this? |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8348 +/- ##
===========================================
+ Coverage 46.12% 84.21% +38.08%
===========================================
Files 200 26 -174
Lines 7502 361 -7141
Branches 1715 102 -1613
===========================================
- Hits 3460 304 -3156
+ Misses 3667 48 -3619
+ Partials 375 9 -366
|
@TkDodo Sure. How should we proceed? Should we keep what we have, replace it, or additionally add the new solution? I love that plugin |
Yeah there are some mentions about devtools on the top of the page, as well as on the devtools page: https://tanstack.com/query/v5/docs/framework/react/devtools I think what I would want is:
Regarding the sections, I think we should do:
The new thing should cover what you can do with subscribe, maybe show the standard example with react-navigation, maybe show a more advanced example with the react-native stack (e.g. keep the top two screen subscribed) I don’t think we need these anymore:
if we can cover everything with the |
Yes, so far I can't think of a scenario that wouldn't work with the new subscribed prop. |
There’s a new prop: |
so:
where |
Well, basically, If we want to keep both the second and the topmost Untested, something like this: const isTopScreen = useNavigationState((state) => {
const currentRouteKey = route.key;
const topRouteKey = state.routes[state.routes.length - 1].key;
return currentRouteKey === topRouteKey;
});
const isSecondScreen = useNavigationState((state) => {
if (state.routes.length > 1) {
const secondRouteKey = state.routes[state.routes.length - 2].key;
return route.key === secondRouteKey;
}
return false;
}); IMO, it's overkill. Since focus should ideally trigger a refetchOnWindowFocus, a disabled subscription would be re-enabled the moment useIsFocused changes again, which I believe is sufficient. |
Great to see that the experience with focused screens in React Native is improving. Would an imperative function do the job instead? Currently, I’m doing something like this, but I’m encountering issues with the isStale() function retuning false while it's actualy stale. export const useEnabledOnFocus = (enabled = true): ((query: Query) => boolean) => {
const isFocused = useRef(false);
const query = useRef<Query<unknown, Error, unknown, QueryKey>>();
useFocusEffect(
React.useCallback(() => {
isFocused.current = true;
const task = InteractionManager.runAfterInteractions(() => {
if (
enabled &&
((query.current?.state.status === "pending" &&
query.current?.state.fetchStatus === "idle") ||
query.current?.isStale())
) {
query.current?.fetch();
}
});
return () => {
isFocused.current = false;
task.cancel();
};
}, [])
);
return (_query: Query<unknown, Error, unknown, QueryKey>) => {
query.current = _query;
const computedEnabled = enabled && isFocused.current;
return computedEnabled;
};
}; Usage: useQuery({
...,
enabled: useEnabledOnFocus()
}) |
FYI, runAfterInteractions does not fire in native-stack, only in the JS stack. And since the native-stack is the recommended way, re-renders are not a concern, as the navigation runs on the UI thread. |
Ok, thanks for the explanation ! Can't wait to try the new api |
@hirbod did you have some time to try out the preview build? |
@TkDodo will do this weekend. The week was very tough with our release and hotfixes :D |
@flodlc There’s one with nearly 100% feature parity and React Navigation adapters (also works with Expo Router): https://github.com/okwasniewski/react-native-bottom-tabs. Either way, bottom tabs simply switch screens without animation, so there’s no such thing as jank with them. @TkDodo finally have so time, pulling now and testing |
@TkDodo Also, check Discord, but FYI: I gave it a thorough test—it works perfectly fine. LGTM! |
Actually I was thinking about the react navigation implementation that allows "shift" animation. |
No description provided.