-
Notifications
You must be signed in to change notification settings - Fork 406
chore(clerk-js, shared): Use experimental paginated hooks for querying commerce data #6159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(clerk-js, shared): Use experimental paginated hooks for querying commerce data #6159
Conversation
🦋 Changeset detectedLatest commit: 27a6c17 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 Git ↗︎
|
📝 WalkthroughWalkthroughThis change introduces new experimental paginated hooks for commerce data: Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms (5)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (9)
packages/clerk-js/src/ui/components/PricingTable/PricingTable.tsx (1)
62-63: Update comment to match the new hook name.The comment says "Pre-fetch payment sources" but the code now uses
usePaymentMethods(). Consider updating the comment for consistency.- // Pre-fetch payment sources + // Pre-fetch payment methods usePaymentMethods();packages/shared/src/react/types.ts (1)
93-103: Nice JSDoc formatting improvements.The improved spacing and formatting in the JSDoc comments enhances readability.
packages/shared/src/react/hooks/usePaymentAttempts.tsx (2)
6-9: Complete the JSDoc documentation.The JSDoc comment for the type is incomplete and should provide meaningful documentation for API consumers.
/** - * @interface + * Parameters for the usePaymentAttempts hook */
11-21: Complete the JSDoc documentation.The JSDoc comment for the hook is incomplete and should describe its purpose and usage.
/** - * + * Hook for fetching paginated payment attempts data + * @experimental This hook is experimental and may change in future versions */packages/shared/src/react/hooks/useSubscriptionItems.tsx (2)
6-9: Complete the JSDoc documentation.The JSDoc comment for the type is incomplete and should provide meaningful documentation.
/** - * @interface + * Parameters for the useSubscriptionItems hook */
11-21: Complete the JSDoc documentation.The JSDoc comment for the hook is incomplete and should describe its purpose and usage.
/** - * + * Hook for fetching paginated subscription items data + * @experimental This hook is experimental and may change in future versions */packages/shared/src/react/hooks/usePaymentMethods.tsx (2)
6-9: Complete the JSDoc documentation.The JSDoc comment for the type is incomplete and should provide meaningful documentation.
/** - * @interface + * Parameters for the usePaymentMethods hook */
11-26: Complete the JSDoc documentation.The JSDoc comment for the hook is incomplete and should describe its purpose, especially the organization/user context switching behavior.
/** - * + * Hook for fetching paginated payment methods data + * Automatically switches between organization and user context based on the resource parameter + * @experimental This hook is experimental and may change in future versions */packages/shared/src/react/hooks/usePagesOrInfinite.ts (1)
14-16: Add JSDoc for the helper function.The
getDifferentKeysfunction lacks documentation explaining its purpose and behavior.-/** - * - */ +/** + * Returns an object containing keys from obj1 that are not present in obj2. + * @param obj1 - The object to extract keys from + * @param obj2 - The object whose keys will be excluded + * @returns Object with keys from obj1 not present in obj2 + */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
.changeset/smart-pandas-carry.md(1 hunks).changeset/violet-terms-fix.md(1 hunks)packages/clerk-js/src/ui/components/Checkout/CheckoutForm.tsx(2 hunks)packages/clerk-js/src/ui/components/PaymentAttempts/PaymentAttemptsList.tsx(2 hunks)packages/clerk-js/src/ui/components/PaymentSources/PaymentSources.tsx(5 hunks)packages/clerk-js/src/ui/components/PricingTable/PricingTable.tsx(2 hunks)packages/clerk-js/src/ui/components/Statements/StatementsList.tsx(2 hunks)packages/clerk-js/src/ui/contexts/components/PaymentAttempts.tsx(1 hunks)packages/clerk-js/src/ui/contexts/components/Plans.tsx(4 hunks)packages/clerk-js/src/ui/contexts/components/Statements.tsx(1 hunks)packages/shared/src/react/hooks/createCommerceHook.tsx(1 hunks)packages/shared/src/react/hooks/index.ts(1 hunks)packages/shared/src/react/hooks/usePagesOrInfinite.ts(5 hunks)packages/shared/src/react/hooks/usePaymentAttempts.tsx(1 hunks)packages/shared/src/react/hooks/usePaymentMethods.tsx(1 hunks)packages/shared/src/react/hooks/useStatements.tsx(1 hunks)packages/shared/src/react/hooks/useSubscriptionItems.tsx(1 hunks)packages/shared/src/react/types.ts(3 hunks)packages/types/src/user.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/types/src/user.ts
[error] 184-184: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
🪛 ESLint
packages/types/src/user.ts
[error] 184-184: The {} ("empty object") type allows any non-nullish value, including literals like 0 and "".
- If that's what you want, disable this lint rule with an inline comment or configure the 'allowObjectTypes' rule option.
- If you want a type meaning "any object", you probably want
objectinstead. - If you want a type meaning "any value", you probably want
unknowninstead.
(@typescript-eslint/no-empty-object-type)
⏰ Context from checks skipped due to timeout of 90000ms (24)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (nextjs, chrome, 13)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (tanstack-react-router, chrome)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (quickstart, chrome)
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Unit Tests (18, --filter=@clerk/astro --filter=@clerk/backend --filter=@clerk/express --filter=@c...
- GitHub Check: Static analysis
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (26)
packages/clerk-js/src/ui/components/PricingTable/PricingTable.tsx (1)
5-5: LGTM - Hook import updated correctly.The import change from
usePaymentSourcestousePaymentMethodsaligns with the experimental hook refactor..changeset/smart-pandas-carry.md (1)
1-6: LGTM - Changeset accurately documents the changes.The changeset properly documents the patch update for using experimental hooks from
@clerk/sharedto query commerce data..changeset/violet-terms-fix.md (1)
1-11: LGTM - Changeset properly documents the new experimental hooks.The changeset accurately documents the introduction of the experimental paginated commerce hooks with appropriate versioning (minor releases) and clear descriptions.
packages/clerk-js/src/ui/contexts/components/PaymentAttempts.tsx (1)
9-11: LGTM - Data structure changes handled correctly.The changes properly reflect the new data structure where
usePaymentAttemptsnow returns a flat array instead of a nested{data: [...]}object. Both the callback logic and dependency array have been updated consistently.packages/clerk-js/src/ui/contexts/components/Statements.tsx (1)
9-11: ```shell
#!/bin/bashLocate files containing usePagesOrInfinite and inspect their contents
FILES=$(rg -l "usePagesOrInfinite" packages/shared/src/react/hooks)
if [[ -z "$FILES" ]]; then
echo "No files found containing usePagesOrInfinite"
exit 1
fi
echo "Found files:"
echo "$FILES"
for FILE in $FILES; do
echo "=== $FILE ==="
sed -n '1,200p' "$FILE"
done</details> <details> <summary>packages/clerk-js/src/ui/components/Statements/StatementsList.tsx (3)</summary> `16-16`: **LGTM! Improved data destructuring.** The addition of `count` destructuring aligns well with the new experimental hook structure and simplifies the component logic. --- `23-23`: **Good simplification of itemCount usage.** Using `count` directly is cleaner than the previous nested property access pattern. --- `32-32`: ```shell #!/bin/bash # Inspect the useStatements hook implementation to see default data handling rg -n -C5 "export const useStatements" packages/clerk-js/src/ui/contexts/components/Plans.tsx rg -n -C5 "__experimental_useStatements" packages/clerk-js/src/ui/contexts/components/Plans.tsxpackages/clerk-js/src/ui/components/Checkout/CheckoutForm.tsx (2)
194-194: LGTM! Simplified data extraction.The direct destructuring of
dataaspaymentSourcesis cleaner than the previous pattern and aligns with the new experimental hook structure.
20-20: ```shell
#!/bin/bashRetry searching for remaining references to the old hook name across .ts and .tsx files
rg "usePaymentSources" -g '.ts' -g '.tsx'
</details> <details> <summary>packages/clerk-js/src/ui/components/PaymentAttempts/PaymentAttemptsList.tsx (3)</summary> `15-15`: **LGTM! Consistent with the new hook pattern.** Adding `count` to the destructuring follows the same pattern used in other components and simplifies the data handling. --- `22-22`: **Good use of the direct count property.** Using `count` directly instead of the nested property pattern is cleaner and more maintainable. --- `32-32`: #!/bin/bash set -e echo "=== PaymentAttemptsList.tsx (first 200 lines) ===" sed -n '1,200p' packages/clerk-js/src/ui/components/PaymentAttempts/PaymentAttemptsList.tsx echo echo "=== createCommerceHook implementation ===" HOOK_FILE=$(rg -l "export function createCommerceHook" packages/shared/src | head -n1) echo "Found at: $HOOK_FILE" sed -n '1,200p' "$HOOK_FILE" </details> <details> <summary>packages/shared/src/react/types.ts (2)</summary> `113-117`: **LGTM! Well-defined fetchOnMount option.** The addition of `fetchOnMount` with proper JSDoc documentation and sensible default provides good control over data fetching behavior. --- `137-141`: **Consistent type definition.** Good consistency in adding the `fetchOnMount` option to both config interfaces with matching documentation and defaults. </details> <details> <summary>packages/shared/src/react/hooks/useStatements.tsx (1)</summary> `1-15`: **LGTM! Clean and consistent hook implementation.** The hook follows the established pattern correctly with proper TypeScript typing and context usage. The implementation is concise and well-structured. </details> <details> <summary>packages/shared/src/react/hooks/index.ts (1)</summary> `11-14`: **LGTM! Proper experimental API exports.** The experimental hooks are correctly exported with the `__experimental_` prefix, clearly indicating their experimental status to consumers. </details> <details> <summary>packages/shared/src/react/hooks/usePaymentAttempts.tsx (1)</summary> `14-21`: **LGTM! Correct hook implementation.** The hook implementation follows the established pattern correctly with proper type parameters and context usage. </details> <details> <summary>packages/shared/src/react/hooks/useSubscriptionItems.tsx (1)</summary> `14-21`: ```shell #!/bin/bash # Search for createCommerceHook definition to inspect how resourceType is used rg -n "createCommerceHook" -A20 packages/shared/src/react/hookspackages/shared/src/react/hooks/usePaymentMethods.tsx (1)
14-26: ```shell
#!/bin/bashLocate and inspect createCommerceHook implementation to verify resourceType handling
rg -l "createCommerceHook" packages/shared/src/react/hooks | xargs -I {} sed -n '1,200p' {}
</details> <details> <summary>packages/clerk-js/src/ui/components/PaymentSources/PaymentSources.tsx (2)</summary> `1-13`: **Hook migration looks good!** The migration from `usePaymentSources` to `usePaymentMethods` aligns with the new experimental paginated hooks architecture. --- `117-122`: **Clean refactor to use the new hook structure.** The simplified data structure (from `data.paymentSources` to `paymentMethods`) and direct use of `revalidatePaymentMethods` improves code readability. </details> <details> <summary>packages/shared/src/react/hooks/createCommerceHook.tsx (1)</summary> `1-91`: **Well-structured hook factory implementation!** The generic factory pattern provides excellent type safety and consistency across commerce hooks. Good practices observed: - Proper provider validation - Telemetry integration - Clean separation of user/organization scoping - Safe parameter handling </details> <details> <summary>packages/clerk-js/src/ui/contexts/components/Plans.tsx (3)</summary> `1-9`: **Import updates align with the new hook architecture.** The experimental hook imports properly replace the previous SWR-based implementations. --- `43-72`: **Consistent and clean hook implementations.** All three hooks follow a uniform pattern with proper scoping and pagination configuration. The `fetchOnMount` parameter in `useStatements` provides flexibility for controlling initial data loading. --- `178-188`: **Revalidation logic properly updated.** The cache invalidation has been correctly migrated to use the new hooks' `revalidate` methods, maintaining consistency with the new architecture. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| pageSize: pageSizeRef.current, | ||
| }; | ||
|
|
||
| // cacheMode being `true` indicates that the cache key is defined, but the fetcher is not. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💭 This comment feels like it should be reflected in the __experimental_mode jsdoc description.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consumers of the exported hooks, do not have control over the fetcher, thus should not be mentioned
Description
This PR introduces new experimental paginated hooks for commerce data in the
@clerk/sharedpackage. The new hooks (useStatements, usePaymentAttempts, and usePaymentMethods) provide a standardized way to fetch and paginate commerce-related resources. These hooks are prefixed with__experimental_and are used internally to refactor the existing commerce data fetching logic in@clerk/clerk-js. The changes improve code organization and provide a more consistent API for handling paginated commerce data.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit