-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
feat(bootstrap): Cache projects in IndexedDB #89139
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
Conversation
Implements the caching part of https://www.notion.so/sentry/Tech-Spec-Frontend-Bootstrap-Cache-Project-Loading-Performance-1748b10e4b5d80759127c15d3772e063 Went with IndexedDB because some organizations have multiple megabytes of projects to cache. This helps bandaid the entire app waiting for projects to load by storing them in the cache and revalidating.
❌ 919 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
static/app/appQueryClient.tsx
Outdated
} | ||
|
||
export function restoreQueryCache() { | ||
if (!isProjectsCacheEnabled) { |
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.
revert before merging
import {DEFAULT_QUERY_CLIENT_CONFIG, QueryClient} from 'sentry/utils/queryClient'; | ||
|
||
/** | ||
* Named it appQueryClient because we already have a queryClient in sentry/utils/queryClient |
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.
Can we have this comment explain the difference to try and help avoid people using this one if they don't need it?
static/app/appQueryClient.tsx
Outdated
return ( | ||
// Query is not pending or failed | ||
query.state.status === 'success' && | ||
// Query is not stale |
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.
can drop this comment I think, since it literally reads not query is stale
// Query is not pending or failed | ||
query.state.status === 'success' && | ||
!query.isStale() && | ||
Array.isArray(query.queryKey) && |
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 check is unnecessary - the queryKey
is always an Array.
// Mark queries as stale so they won't be recached | ||
appQueryClient.invalidateQueries({queryKey: ['bootstrap-projects']}); |
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 not only marks them as stale, but also refetches them if they are still active!
to avoid this, we need:
appQueryClient.invalidateQueries({
queryKey: ['bootstrap-projects'],
refetchType: 'none',
});
@@ -30,7 +25,7 @@ function Main() { | |||
const [router] = useState(buildRouter); | |||
|
|||
return ( | |||
<QueryClientProvider client={queryClient}> | |||
<QueryClientProvider client={appQueryClient}> |
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 really want to use the PersistQueryClientProvider
here if we use persistence. It will handle restoring and subscriptions to the storage, and avoids race conditions between queries that mount while we are still restoring from indexDB.
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
Implements the caching part of https://www.notion.so/sentry/Tech-Spec-Frontend-Bootstrap-Cache-Project-Loading-Performance-1748b10e4b5d80759127c15d3772e063 Went with IndexedDB because some organizations have multiple megabytes of projects to cache. This helps bandaid the entire app waiting for projects to load by storing them in the cache and revalidating.
Implements the caching part of https://www.notion.so/sentry/Tech-Spec-Frontend-Bootstrap-Cache-Project-Loading-Performance-1748b10e4b5d80759127c15d3772e063
Went with IndexedDB because some organizations have multiple megabytes of projects to cache.
This helps bandaid the entire app waiting for projects to load by storing them in the cache and revalidating.