-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
fix(react): allow using enabled
when using useQuery().promise
#8501
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
base: main
Are you sure you want to change the base?
Changes from all commits
3b569b4
7bf0f38
e01497a
1455669
2ad266e
b7cf7fc
b222b14
a7994c2
f1daa83
a269d27
3445626
7669e03
46b7e06
2f69183
5e9f958
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -78,9 +78,7 @@ export function useBaseQuery< | |
useClearResetErrorBoundary(errorResetBoundary) | ||
|
||
// this needs to be invoked before creating the Observer because that can create a cache entry | ||
const isNewCacheEntry = !client | ||
.getQueryCache() | ||
.get(defaultedOptions.queryHash) | ||
const cacheEntry = client.getQueryCache().get(defaultedOptions.queryHash) | ||
|
||
const [observer] = React.useState( | ||
() => | ||
|
@@ -152,7 +150,16 @@ export function useBaseQuery< | |
!isServer && | ||
willFetch(result, isRestoring) | ||
) { | ||
const promise = isNewCacheEntry | ||
// This fetching in the render should likely be done as part of the getOptimisticResult() considering https://github.com/TanStack/query/issues/8507 | ||
const state = cacheEntry?.state | ||
|
||
const shouldFetch = | ||
!state || | ||
(state.data === undefined && | ||
state.status === 'pending' && | ||
state.fetchStatus === 'idle') | ||
Comment on lines
+156
to
+160
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would really want this to only trigger if there is no cache entry. As soon as you have a cache entry, no matter what state, the “thing” becomes observable by other components, making doing something “in render” not allowed by the rules of react. I guess this was done to catch transitions from |
||
|
||
const promise = shouldFetch | ||
? // Fetch immediately on render in order to ensure `.promise` is resolved even if the component is unmounted | ||
fetchOptimistic(defaultedOptions, observer, errorResetBoundary) | ||
: // subscribe to the "cache promise" so that we can finalize the currentThenable once data comes in | ||
|
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.
Maybe it should just have a rejected promise when
enabled
isfalse
instead?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.
agree that’s how it should be 👍