chore(shared): Add stable keys registry for billing and paginated hooks #7248
Conversation
🦋 Changeset detectedLatest commit: 42530a0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types 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.
|
WalkthroughA new centralized stable-keys module ( Changes
Sequence Diagram(s)No sequence diagram provided — changes centralize constants and adjust typings without altering runtime control flow. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
panteliselef
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/shared/src/react/stable-keys.ts (1)
15-28: Consider consistent naming for billing keys.The billing-related keys have inconsistent prefixing:
PLANS_KEY = 'plans'(no prefix)SUBSCRIPTION_KEY = 'commerce-subscription'(commerce prefix)PAYMENT_METHODS_KEY = 'commerce-payment-methods'(commerce prefix)PAYMENT_ATTEMPTS_KEY = 'billing-payment-attempts'(billing prefix)STATEMENTS_KEY = 'billing-statements'(billing prefix)For consistency, consider either:
- Adding the
billing-prefix to PLANS_KEY →'billing-plans'(matches the old value and aligns with PAYMENT_ATTEMPTS_KEY, STATEMENTS_KEY)- Or removing prefixes from all billing keys for brevity
This would improve predictability and reduce confusion about naming conventions.
📜 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 (12)
.changeset/flat-grapes-visit.md(1 hunks)packages/shared/src/react/hooks/createCacheKeys.ts(1 hunks)packages/shared/src/react/hooks/useAPIKeys.ts(2 hunks)packages/shared/src/react/hooks/useOrganization.tsx(5 hunks)packages/shared/src/react/hooks/useOrganizationList.tsx(4 hunks)packages/shared/src/react/hooks/usePaymentAttempts.tsx(1 hunks)packages/shared/src/react/hooks/usePaymentMethods.tsx(1 hunks)packages/shared/src/react/hooks/usePlans.tsx(1 hunks)packages/shared/src/react/hooks/useStatements.tsx(1 hunks)packages/shared/src/react/hooks/useSubscription.rq.tsx(2 hunks)packages/shared/src/react/hooks/useSubscription.swr.tsx(2 hunks)packages/shared/src/react/stable-keys.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (10)
packages/shared/src/react/hooks/useOrganization.tsx (1)
packages/shared/src/react/stable-keys.ts (1)
STABLE_KEYS(30-51)
packages/shared/src/react/hooks/createCacheKeys.ts (1)
packages/shared/src/react/stable-keys.ts (1)
ResourceCacheStableKey(53-53)
packages/shared/src/react/hooks/usePlans.tsx (4)
packages/shared/src/react/hooks/index.ts (1)
usePlans(15-15)packages/clerk-js/src/ui/contexts/components/Plans.tsx (1)
usePlans(80-91)packages/shared/src/react/hooks/createBillingPaginatedHook.tsx (1)
createBillingPaginatedHook(73-152)packages/shared/src/react/stable-keys.ts (1)
STABLE_KEYS(30-51)
packages/shared/src/react/hooks/useStatements.tsx (2)
packages/shared/src/react/hooks/createBillingPaginatedHook.tsx (1)
createBillingPaginatedHook(73-152)packages/shared/src/react/stable-keys.ts (1)
STABLE_KEYS(30-51)
packages/shared/src/react/hooks/useOrganizationList.tsx (1)
packages/shared/src/react/stable-keys.ts (1)
STABLE_KEYS(30-51)
packages/shared/src/react/hooks/usePaymentAttempts.tsx (5)
packages/shared/src/react/hooks/index.ts (1)
usePaymentAttempts(13-13)packages/clerk-js/src/ui/contexts/components/Plans.tsx (1)
usePaymentAttempts(56-59)packages/shared/src/react/hooks/createBillingPaginatedHook.tsx (1)
createBillingPaginatedHook(73-152)packages/shared/src/types/billing.ts (1)
GetPaymentAttemptsParams(425-425)packages/shared/src/react/stable-keys.ts (1)
STABLE_KEYS(30-51)
packages/shared/src/react/hooks/useAPIKeys.ts (1)
packages/shared/src/react/stable-keys.ts (1)
STABLE_KEYS(30-51)
packages/shared/src/react/hooks/useSubscription.rq.tsx (1)
packages/shared/src/react/stable-keys.ts (1)
STABLE_KEYS(30-51)
packages/shared/src/react/hooks/usePaymentMethods.tsx (2)
packages/shared/src/react/hooks/createBillingPaginatedHook.tsx (1)
createBillingPaginatedHook(73-152)packages/shared/src/react/stable-keys.ts (1)
STABLE_KEYS(30-51)
packages/shared/src/react/hooks/useSubscription.swr.tsx (1)
packages/shared/src/react/stable-keys.ts (1)
STABLE_KEYS(30-51)
⏰ 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). (6)
- GitHub Check: pr-title-lint
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (12)
.changeset/flat-grapes-visit.md (1)
1-2: Reminder: Complete the changeset description.The changeset file is currently empty. Before merging, add a description of the changes and specify the affected packages and version bump type (patch/minor/major).
packages/shared/src/react/hooks/useSubscription.swr.tsx (1)
12-12: LGTM! Clean refactor to use centralized stable key.The replacement of the hardcoded string literal with
STABLE_KEYS.SUBSCRIPTION_KEYimproves maintainability and reduces the risk of typos across the codebase.Also applies to: 44-44
packages/shared/src/react/hooks/useStatements.tsx (1)
3-3: LGTM! Consistent use of centralized stable key.The refactor aligns with the broader PR pattern of replacing string literals with constants from the
STABLE_KEYSregistry.Also applies to: 11-11
packages/shared/src/react/hooks/useAPIKeys.ts (1)
6-6: LGTM! Proper use of the centralized stable key registry.Replacing the hardcoded
'apiKeys'string withSTABLE_KEYS.API_KEYS_KEYmaintains consistency with the rest of the PR's refactoring effort.Also applies to: 108-108
packages/shared/src/react/hooks/usePaymentMethods.tsx (1)
3-3: LGTM! Clean refactor for billing hook.The change from
'commerce-payment-methods'toSTABLE_KEYS.PAYMENT_METHODS_KEYis consistent with the centralized stable key approach across all billing hooks.Also applies to: 11-11
packages/shared/src/react/hooks/useOrganization.tsx (1)
20-20: LGTM! Comprehensive refactor of all organization-related keys.All four cache key replacements correctly reference the centralized
STABLE_KEYSconstants:
DOMAINS_KEYMEMBERSHIP_REQUESTS_KEYMEMBERSHIPS_KEYINVITATIONS_KEYThis ensures consistency across the organization context.
Also applies to: 372-372, 394-394, 416-416, 438-438
packages/shared/src/react/hooks/useOrganizationList.tsx (1)
14-14: LGTM! Complete refactor of organization list keys.All three user-related cache keys now reference the centralized constants:
USER_MEMBERSHIPS_KEYUSER_INVITATIONS_KEYUSER_SUGGESTIONS_KEYThis maintains consistency with the organization hooks refactor.
Also applies to: 322-322, 345-345, 367-367
packages/shared/src/react/hooks/useSubscription.rq.tsx (1)
13-13: LGTM! React Query implementation aligned with SWR.Both the React Query and SWR implementations of
useSubscriptionnow reference the sameSTABLE_KEYS.SUBSCRIPTION_KEY, ensuring consistency across different data-fetching strategies.Also applies to: 29-29
packages/shared/src/react/hooks/usePaymentAttempts.tsx (1)
3-3: LGTM! Clean refactor to centralized stable keys.The cache key value remains unchanged (
'billing-payment-attempts'), making this a safe refactor that improves maintainability and type safety.Also applies to: 11-11
packages/shared/src/react/stable-keys.ts (1)
30-53: LGTM! Type-safe stable keys implementation.The const assertion and type definition properly ensure type safety for cache keys across the codebase.
packages/shared/src/react/hooks/createCacheKeys.ts (1)
1-1: LGTM! Excellent type safety improvement.Replacing the generic
StableKey extends stringwith the concreteResourceCacheStableKeytype ensures that only values fromSTABLE_KEYScan be used as cache key prefixes. This prevents arbitrary strings and provides better autocomplete and compile-time validation.Also applies to: 6-15
packages/shared/src/react/hooks/usePlans.tsx (1)
11-11: No cache key change—review comment is incorrect.The original review comment assumed a cache key change from
'billing-plans'to'plans', but verification shows:
PLANS_KEYhas always been'plans'since its creation in stable-keys.ts (commit 95a6efe)- No reference to
'billing-plans'exists anywhere in the codebaseusePlans.tsxcurrently and historically usesSTABLE_KEYS.PLANS_KEY- No uncommitted changes to this file (git diff is empty)
The line shown is the current stable state, not a modification. There is no breaking change to address.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts (1)
47-51: Consider supplementing with tests that use actual stable keys from the registry.While the type assertion approach is reasonable for testing the generic behavior of
usePagesOrInfinite, the test suite would benefit from also including test cases that use actualSTABLE_KEYSconstants. This would:
- Validate that the integration with the new stable keys registry works correctly in practice
- Catch potential issues if the registry structure or type definition changes
- Align the tests more closely with how the hook is used in production code (where other files use
STABLE_KEYSconstants instead of arbitrary strings)Consider adding a few test cases like:
import { STABLE_KEYS } from '../../stable-keys'; // Example test case using actual stable key it('works with actual stable keys from registry', async () => { const fetcher = vi.fn(async (p: any) => ({ data: [{ id: 'item-1' }], total_count: 1, })); const params = { initialPage: 1, pageSize: 10 }; const config = buildConfig(params); // Use actual stable key without casting const keys = createCacheKeys({ stablePrefix: STABLE_KEYS.ORGANIZATIONS, // or another appropriate key authenticated: true, tracked: {}, untracked: { args: params }, }); const { result } = renderUsePagesOrInfinite({ fetcher, config, keys }); await waitFor(() => expect(result.current.isLoading).toBe(false)); expect(result.current.data).toHaveLength(1); });
📜 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)
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts(2 hunks)packages/shared/src/react/hooks/createBillingPaginatedHook.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts (1)
packages/shared/src/react/stable-keys.ts (1)
ResourceCacheStableKey(53-53)
packages/shared/src/react/hooks/createBillingPaginatedHook.tsx (1)
packages/shared/src/react/stable-keys.ts (1)
ResourceCacheStableKey(53-53)
⏰ 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: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts (1)
5-5: LGTM!The import is correctly added to support the new
ResourceCacheStableKeytype.
@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/vue
commit: |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx (1)
37-37: Consider using a valid stable key or creating a test constant.The type assertions
'dummy' as ResourceCacheStableKeybypass the type system's constraint thatresourceTypeshould be a valid key from theSTABLE_KEYSregistry. While acceptable for test code, this undermines the type safety introduced by the centralized keys registry.Consider one of these approaches:
- Use an actual stable key from
STABLE_KEYSif appropriate for testing- Export a dedicated test constant (e.g.,
TEST_RESOURCE_KEY) fromstable-keys.ts- Add a comment explaining why the type assertion is acceptable in this test context
Apply this diff to add clarifying comments:
const useDummyAuth = createBillingPaginatedHook<DummyResource, DummyParams>({ hookName: 'useDummyAuth', + // Note: 'dummy' is used for testing purposes only and is not a real stable key resourceType: 'dummy' as ResourceCacheStableKey, useFetcher: useFetcherMock, }); const useDummyUnauth = createBillingPaginatedHook<DummyResource, DummyParams>({ hookName: 'useDummyUnauth', + // Note: 'dummy' is used for testing purposes only and is not a real stable key resourceType: 'dummy' as ResourceCacheStableKey, useFetcher: useFetcherMock, options: { unauthenticated: true }, });Also applies to: 43-43
📜 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)
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx(2 hunks)packages/shared/src/react/stable-keys.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/shared/src/react/stable-keys.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx (2)
packages/shared/src/react/stable-keys.ts (1)
ResourceCacheStableKey(53-53)packages/shared/src/react/hooks/createBillingPaginatedHook.tsx (1)
createBillingPaginatedHook(74-153)
⏰ 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: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx (1)
5-5: LGTM!The import is necessary for the type casting applied to the test hooks.
Description
Moves all keys into a single file, for easier discovery and maintenance.
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit