-
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
Support startTransition
with refetch
and fetchMore
+ deprecate suspensePolicy
#10809
Conversation
…from QuerySubscription
🦋 Changeset detectedLatest commit: 9884348 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 |
// around with the internals of the observable, there should always be a | ||
// concast after subscribing. | ||
invariant( | ||
observable['concast'], |
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 no longer need access to the concast because we now resolve a custom promise on the first emit of data from the observer. This has the same previously where we'd resolve the promise on the first chunk of data for deferred queries.
@@ -117,15 +104,21 @@ export class QuerySubscription<TData = any> { | |||
} | |||
|
|||
refetch(variables: OperationVariables | undefined) { |
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.
In a followup PR, I'd like to update the functionality of refetch
a bit to allow for deferred queries to be incrementally rendered, as if it was kicked off the first time. The promise returned from this function should remain the same and only resolve when all data has been sent by the server, by the component should re-render with incremental data. For now, this case is not something considered in this PR.
@@ -2435,72 +2436,6 @@ describe('useSuspenseQuery', () => { | |||
]); | |||
}); | |||
|
|||
it.each<SuspenseQueryHookFetchPolicy>([ |
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.
With suspensePolicy
now gone, we can remove all of the tests that tested against it 🎉 . I've added additional tests for startTransition
that mimics this behavior.
There are still a few refactors I'd like to do at some point, but this PR is holding up enough work that I think this can be done in a follow-up PR. |
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.
Being able to use startTransition
instead of SuspensePolicy
🎉🎉🎉
Excited to integrate these changes with useBackgroundQuery
too!
Fixes #10676
When using
refetch
orfetchMore
withstartTransition
, the hook now behaves correctly. Because of this,suspensePolicy
has been deprecated in favor ofstartTransition
. You can now prevent the suspense boundary from being shown by wrapping the call torefetch
/fetchMore
instartTransition
Checklist: