Repository navigation
fix(query-persist-client-core): handle nullish persisted query state - #11919
VGontier-cmd wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: c33c2a3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 24 packages
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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe persister now identifies entries with missing or null state as expired or busted. Tests cover their removal during retrieval, garbage collection, and restoration, including restoration of a subsequent valid query. ChangesMalformed persisted entry handling
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to No actionable issue introduced by this change remains; it is mergeable after normal checks. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| function isExpiredOrBusted( | ||
| persistedQuery: | ||
| | { buster?: string; state?: Partial<QueryState> | null } | ||
| | null | ||
| | undefined, | ||
| ) { | ||
| if (persistedQuery?.state?.dataUpdatedAt) { |
There was a problem hiding this comment.
Digging into this, this function isExpiredOrBusted isn't the earliest that we can check persistedQuery'. We also don't runisExpiredOrBusted on removeQueries at all.
We could instead keep PersistedQuery type signature in isExpiredOrBusted to keep the function being bloated with that responsibility and instead have an type guard function:
function isPersistedQuery(value: unknown): value is PersistedQuery {
return (
typeof value === 'object' &&
value !== null &&
typeof (value as PersistedQuery).state === 'object' &&
(value as PersistedQuery).state !== null
)
}which gets called at each deserialize call and treated the same as a parse failure (remove and continue).
We should also add a test for the removeQueries case.
This branch has not been deployed
2 participants
🎯 Changes
Fixes #11918.
Treat null persisted values and missing/null state as entries without
dataUpdatedAt, sorestoreQueries()andpersisterGc()remove them and continue instead of throwing.The internal expiry check accepts this input shape; public types are unchanged. Tests cover removing the malformed entry, processing subsequent entries, preserving valid
nullquery data, and the existingretrieveQuery()behavior.Verified locally:
pnpm nx run @tanstack/query-persist-client-core:test:lib— 65 tests passed.pnpm run build:all— 25 packages built.pnpm run test:pr— all 128 tasks passed.✅ Checklist
pnpm run test:pr, or these tests do not apply to this pull request.🚀 Release Impact
Summary by CodeRabbit