Uh oh!
There was an error while loading. Please reload this page.
feat(shared): Support for keepPreviousData in useSubscription.rq - #7203
Conversation
🦋 Changeset detectedLatest commit: 85a1cb5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 22 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 |
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds internal KeepPreviousDataFn and wiring so keepPreviousData affects React Query placeholderData in useSubscription and usePagesOrInfinite; adjusts pagination options passing and adds tests validating keepPreviousData true/false behavior. Also adds a changelog entry. Changes
Sequence Diagram(s)sequenceDiagram
participant Consumer as Hook Consumer
participant Hook as useSubscription / usePagesOrInfinite
participant RQ as ReactQuery
Note over Hook,RQ: New internal helper KeepPreviousDataFn exists\nand is used as placeholderData when enabled
Consumer->>Hook: mount / trigger refetch (params.keepPreviousData = true|false)
Hook->>RQ: request data (config includes placeholderData = KeepPreviousDataFn or undefined)
alt keepPreviousData = true (queries enabled)
RQ-->>Consumer: returns previous data immediately (isFetching=true)
RQ->>Hook: final data arrives later (isFetching=false/isLoading=false)
Hook-->>Consumer: updates with new data
else keepPreviousData = false
RQ-->>Consumer: placeholder is undefined (isLoading=true)
RQ->>Hook: final data arrives later
Hook-->>Consumer: updates with new data
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
Comment |
# Conflicts: # packages/clerk-js/src/ui/contexts/components/Plans.tsx # packages/shared/src/react/hooks/useSubscription.rq.tsx
keepPreviousData in useSubscription.rqThere was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx (1)
14-19: Duplicate helper function identified.This
KeepPreviousDataFnis identical to the one inuseSubscription.rq.tsx. See the review comment onuseSubscription.rq.tsx(lines 17-22) for the refactoring suggestion to extract this to a shared utility.
🧹 Nitpick comments (1)
packages/shared/src/react/hooks/useSubscription.rq.tsx (1)
17-22: Consider extractingKeepPreviousDataFnto a shared utility.This function is duplicated identically in
usePagesOrInfinite.rq.tsx(lines 14-19). Extract it to a shared location to follow DRY principles and simplify maintenance.Consider creating a shared utility file:
// packages/shared/src/react/utils/reactQueryHelpers.ts/** * @internal */exportfunctionKeepPreviousDataFn<Data>(previousData: Data): Data{returnpreviousData;}Then import it in both files:
+import { KeepPreviousDataFn } from '../utils/reactQueryHelpers';+-/**- * @internal- */-function KeepPreviousDataFn<Data>(previousData: Data): Data {- return previousData;-}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
.changeset/soft-beers-sit.md(1 hunks)packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx(2 hunks)packages/shared/src/react/hooks/createBillingPaginatedHook.tsx(1 hunks)packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx(2 hunks)packages/shared/src/react/hooks/useSubscription.rq.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/shared/src/react/hooks/createBillingPaginatedHook.tsx (1)
packages/shared/src/react/types.ts (1)
PaginatedHookConfig(89-102)
packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx (4)
packages/shared/src/react/hooks/__tests__/mocks/clerk.ts (1)
createMockOrganization(61-63)packages/shared/src/react/hooks/useSubscription.rq.tsx (1)
useSubscription(30-84)packages/shared/src/react/hooks/useSubscription.swr.tsx (1)
useSubscription(22-71)packages/shared/src/utils/createDeferredPromise.ts (1)
createDeferredPromise(11-19)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (8)
packages/shared/src/react/hooks/useSubscription.rq.tsx (2)
46-46: LGTM!The extraction of
keepPreviousDatawith a sensible default is clean and consistent with the codebase patterns.
70-70: LGTM!The conditional logic correctly applies
placeholderDataonly when bothkeepPreviousDatais enabled and queries are active, preventing stale data retention when queries are disabled..changeset/soft-beers-sit.md (1)
1-5: LGTM!The changelog entry clearly documents the new
keepPreviousDatafeature and is correctly scoped as a patch-level change.packages/shared/src/react/hooks/createBillingPaginatedHook.tsx (1)
108-111: Verify the necessity of the type cast.The wrapping and casting to
PaginatedHookConfig<unknown>suggests a type compatibility issue. While this should work at runtime, theunknowntype parameter may be masking a type error or indicate that the generic type flow could be improved.Consider investigating whether the type parameter can be more specific or if the wrapping/spreading pattern can be simplified:
// Current approach ...({keepPreviousData: safeValues.keepPreviousData,infinite: safeValues.infinite,}asPaginatedHookConfig<unknown>),// Alternative if types align properlykeepPreviousData: safeValues.keepPreviousData,infinite: safeValues.infinite,If the cast is necessary due to type system limitations, consider adding a comment explaining why.
packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx (1)
75-75: LGTM!The conditional
placeholderDataconfiguration correctly preserves previous data during pagination whenkeepPreviousDatais enabled.packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx (3)
4-4: LGTM!The import of
createDeferredPromiseenables precise control over async timing in the new tests.
149-177: LGTM! Excellent test coverage.This test thoroughly validates the
keepPreviousData=truebehavior:
- Verifies previous data is retained during refetch (
isFetching=true,isLoading=false)- Confirms data updates after the deferred promise resolves
- Validates the correct number of fetcher invocations
The use of
createDeferredPromiseto control async timing is a solid testing pattern.
179-209: LGTM! Comprehensive test coverage.This test effectively validates the
keepPreviousData=falsebehavior:
- Confirms data is cleared during refetch (
isFetching=true,isLoading=true,data=undefined)- Verifies data populates after resolution
- Validates correct fetcher call count
The test properly exercises the distinction between
isLoadingandisFetchingstates, which is important for React Query semantics.
@clerk/agent-toolkit@clerk/astro@clerk/backend@clerk/chrome-extension@clerk/clerk-js@clerk/dev-cli@clerk/elements@clerk/clerk-expo@clerk/expo-passkeys@clerk/express@clerk/fastify@clerk/localizations@clerk/nextjs@clerk/nuxt@clerk/clerk-react@clerk/react-router@clerk/remix@clerk/shared@clerk/tanstack-react-start@clerk/testing@clerk/themes@clerk/types@clerk/upgrade@clerk/vuecommit: |
Uh oh!
There was an error while loading. Please reload this page.
Description
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Tests
Documentation