add ability to warn when suspense queries aren't prefetched #8309
Conversation
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 0e5b86c. 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. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8309 +/- ##
===========================================
+ Coverage 45.96% 62.67% +16.71%
===========================================
Files 199 135 -64
Lines 7501 4799 -2702
Branches 1717 1348 -369
===========================================
- Hits 3448 3008 -440
+ Misses 3676 1549 -2127
+ Partials 377 242 -135
|
| * Use this if you intend to only consume prefetched queries. | ||
| * Defaults to `false`. | ||
| */ | ||
| warnOnServerFetches?: boolean |
There was a problem hiding this comment.
bike shedding: discussed with co-workers, something like strictPrefetch/strictPrefetches? Happy with whatever the consensus is
There was a problem hiding this comment.
We could also consider different levels, so for example strict might lead to a runtime exception when compared to passing a warn value to simply log it 🤔
- given @TkDodo's recent talk, this should probably also accept a callback that gets the query(?) and not just a static boolean flag
- warning log might be too specific, would it be better for this to be a function and allow the developer to control what should happen? Perhaps they want to log more/less info about the query to help track down the query that's missing a prefetch
I do like the callback approach more though, we could add an option called onPrefetchMiss or something along those lines.
This would give us more control over the outcome, e.g. could add observability, warning, throw an exception, etc.
There was a problem hiding this comment.
I do like the callback approach more though, we could add an option called
onPrefetchMiss or something along those lines.
This would give us more control over the outcome, e.g. could add observability, > warning, throw an exception, etc.
yea definetely see this useful for much more than just warning for un-prefetched queries. would love @TkDodo's thoughts on what API he prefers
|
I had as similar idea recently, but wanted to actually create some kind of |
|
Yea that's a much more thought out idea compared to this PR which I put together in 5 minutes after a meeting :) |
😂 . Would you like to give implementing the first part of it a try? Specifically, the under-prefetching part, as it overlaps with what you were trying to do here:
Bonus points if it would also work with the |
When using the promise-rsc-hydration thingy we aim to prefetch all queries in the RSCs and just consume the streamed promises in our components.
This can be a bit fragile since forgetting to prefetch may not be obvious to find. This PR adds a new option that, when enabled, will log at warning level when
useSuspenseQueryis called on a new query that's not already observed.as with all options, this can be set globally when constructing the QueryClient and overridden locally cause you may have queries
Questions
usePrefetchQuery -> useSuspenseQuerypattern in a SPA as well