Uh oh!
There was an error while loading. Please reload this page.
feat(shared): Lax revalidation for RQ variant hooks - #7228
Conversation
# Conflicts: # packages/shared/src/react/hooks/createBillingPaginatedHook.tsx
…ef/improve-revalidations-in-rq-variants
🦋 Changeset detectedLatest commit: 415d329 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 extensive revalidation tests across multiple hooks, introduces an Changes
Sequence DiagramsequenceDiagram
participant Caller
participant Hook as usePagesOrInfinite.revalidate()
participant QC as QueryClient
rect rgb(245,245,255)
Note over Hook: OLD (sync) - choose one variant to invalidate
Caller->>Hook: revalidate()
alt was infinite
Hook->>QC: invalidateQueries(infiniteQueryKey)
else
Hook->>QC: invalidateQueries(pagesQueryKey)
end
QC-->>Hook: done
Hook-->>Caller: done
end
rect rgb(245,255,245)
Note over Hook: NEW (async) - generalized invalidationKey → sequential invalidations
Caller->>Hook: revalidate()
Hook->>QC: await invalidateQueries(keys.invalidationKey)
QC-->>Hook: resultA
Hook->>Hook: derive stablePrefix + '-inf' + rest
Hook->>QC: await invalidateQueries(derivedInfKey)
QC-->>Hook: resultB
Hook-->>Caller: Promise(resultB)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
⏰ 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)
🔇 Additional comments (6)
Comment |
# Conflicts: # packages/shared/src/react/hooks/useAPIKeys.ts # packages/shared/src/react/hooks/usePageOrInfinite.types.ts
@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: |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx (1)
1-1: Plan revalidation tests nicely pin down cache and isolation behavior
"revalidate refetches plans and updates cache"confirms the hook uses the updatedrevalidatesemantics to refresh data and update the cached page."revalidate for user plans does not refetch organization plans"effectively asserts that invalidation is scoped by thefordimension, so org plans aren’t accidentally refetched when user plans are revalidated. TheisRQgating andcalls.every(value => value === 'user')check express this clearly.Together these tests give good confidence that user vs organization plan queries won’t interfere with each other while still benefiting from the generalized invalidation.
You could optionally also assert that at least one revalidation call has
for: 'user'even in the RQ branch (usingwaitForon both length and argument shape) to strengthen the assertion, but it’s not strictly necessary.Also applies to: 207-278
packages/shared/src/react/hooks/__tests__/useAPIKeys.spec.tsx (1)
1-211: Comprehensive revalidation coverage for useApiKeysThis new suite thoroughly exercises
useApiKeysrevalidation:
- Verifies that
revalidaterefreshes data and increments the underlyinggetAllcall count.- Confirms cascade behavior between paginated and infinite variants only in RQ mode via
__CLERK_USE_RQ__.- Checks that cascades respect configuration boundaries (different
pageSize,query, and especially differentsubjectvalues).The subject-isolation test in particular is a nice guardrail against over-broad invalidation. Overall the structure and use of
act/waitForlook solid.You might optionally tighten the “different pageSize” and “different query filters” tests by asserting that, in the RQ branch, at least one revalidation call corresponds to each configuration (e.g., checking
pageSize/queryvalues ingetAllSpy.mock.calls) to more directly verify that both related queries are refetched.
📜 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 (7)
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx(2 hunks)packages/shared/src/react/hooks/__tests__/useAPIKeys.spec.tsx(1 hunks)packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts(2 hunks)packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx(2 hunks)packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx(2 hunks)packages/shared/src/react/hooks/usePageOrInfinite.types.ts(1 hunks)packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx (1)
packages/clerk-js/src/test/utils.ts (3)
renderHook(77-77)waitFor(73-73)act(75-75)
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx (2)
packages/shared/src/types/billing.ts (1)
BillingPlanResource(120-187)packages/shared/src/react/hooks/usePlans.tsx (1)
usePlans(8-21)
packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx (2)
packages/shared/src/react/hooks/useSubscription.rq.tsx (1)
useSubscription(43-99)packages/shared/src/react/hooks/useSubscription.swr.tsx (1)
useSubscription(22-71)
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts (2)
packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx (1)
usePagesOrInfinite(21-291)packages/shared/src/react/hooks/usePagesOrInfinite.swr.tsx (1)
usePagesOrInfinite(37-237)
⏰ 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)
- GitHub Check: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (6)
packages/shared/src/react/hooks/__tests__/useSubscription.spec.tsx (1)
1-1: Revalidation test for subscriptions is well-structuredImporting
actand the new"revalidate fetches the latest subscription data"test correctly exercise the hook’srevalidatecontract (including async behavior) and mirror patterns used elsewhere in this suite. This should protect against regressions in both RQ and SWR variants.Also applies to: 217-233
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts (2)
505-533: Good coverage of basic revalidate refetch semanticsThe
"refetches current data when revalidate is invoked"test cleanly validates thatrevalidateperforms a fresh fetch and updatesdata, usingact+waitForin an async-friendly way. This aligns with the new asyncrevalidateimplementation and should remain stable across RQ/SWR implementations.
684-724: Cascade revalidation expectations are clearly specifiedThe
"cascades revalidation to related queries only in React Query mode"test nicely captures the intended behavior: in RQ mode, invalidating via the paginated hook also refetches the infinite counterpart, while non-RQ mode keeps revalidation local. The__CLERK_USE_RQ__gating plus lenient>= 2assertion make this resilient to minor fetch scheduling differences.packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx (1)
1-1: Revalidation behavior for billing hooks is well-validatedThe added
"revalidate behavior"suite does a good job of:
- Verifying that
revalidateactually refreshes paginated data.- Asserting that, in RQ mode, revalidation of the paginated hook also triggers the infinite variant while staying local in non-RQ mode.
This ties the higher-level
createBillingPaginatedHookbehavior back to the updatedusePagesOrInfiniterevalidation strategy without over-constraining call counts.Also applies to: 490-550
packages/shared/src/react/hooks/usePageOrInfinite.types.ts (1)
21-22: Invalidation key typing matches the new revalidate implementationDefining
InvalidationQueryKeyas[string, boolean, Record<string, unknown>]and wiringkeys.invalidationKeyto that shape lines up with howusePagesOrInfinite.rq.tsxnow destructures and uses the invalidation key (stable prefix, authenticated flag, tracked keys). This tighter typing should help prevent accidentally passing full query keys or mismatched structures into the hook.It’s worth confirming that all
createCacheKeyscall sites (and any customkeysobjects, if they exist) still satisfy this narrowerinvalidationKeyshape once type-checking and the test suite run.Also applies to: 28-28
packages/shared/src/react/hooks/usePagesOrInfinite.rq.tsx (1)
268-271: Async revalidate with generalized invalidation key looks correctThe new
revalidateimplementation:
- First invalidates
keys.invalidationKey, which matches the paginated query key prefix.- Then derives
[stablePrefix + '-inf', ...rest]to invalidate the corresponding infinite queries by prefix.- Returns the second invalidation promise, making
revalidateawaitable in tests and consumers.This matches the intended “cascade across pagination/infinite variants sharing the same invalidation key” behavior and is consistent with how tests assert fan-out in RQ mode.
Please ensure this aligns with the exact
invalidateQueriesprefix-matching semantics in your TanStack Query version (v5 APIs), and that no call sites relied onrevalidatebeing strictly synchronous; the tests added in this PR already cover the new async contract, but a quick type-check / test run will confirm.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/shared/src/react/hooks/__tests__/useAPIKeys.spec.tsx (3)
8-14: Consider a more specific type assertion for the mock return value.The
Record<string, unknown>type for data array items is quite loose and could mask type mismatches between the mock and actual API response.Consider defining a more specific type for the mock API key data, for example:
+type MockAPIKey = { id: string; [key: string]: unknown };+ const getAllSpy = vi.fn( async () => ({ data: [], total_count: 0, - }) as { data: Array<Record<string, unknown>>; total_count: number },+ }) as { data: Array<MockAPIKey>; total_count: number }, );
64-99: Consider extracting the RQ flag check pattern.The pattern of checking
(globalThis as any).__CLERK_USE_RQ__and conditionally asserting call counts is repeated across multiple tests (also at lines 128, 166).Consider extracting a helper function to reduce duplication:
constexpectCallsBasedOnRQMode=async(spy: any,rqCalls: number,nonRqCalls: number)=>{constisRQ=Boolean((globalThisasany).__CLERK_USE_RQ__);if(isRQ){awaitwaitFor(()=>expect(spy.mock.calls.length).toBeGreaterThanOrEqual(rqCalls));}else{awaitwaitFor(()=>expect(spy).toHaveBeenCalledTimes(nonRqCalls));}};Then use it as:
awaitexpectCallsBasedOnRQMode(getAllSpy,2,1);
177-210: Simplify the subject verification assertion.Line 209's assertion
expect(subjects[0] === undefined || subjects[0] === 'user_primary').toBe(true)is unnecessarily complex.Apply this diff to make the assertion clearer:
expect(getAllSpy).toHaveBeenCalledTimes(1); const subjects = getAllSpy.mock.calls.map(call => (call[0] as { subject?: string })?.subject); expect(subjects).not.toContain('user_secondary'); - expect(subjects[0] === undefined || subjects[0] === 'user_primary').toBe(true);+ expect(subjects[0]).toMatch(/^(user_primary)?$/);Or even more explicitly:
- expect(subjects[0] === undefined || subjects[0] === 'user_primary').toBe(true);+ expect(['user_primary', undefined]).toContain(subjects[0]);
📜 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 (2)
.changeset/young-impalas-grab.md(1 hunks)packages/shared/src/react/hooks/__tests__/useAPIKeys.spec.tsx(1 hunks)
⏰ 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)
- GitHub Check: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (4)
.changeset/young-impalas-grab.md (1)
1-14: LGTM! Clear documentation of the behavior change.The changeset clearly describes the relaxed revalidation behavior, and the code example effectively demonstrates how revalidation now works across different hook configurations.
packages/shared/src/react/hooks/__tests__/useAPIKeys.spec.tsx (3)
32-38: LGTM! Proper test isolation.The
beforeEachhook correctly clears mocks and the query client, ensuring each test starts with a clean state.
40-62: LGTM! Clear revalidation test.This test effectively verifies that calling
revalidate()fetches fresh data from the API.
1-211: Excellent test coverage for revalidation behavior.The test suite comprehensively covers:
- Basic revalidation functionality
- Cascading revalidation across paginated and infinite modes
- Configuration variations (pageSize, query filters)
- Subject isolation (negative test)
- Behavior differences based on the RQ feature flag
All async operations are properly handled with
act()andwaitFor(), and test isolation is maintained through proper cleanup.
Description
The
revalidate()function returned from the new RQ variant hooks will revalidate based on a providedinvalidationKeyand it will revalidate pagination and infinite mode hooks that match with it.This changes comes as new "sane" default as this ensures that data will be revalidated across different configuration between the host application and the AIOs.
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
Breaking Changes
Tests