feat(react): add tanstack layer for caching #66
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #66 +/- ##
==========================================
+ Coverage 83.62% 85.62% +1.99%
==========================================
Files 125 127 +2
Lines 10320 10738 +418
Branches 1092 1325 +233
==========================================
+ Hits 8630 9194 +564
+ Misses 1690 1544 -146 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
chakrihacker
changed the title
chakrihacker
added
enhancement
🚀 Preview deploymentBranch: 📝 Preview URL: https://auth0-universal-components-ckpg12x5w-okta.vercel.app Updated at 2026-01-29T14:29:26.630Z |
| * Time in milliseconds until inactive cached data is garbage collected. | ||
| * | ||
| * When a query has no active observers (no mounted components using it): | ||
| * - Data remains in cache for this duration |
There was a problem hiding this comment.
too many inline comments reduces code readability , can we please reduce it
| * Default cache configuration values. | ||
| * | ||
| * These defaults provide a balance between reducing API calls and keeping data fresh: | ||
| * - `staleTime: 30s` - Data is considered fresh for 30 seconds, avoiding redundant fetches |
There was a problem hiding this comment.
same here reduce inline comments
|
|
||
| const CACHE_CONFIG = { | ||
| CONFIG_STALE_TIME: 5 * 60 * 1000, | ||
| CONFIG_GC_TIME: 10 * 60 * 1000, |
There was a problem hiding this comment.
why are we defining cache config here again. ?
| const queryState = queryClient.getQueryState(configQueryKeys.details()); | ||
|
|
||
| // Only refetch if data is stale or doesn't exist | ||
| if (existingData && queryState && !queryState.isInvalidated) { |
There was a problem hiding this comment.
wont tanstack already do this via staleTime and refetch()
| retry: (failureCount, error) => { | ||
| // Don't retry on 404 errors (config not set) | ||
| if (hasApiErrorBody(error) && error.body?.status === 404) { | ||
| return false; |
There was a problem hiding this comment.
shoudl we set data to null here
There was a problem hiding this comment.
below there is a line where we decide if data is there or not and return null based on that
const idpConfig = idpConfigQuery.data ?? null;
packages/react/src/blocks/my-organization/idp-management/__tests__/sso-provider-table.test.tsx
Outdated
Show resolved
Hide resolved
| const errorMessage = | ||
| error instanceof Error | ||
| ? t('organization_changes_error_message', { message: error.message }) |
There was a problem hiding this comment.
I see we already already generating error msg in try do we need to have this again here?
There was a problem hiding this comment.
looks like this is still unresolved, to more context you see from line 49 we already have error handler with toast, but again we are creating the similar error notification with toast, can we reduce the redundant part here? I know its been part of existing code itself
Changes
Please describe both what is changing and why this is important. Include:
Added Tanstack Query for the following hooks
my-organization
domains
Testing
Please describe how this can be tested by reviewers. Be specific about anything not tested and reasons why. If this library has unit and tests should be added for new functionality and existing tests should complete without errors.
Checklist