chore: Migrate Relay { cacheConfig: force true } to store-and-network fetchPolicy#11744
Conversation
olerichter00
changed the title
{ cacheConfig: force true } to "store-and-network" fetchPolicy
| } | ||
|
|
||
| export const InboxQueryRenderer: React.FC<InboxQueryRendererProps> = memo((props) => { | ||
| const data = useLazyLoadQuery<InboxQuery>( |
There was a problem hiding this comment.
Migrating the query renderer to useLazyLoadQuery to be able to prefetch and reload the data when the user opens the screen.
|
|
||
| const DEFAULT_SORT_OPTION = SORT_OPTIONS[0].value as CollectionArtworkSorts | ||
|
|
||
| export const artworkListVariables = { count: PAGE_SIZE, sort: DEFAULT_SORT_OPTION } |
There was a problem hiding this comment.
Extracting default variables for prefetching.
olerichter00
force-pushed
the
olerichter00/ONYX-1630/replace-relay-cacheconfig-with-fetchpolicy
branch
from
591c697 to
647afac
Compare
olerichter00
changed the title
{ cacheConfig: force true } to "store-and-network" fetchPolicy{ cacheConfig: force true } to store-and-network" fetchPolicy
olerichter00
changed the title
{ cacheConfig: force true } to store-and-network" fetchPolicy{ cacheConfig: force true } to store-and-network fetchPolicy
olerichter00
deleted the
olerichter00/ONYX-1630/replace-relay-cacheconfig-with-fetchpolicy
branch
| return ( | ||
| <Suspense fallback={<InboxPlaceholder />}> | ||
| <InboxQueryRenderer {...props} /> | ||
| </Suspense> |
There was a problem hiding this comment.
@olerichter00 sorry for the late drive by review but I think there is a subtle but important difference here vs renderWithPlaceholder. I believe renderWithPlaceholder will render the failure view in event of errors on the same screen we are on whereas without it the error will bubble up and potentially cause app wide unable to load making tabs inaccessible. tl;dr we need an ErrorBoundary in addition to the Suspense:
There was a problem hiding this comment.
Thanks for the late review :)
I tested the error handling of the screen (by throwing an error inside of the component), and it looks fine to me. The "Unable to Load" screen is displayed; it's possible to reload and access other tabs. Or am I missing something?
I guess, the global error AppWideErrorBoundary handles the error with displaying the tabs correctly 🤔
Simulator.Screen.Recording.-.iPhone.16.-.2025-03-24.at.15.14.59.mp4
There was a problem hiding this comment.
it is good it is showing the tabs, but really we shouldn't hit the app wide error boundary whenever possible which is what the error is saying and instead add an error boundary that is as local as possible to where the error occurred
There was a problem hiding this comment.
#11035 e.g. trying to maintain as much nav as possible possibly a simpler error message
There was a problem hiding this comment.
Yeah, that makes sense 👍 I've created a PR that adds local error boundaries with withSuspense to the Inbox and the Collection Artist screens: #11775
4 participants
This PR resolves ONYX-1630
Description
This PR replaces Relay's
cacheConfig: force: truewithfetchPolicy: "store-and-network"for screens usinguseLazyloadQuery. It also adds a few more queries to routes.tsx for prefetching.Relay's "store-and-network" fetch policy does not seem to be supported by
QueryRenderer. During testing, the component did not run a new query when the screen opened, and the query was already cached. I will further investigate and probably migrate the remaining screens that useQueryRenderertouseLazyLoadQuery.PR Checklist
To the reviewers 👀
Changelog updates
Changelog updates
Cross-platform user-facing changes
iOS user-facing changes
Android user-facing changes
Dev changes
{ cacheConfig: force true }to "store-and-network" fetchPolicy (for prefetching) - oleNeed help with something? Have a look at our docs, or get in touch with us.