-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Handle changed suspense query options #11025
Handle changed suspense query options #11025
Conversation
🦋 Changeset detectedLatest commit: 5fc6f03 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
size-limit report 📦
|
currentFetchPolicy === 'standby' && | ||
currentFetchPolicy !== watchQueryOptions.fetchPolicy |
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.
This would also trigger if the user manually sets a fetchPolicy
of "standby"
and changes away from that. Is this really the intended behaviour in that case?
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.
We have a validation in the hook that ensures users can't set standby
directly:
apollo-client/src/react/hooks/useSuspenseQuery.ts
Lines 265 to 280 in e47cfd0
function validateFetchPolicy( | |
fetchPolicy: WatchQueryFetchPolicy = 'cache-first' | |
) { | |
const supportedFetchPolicies: WatchQueryFetchPolicy[] = [ | |
'cache-first', | |
'network-only', | |
'no-cache', | |
'cache-and-network', | |
]; | |
invariant( | |
supportedFetchPolicies.includes(fetchPolicy), | |
`The fetch policy \`%s\` is not supported with suspense.`, | |
fetchPolicy | |
); | |
} |
Agreed though its not super apparent looking at this directly (which means we have an implicit dependency here). But even if we let that be the case (something I've considered), I would agree this should still the same behavior. "standby" is like a "wait to fetch" fetch policy, so we'd want to execute a request as soon as the user switches away from it. In this way, standby
is like skip: true
(which is exactly how skip
is implemented!).
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.
LGTM!
…-suspense-query-options
Closes #10680
In the current release of the library, changes to some options passed to
useSuspenseQuery
anduseBackgroundQuery
were not applied to the underlyingObservableQuery
. For example, if someone wanted to change theerrorPolicy
between renders, the updatederrorPolicy
was not applied on the next fetch.This PR now adds proper behavior when changing any option on these hooks. The following options have these behaviors:
errorPolicy
- Apply to the next fetchcontext
- Apply to the next fetchcanonizeResults
- Immediately updatedata
to use canonized resultsreturnPartialData
- Apply to the next fetchrefetchWritePolicy
- Apply to the nextrefetch
fetchPolicy
- Apply to the next fetchChecklist: