feat(shared,ui): Remove virtual routing option #7466
Conversation
🦋 Changeset detectedLatest commit: 9dfb76d The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 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.
|
WalkthroughRemoves the public Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
![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)
.changeset/silent-ducks-wait.md (1)
6-6: Changeset description is clear and accurate.The description correctly explains the change and its rationale. Consider an optional enhancement to mention the new
WithInternalRouting<T>pattern for consumers reviewing the changelog, so they understand how internal routing is now handled.Optional enhancement:
-Remove `virtual` from the `routing` option. The `virtual` value is only used internally and should not be part of the public API. +Remove `virtual` from the `routing` option, which is now accessible only via the internal `WithInternalRouting<T>` wrapper. This ensures the public API surface exposes only `path` and `hash` routing strategies.
📜 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 (1)
.changeset/silent-ducks-wait.md(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). (25)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 16, RQ)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (billing, chrome, RQ)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (machine, chrome, RQ)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (quickstart, chrome, 16)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (quickstart, chrome, 15)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
.changeset/silent-ducks-wait.md (1)
1-4: Verify versioning strategy for public API removal.Removing
routing="virtual"from the public API surface—even if it was internal-only or undocumented—could be a breaking change for users who have relied on this option. Patch-level bumps typically indicate bug fixes, while removing API options usually warrants a minor (new feature) or major (breaking) version bump.Please confirm whether:
- This is a documented/used public API, or truly internal and undiscovered by users.
- The semver strategy in this project treats API removals as patch, minor, or major changes.
- Any migration path or deprecation warning was needed prior to removal.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/ui/src/customizables/parseAppearance.ts (1)
23-26: Remove redundant key exclusions.The keys
'captcha','elements','theme', and'variables'are already excluded viakeyof Themeand don't need to be listed again in the literal union. This redundancy reduces code clarity.Apply this diff to remove the redundant keys:
type PublicAppearanceTopLevelKey = Exclude< keyof Appearance, - keyof Theme | 'captcha' | 'cssLayerName' | 'elements' | 'layout' | 'theme' | 'variables' + keyof Theme | 'cssLayerName' | 'layout' >;
📜 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)
packages/shared/src/types/clerk.ts(1 hunks)packages/ui/src/components/OrganizationProfile/index.tsx(3 hunks)packages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsx(3 hunks)packages/ui/src/components/SignIn/index.tsx(3 hunks)packages/ui/src/components/SignUp/index.tsx(3 hunks)packages/ui/src/components/UserProfile/index.tsx(3 hunks)packages/ui/src/components/UserVerification/UserVerificationFactorTwo.tsx(1 hunks)packages/ui/src/components/UserVerification/index.tsx(2 hunks)packages/ui/src/customizables/parseAppearance.ts(1 hunks)packages/ui/src/internal/index.ts(1 hunks)packages/ui/src/internal/routing.ts(1 hunks)packages/ui/src/types.ts(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/ui/src/components/UserVerification/UserVerificationFactorTwo.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/ui/src/components/SignUp/index.tsx
- packages/ui/src/components/SignIn/index.tsx
- packages/shared/src/types/clerk.ts
🧰 Additional context used
📓 Path-based instructions (13)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/ui/src/internal/index.tspackages/ui/src/internal/routing.tspackages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsxpackages/ui/src/components/UserProfile/index.tsxpackages/ui/src/customizables/parseAppearance.tspackages/ui/src/types.tspackages/ui/src/components/OrganizationProfile/index.tsxpackages/ui/src/components/UserVerification/index.tsx
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/ui/src/internal/index.tspackages/ui/src/internal/routing.tspackages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsxpackages/ui/src/components/UserProfile/index.tsxpackages/ui/src/customizables/parseAppearance.tspackages/ui/src/types.tspackages/ui/src/components/OrganizationProfile/index.tsxpackages/ui/src/components/UserVerification/index.tsx
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/ui/src/internal/index.tspackages/ui/src/internal/routing.tspackages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsxpackages/ui/src/components/UserProfile/index.tsxpackages/ui/src/customizables/parseAppearance.tspackages/ui/src/types.tspackages/ui/src/components/OrganizationProfile/index.tsxpackages/ui/src/components/UserVerification/index.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Files:
packages/ui/src/internal/index.tspackages/ui/src/internal/routing.tspackages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsxpackages/ui/src/components/UserProfile/index.tsxpackages/ui/src/customizables/parseAppearance.tspackages/ui/src/types.tspackages/ui/src/components/OrganizationProfile/index.tsxpackages/ui/src/components/UserVerification/index.tsx
packages/**/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Implement proper logging with different levels
Files:
packages/ui/src/internal/index.tspackages/ui/src/internal/routing.tspackages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsxpackages/ui/src/components/UserProfile/index.tsxpackages/ui/src/customizables/parseAppearance.tspackages/ui/src/types.tspackages/ui/src/components/OrganizationProfile/index.tsxpackages/ui/src/components/UserVerification/index.tsx
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/ui/src/internal/index.tspackages/ui/src/internal/routing.tspackages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsxpackages/ui/src/components/UserProfile/index.tsxpackages/ui/src/customizables/parseAppearance.tspackages/ui/src/types.tspackages/ui/src/components/OrganizationProfile/index.tsxpackages/ui/src/components/UserVerification/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Implement type guards forunknowntypes using the patternfunction isType(value: unknown): value is Type
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details in classes
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Use mixins for shared behavior across unrelated classes in TypeScript
Use generic constraints with bounded type parameters like<T extends { id: string }>
Use utility types likeOmit,Partial, andPickfor data transformation instead of manual type construction
Use discriminated unions instead of boolean flags for state management and API responses
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation at the type level
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document functions with JSDoc comments including @param, @returns, @throws, and @example tags
Create custom error classes that extend Error for specific error types
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining (?.) and nullish coalescing (??) operators for safe property access
Let TypeScript infer obvious types to reduce verbosity
Useconst assertionswithas constfor literal types
Usesatisfiesoperator for type checking without widening types
Declare readonly arrays and objects for immutable data structures
Use spread operator and array spread for immutable updates instead of mutations
Use lazy loading for large types...
Files:
packages/ui/src/internal/index.tspackages/ui/src/internal/routing.tspackages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsxpackages/ui/src/components/UserProfile/index.tsxpackages/ui/src/customizables/parseAppearance.tspackages/ui/src/types.tspackages/ui/src/components/OrganizationProfile/index.tsxpackages/ui/src/components/UserVerification/index.tsx
**/index.ts
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
Avoid barrel files (index.ts re-exports) as they can cause circular dependencies
Files:
packages/ui/src/internal/index.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use ESLint with custom configurations tailored for different package types
Files:
packages/ui/src/internal/index.tspackages/ui/src/internal/routing.tspackages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsxpackages/ui/src/components/UserProfile/index.tsxpackages/ui/src/customizables/parseAppearance.tspackages/ui/src/types.tspackages/ui/src/components/OrganizationProfile/index.tsxpackages/ui/src/components/UserVerification/index.tsx
**/*.{js,ts,jsx,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use Prettier for code formatting across all packages
Files:
packages/ui/src/internal/index.tspackages/ui/src/internal/routing.tspackages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsxpackages/ui/src/components/UserProfile/index.tsxpackages/ui/src/customizables/parseAppearance.tspackages/ui/src/types.tspackages/ui/src/components/OrganizationProfile/index.tsxpackages/ui/src/components/UserVerification/index.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.tsx: Use error boundaries in React components
Minimize re-renders in React components
**/*.tsx: Use proper type definitions for props and state in React components
Leverage TypeScript's type inference where possible in React components
Use proper event types for handlers in React components
Implement proper generic types for reusable React components
Use proper type guards for conditional rendering in React components
Files:
packages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsxpackages/ui/src/components/UserProfile/index.tsxpackages/ui/src/components/OrganizationProfile/index.tsxpackages/ui/src/components/UserVerification/index.tsx
**/*.{md,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Update documentation for API changes
Files:
packages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsxpackages/ui/src/components/UserProfile/index.tsxpackages/ui/src/components/OrganizationProfile/index.tsxpackages/ui/src/components/UserVerification/index.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.{jsx,tsx}: Always use functional components with hooks instead of class components
Follow PascalCase naming for components (e.g.,UserProfile,NavigationMenu)
Keep components focused on a single responsibility - split large components
Limit component size to 150-200 lines; extract logic into custom hooks
Use composition over inheritance - prefer smaller, composable components
Export components as named exports for better tree-shaking
One component per file with matching filename and component name
Separate UI components from business logic components
Use useState for simple state management in React components
Use useReducer for complex state logic in React components
Implement proper state initialization in React components
Use proper state updates with callbacks in React components
Implement proper state cleanup in React components
Use Context API for theme/authentication state management
Implement proper state persistence in React applications
Use React.memo for expensive components
Implement proper useCallback for handlers in React components
Use proper useMemo for expensive computations in React components
Implement proper virtualization for lists in React components
Use proper code splitting with React.lazy in React applications
Implement proper cleanup in useEffect hooks
Use proper refs for DOM access in React components
Implement proper event listener cleanup in React components
Use proper abort controllers for fetch in React components
Implement proper subscription cleanup in React components
Use proper HTML elements for semantic HTML in React components
Implement proper ARIA attributes for accessibility in React components
Use proper heading hierarchy in React components
Implement proper form labels in React components
Use proper button types in React components
Implement proper focus management for keyboard navigation in React components
Use proper keyboard shortcuts in React components
Implement proper tab order in React components
Use proper ...
Files:
packages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsxpackages/ui/src/components/UserProfile/index.tsxpackages/ui/src/components/OrganizationProfile/index.tsxpackages/ui/src/components/UserVerification/index.tsx
🧬 Code graph analysis (6)
packages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsx (1)
packages/shared/src/types/signInCommon.ts (1)
SignInSecondFactor(86-86)
packages/ui/src/components/UserProfile/index.tsx (5)
packages/shared/src/types/clerk.ts (1)
UserProfileProps(1571-1609)packages/ui/src/types.ts (1)
UserProfileProps(43-43)packages/ui/src/elements/contexts/index.tsx (1)
withCardStateProvider(72-81)packages/ui/src/internal/index.ts (1)
WithInternalRouting(4-4)packages/ui/src/internal/routing.ts (1)
WithInternalRouting(3-5)
packages/ui/src/customizables/parseAppearance.ts (2)
packages/ui/src/internal/index.ts (2)
Appearance(40-40)Theme(68-68)packages/ui/src/internal/appearance.ts (2)
Appearance(998-1076)Theme(817-858)
packages/ui/src/types.ts (4)
packages/ui/src/internal/index.ts (1)
WithInternalRouting(4-4)packages/ui/src/internal/routing.ts (1)
WithInternalRouting(3-5)packages/shared/src/types/clerk.ts (3)
SignInProps(1349-1413)UserProfileProps(1571-1609)SignUpProps(1512-1567)packages/shared/src/types/redirects.ts (2)
SignInFallbackRedirectUrl(108-115)SignInForceRedirectUrl(117-122)
packages/ui/src/components/OrganizationProfile/index.tsx (5)
packages/shared/src/types/clerk.ts (1)
OrganizationProfileProps(1613-1652)packages/ui/src/types.ts (1)
OrganizationProfileProps(37-37)packages/ui/src/elements/contexts/index.tsx (1)
withCardStateProvider(72-81)packages/ui/src/internal/index.ts (1)
WithInternalRouting(4-4)packages/ui/src/internal/routing.ts (1)
WithInternalRouting(3-5)
packages/ui/src/components/UserVerification/index.tsx (4)
packages/ui/src/internal/index.ts (1)
WithInternalRouting(4-4)packages/ui/src/internal/routing.ts (1)
WithInternalRouting(3-5)packages/shared/src/types/clerk.ts (1)
__internal_UserVerificationProps(1427-1452)packages/ui/src/types.ts (1)
__internal_UserVerificationProps(33-33)
⏰ 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). (25)
- GitHub Check: Integration Tests (machine, chrome, RQ)
- GitHub Check: Integration Tests (billing, chrome, RQ)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 16, RQ)
- GitHub Check: Integration Tests (quickstart, chrome, 16)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (quickstart, chrome, 15)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (17)
packages/ui/src/customizables/parseAppearance.ts (1)
23-26: The exclusion of'options'is intentional and correct—it's aThemeproperty, not a component-scoped appearance key.PublicAppearanceTopLevelKeyis designed to include only component-specific cascade keys (likesignIn,signUp,userButton, etc.), and excludingkeyof Themeproperly removes all theme-level properties includingoptions. No code in the repository usesappearanceKey='options', confirming this was never a supported pattern and there is no breaking change.packages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsx (4)
1-1: LGTM! Type refinement improves accuracy.The change from
SignInFactortoSignInSecondFactorbetter reflects that this component specifically handles second-factor authentication methods, improving type safety and code clarity.
4-7: LGTM! Import path modernization.The migration from relative to absolute import paths (using
@/prefix) improves code maintainability and follows modern TypeScript conventions.
18-18: LGTM! Function signatures consistently updated.The function signatures correctly use
SignInSecondFactorthroughout, maintaining type consistency with the import change.Also applies to: 96-96
115-116: LGTM! Proper exhaustiveness check.The exhaustive type check pattern ensures all
SignInSecondFactorcases are handled at compile-time, providing strong type safety guarantees.packages/ui/src/internal/index.ts (1)
4-4: LGTM! Proper type-only export.The type-only export of
WithInternalRoutingfollows TypeScript best practices and enables better tree-shaking.packages/ui/src/components/UserVerification/index.tsx (2)
4-7: LGTM! Import updates align with refactoring.Import paths properly updated to absolute paths and the new
WithInternalRoutingtype is correctly imported using type-only import syntax.
36-36: LGTM! Public API updated to use internal routing wrapper.The component signature correctly uses
WithInternalRouting<__internal_UserVerificationProps>to wrap the props with the internal routing type, aligning with the PR's objective to control routing options at the type level.packages/ui/src/internal/routing.ts (1)
3-5: LGTM! Well-designed discriminated union for internal routing.The
WithInternalRouting<T>type creates a proper discriminated union that:
- Enforces path-based routing when
pathis provided- Allows hash/virtual routing when
pathis absent- Provides compile-time safety through mutual exclusion of
pathandroutingvariantsNote: The inclusion of
'virtual'in this internal type aligns with the PR objective—virtualrouting is removed from the public API but retained for internal modal implementations.packages/ui/src/components/UserProfile/index.tsx (3)
4-9: LGTM! Imports properly updated.Import paths correctly use absolute paths and all necessary types for the internal routing refactor are included.
18-18: LGTM! Proper separation of public and internal APIs.The component structure effectively separates:
_UserProfile: Core implementation (no props)UserProfile: Public API withUserProfilePropsInternalUserProfile: Internal wrapper withWithInternalRouting<UserProfileProps>This pattern allows modal components to use
routing: 'virtual'internally while keeping it out of the public API surface.Also applies to: 53-56
71-71: LGTM! Modal correctly uses internal wrapper.The modal properly uses
InternalUserProfileinstead of the publicUserProfilecomponent, enabling it to passrouting: 'virtual'internally while keeping this option hidden from the public API.packages/ui/src/components/OrganizationProfile/index.tsx (3)
5-13: LGTM! Import structure matches established pattern.Imports are properly updated with absolute paths and include all necessary types for the internal routing integration, consistent with other refactored components.
18-18: LGTM! Consistent component architecture.The component follows the same pattern as
UserProfile:
- Core implementation:
OrganizationProfileInternal- Public API:
OrganizationProfilewithOrganizationProfileProps- Internal wrapper:
InternalOrganizationProfilewithWithInternalRouting<OrganizationProfileProps>This consistency across components improves maintainability.
Also applies to: 59-63
78-78: LGTM! Modal properly uses internal wrapper.The modal correctly renders
InternalOrganizationProfile, enabling internal use ofrouting: 'virtual'while keeping it hidden from the public component API.packages/ui/src/types.ts (2)
29-29: LGTM! Proper type-only import.The type-only import of
WithInternalRoutingfollows TypeScript best practices for better tree-shaking.
70-70: LGTM! Context types consistently updated with internal routing wrapper.All major component context types (SignIn, SignUp, UserProfile, UserVerification, OrganizationProfile, CreateOrganization) are consistently wrapped with
WithInternalRouting<T>, ensuring uniform application of routing restrictions across the component library. This architectural consistency makes the codebase more maintainable and predictable.Also applies to: 76-76, 81-81, 86-86, 103-103, 108-108
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/ui/src/components/OrganizationProfile/index.tsx (1)
18-38: Add explicit return type annotation.Per TypeScript coding guidelines, functions should have explicit return types, especially for components that are part of the public API surface (even if wrapped).
Apply this diff:
-const OrganizationProfileInternal = () => { +const OrganizationProfileInternal = (): JSX.Element | null => {
📜 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 (1)
packages/ui/src/components/OrganizationProfile/index.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/ui/src/components/OrganizationProfile/index.tsx
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/ui/src/components/OrganizationProfile/index.tsx
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/ui/src/components/OrganizationProfile/index.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Files:
packages/ui/src/components/OrganizationProfile/index.tsx
packages/**/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Implement proper logging with different levels
Files:
packages/ui/src/components/OrganizationProfile/index.tsx
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/ui/src/components/OrganizationProfile/index.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.tsx: Use error boundaries in React components
Minimize re-renders in React components
**/*.tsx: Use proper type definitions for props and state in React components
Leverage TypeScript's type inference where possible in React components
Use proper event types for handlers in React components
Implement proper generic types for reusable React components
Use proper type guards for conditional rendering in React components
Files:
packages/ui/src/components/OrganizationProfile/index.tsx
**/*.{md,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Update documentation for API changes
Files:
packages/ui/src/components/OrganizationProfile/index.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.{jsx,tsx}: Always use functional components with hooks instead of class components
Follow PascalCase naming for components (e.g.,UserProfile,NavigationMenu)
Keep components focused on a single responsibility - split large components
Limit component size to 150-200 lines; extract logic into custom hooks
Use composition over inheritance - prefer smaller, composable components
Export components as named exports for better tree-shaking
One component per file with matching filename and component name
Separate UI components from business logic components
Use useState for simple state management in React components
Use useReducer for complex state logic in React components
Implement proper state initialization in React components
Use proper state updates with callbacks in React components
Implement proper state cleanup in React components
Use Context API for theme/authentication state management
Implement proper state persistence in React applications
Use React.memo for expensive components
Implement proper useCallback for handlers in React components
Use proper useMemo for expensive computations in React components
Implement proper virtualization for lists in React components
Use proper code splitting with React.lazy in React applications
Implement proper cleanup in useEffect hooks
Use proper refs for DOM access in React components
Implement proper event listener cleanup in React components
Use proper abort controllers for fetch in React components
Implement proper subscription cleanup in React components
Use proper HTML elements for semantic HTML in React components
Implement proper ARIA attributes for accessibility in React components
Use proper heading hierarchy in React components
Implement proper form labels in React components
Use proper button types in React components
Implement proper focus management for keyboard navigation in React components
Use proper keyboard shortcuts in React components
Implement proper tab order in React components
Use proper ...
Files:
packages/ui/src/components/OrganizationProfile/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Implement type guards forunknowntypes using the patternfunction isType(value: unknown): value is Type
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details in classes
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Use mixins for shared behavior across unrelated classes in TypeScript
Use generic constraints with bounded type parameters like<T extends { id: string }>
Use utility types likeOmit,Partial, andPickfor data transformation instead of manual type construction
Use discriminated unions instead of boolean flags for state management and API responses
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation at the type level
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document functions with JSDoc comments including @param, @returns, @throws, and @example tags
Create custom error classes that extend Error for specific error types
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining (?.) and nullish coalescing (??) operators for safe property access
Let TypeScript infer obvious types to reduce verbosity
Useconst assertionswithas constfor literal types
Usesatisfiesoperator for type checking without widening types
Declare readonly arrays and objects for immutable data structures
Use spread operator and array spread for immutable updates instead of mutations
Use lazy loading for large types...
Files:
packages/ui/src/components/OrganizationProfile/index.tsx
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use ESLint with custom configurations tailored for different package types
Files:
packages/ui/src/components/OrganizationProfile/index.tsx
**/*.{js,ts,jsx,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use Prettier for code formatting across all packages
Files:
packages/ui/src/components/OrganizationProfile/index.tsx
⏰ 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). (25)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 16, RQ)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (quickstart, chrome, 15)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (quickstart, chrome, 16)
- GitHub Check: Integration Tests (machine, chrome, RQ)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (billing, chrome, RQ)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (2)
packages/ui/src/components/OrganizationProfile/index.tsx (2)
5-13: LGTM!The imports are properly structured and support the new internal routing pattern. The addition of
WithInternalRoutingtype is consistent with the PR's objective to separate public and internal routing APIs.
65-83: No action required—the hardcodedrouting: 'virtual'pattern is correct.The
OrganizationProfileCtxtype extendsWithInternalRouting<OrganizationProfileProps>, which explicitly permitsrouting: 'virtual'alongsiderouting: 'hash'orrouting: 'path'. Settingrouting: 'virtual'on line 68 is intentional, type-safe, and consistent with the same pattern used in other modal-like components (e.g.,Waitlist). The type system is working as designed.Likely an incorrect or invalid review comment.
| export const OrganizationProfile: React.ComponentType<OrganizationProfileProps> = | ||
| withCardStateProvider(OrganizationProfileInternal); | ||
|
|
||
| const InternalOrganizationProfile: React.ComponentType<WithInternalRouting<OrganizationProfileProps>> = | ||
| withCardStateProvider(OrganizationProfileInternal); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the type definitions for WithInternalRouting and base props
# Also check the withCardStateProvider implementation
echo "=== WithInternalRouting type definition ==="
rg -n "type WithInternalRouting" --type=ts --type=tsx -A 3
echo -e "\n=== OrganizationProfileProps type definition ==="
rg -n "type OrganizationProfileProps" --type=ts --type=tsx -A 5
echo -e "\n=== OrganizationProfileCtx type definition ==="
rg -n "type OrganizationProfileCtx" --type=ts --type=tsx -A 5
echo -e "\n=== withCardStateProvider implementation ==="
rg -n "export.*withCardStateProvider|function withCardStateProvider" --type=ts --type=tsx -A 10Repository: clerk/javascript
Length of output: 376
🏁 Script executed:
#!/bin/bash
# Check the current file to see actual JSDoc presence and context
echo "=== File content around lines 59-63 ==="
sed -n '50,70p' packages/ui/src/components/OrganizationProfile/index.tsxRepository: clerk/javascript
Length of output: 865
🏁 Script executed:
#!/bin/bash
# Retry without file type flags to find type definitions
echo "=== WithInternalRouting type definition ==="
rg "type WithInternalRouting" -A 3
echo -e "\n=== OrganizationProfileProps type definition ==="
rg "type OrganizationProfileProps\s*=" -A 5
echo -e "\n=== withCardStateProvider implementation ==="
rg "withCardStateProvider\s*=" -B 2 -A 10Repository: clerk/javascript
Length of output: 1797
🏁 Script executed:
#!/bin/bash
# Look for JSDoc on the OrganizationProfile export
echo "=== Checking for JSDoc comments before export ==="
sed -n '45,62p' packages/ui/src/components/OrganizationProfile/index.tsxRepository: clerk/javascript
Length of output: 804
🏁 Script executed:
#!/bin/bash
# Find where WithInternalRouting is defined
echo "=== Finding WithInternalRouting definition ==="
fd -e ts -e tsx | xargs grep -l "WithInternalRouting" | head -5Repository: clerk/javascript
Length of output: 427
🏁 Script executed:
#!/bin/bash
# Find where withCardStateProvider is defined
echo "=== Finding withCardStateProvider ==="
fd -e ts -e tsx | xargs grep -l "withCardStateProvider" | head -10Repository: clerk/javascript
Length of output: 729
Add JSDoc documentation for the public OrganizationProfile export.
This is a public API that must be documented with JSDoc per coding guidelines. Include a description of the component's purpose, usage example, and prop documentation.
The type safety pattern with withCardStateProvider accepting both OrganizationProfileProps and WithInternalRouting<OrganizationProfileProps> is intentional by design, as evidenced by the @ts-expect-error in the HOC implementation that deliberately bridges these type signatures.
🤖 Prompt for AI Agents
In packages/ui/src/components/OrganizationProfile/index.tsx around lines 59 to
63, the public export OrganizationProfile lacks required JSDoc; add a JSDoc
block immediately above the export that documents the component’s purpose (brief
description), a short usage example showing how to import and render it, and
@param/@props tags describing OrganizationProfileProps properties (their types
and meanings) along with any notes about HOC typing/WithInternalRouting
behavior; keep the existing withCardStateProvider usage and do not change the
implementation—only add the comprehensive JSDoc for the public API.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
packages/ui/src/components/SignIn/index.tsx (2)
199-199: Effective internal wrapper for routing isolation.The
InternalSignIncomponent properly encapsulates the internal routing functionality usingWithInternalRouting<SignInProps>, allowing internal code to userouting='virtual'while keeping it hidden from the public API.Consider adding a brief JSDoc comment explaining this component's purpose for future maintainers:
+/** + * Internal SignIn component that supports virtual routing. + * Not exported - use the public `SignIn` component instead. + * @internal + */ const InternalSignIn: React.ComponentType<WithInternalRouting<SignInProps>> = withCoreSessionSwitchGuard(SignInRoot);
201-228: SignInModal successfully migrated to internal routing.The refactor correctly uses
InternalSignIninstead of the publicSignIncomponent, properly isolating the virtual routing functionality to internal usage.The
routing='virtual'prop appears to be set in two places:
- Line 214: In the
SignInContext.Providervalue- Line 222: As a direct prop to
InternalSignInIf
InternalSignInreads the routing value from context, the explicit prop at line 222 may be redundant. Consider verifying whether both are necessary:<div> <InternalSignIn {...signInProps} - routing='virtual' /> </div>However, if the explicit prop is needed for initialization or doesn't read from context for this specific value, keep it as-is.
packages/ui/src/components/SignUp/index.tsx (1)
5-11: LGTM: Import additions support internal routing correctly.All new imports are properly used throughout the file. The
WithInternalRoutingtype import enables the internal virtual routing pattern while maintaining type safety.Minor optimization: Consider using type-only import for
WithInternalRouting:-import { usePreloadTasks } from '@/hooks/usePreloadTasks'; -import type { WithInternalRouting } from '@/internal'; +import { usePreloadTasks } from '@/hooks/usePreloadTasks'; +import type { WithInternalRouting } from '@/internal';(Already correct, no change needed)
packages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsx (2)
115-115: Consider a more idiomatic exhaustiveness check pattern.The current exhaustiveness check
((_: never) => _)(factor)works correctly but is less conventional. Consider using a more readable pattern:Apply this diff for a clearer exhaustiveness check:
- ((_: never) => _)(factor); + factor satisfies never; throw new Error('Invalid sign in strategy');Or alternatively:
- ((_: never) => _)(factor); + const _exhaustiveCheck: never = factor; throw new Error('Invalid sign in strategy');
96-117: Add JSDoc documentation for the exported function.The exported
getButtonLabelfunction lacks JSDoc comments. According to coding guidelines, all public APIs should be documented with@param,@returns, and@throwstags.Apply this diff to add documentation:
+/** + * Returns the localization key for the button label based on the sign-in factor strategy. + * + * @param factor - The second factor authentication method to get the label for + * @returns The localization key for the button label + * @throws {Error} If an invalid or unsupported sign-in strategy is provided + */ export function getButtonLabel(factor: SignInSecondFactor): LocalizationKey {As per coding guidelines: "Maintain comprehensive JSDoc comments for public APIs" and "Document functions with JSDoc comments including @param, @returns, @throws, and @example tags".
packages/ui/src/components/UserProfile/index.tsx (1)
53-56: Clear split between publicUserProfileand internalInternalUserProfiletypingsReusing the same implementation while exposing
UserProfileasReact.ComponentType<UserProfileProps>and keepingInternalUserProfiletyped withWithInternalRouting<UserProfileProps>neatly confines'virtual'routing to internal consumers like modals without changing runtime behavior. If you ever want stricter typing, you could centralize any casts by giving_UserProfilean explicit props type and adjustingwithCardStateProviderusage there, but that’s optional for this PR.
📜 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)
packages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsx(3 hunks)packages/ui/src/components/SignIn/index.tsx(3 hunks)packages/ui/src/components/SignUp/index.tsx(3 hunks)packages/ui/src/components/UserProfile/index.tsx(3 hunks)packages/ui/src/components/UserVerification/UserVerificationFactorTwo.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/ui/src/components/UserVerification/UserVerificationFactorTwo.tsx
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/ui/src/components/SignUp/index.tsxpackages/ui/src/components/UserProfile/index.tsxpackages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsxpackages/ui/src/components/SignIn/index.tsx
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/ui/src/components/SignUp/index.tsxpackages/ui/src/components/UserProfile/index.tsxpackages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsxpackages/ui/src/components/SignIn/index.tsx
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/ui/src/components/SignUp/index.tsxpackages/ui/src/components/UserProfile/index.tsxpackages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsxpackages/ui/src/components/SignIn/index.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Files:
packages/ui/src/components/SignUp/index.tsxpackages/ui/src/components/UserProfile/index.tsxpackages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsxpackages/ui/src/components/SignIn/index.tsx
packages/**/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Implement proper logging with different levels
Files:
packages/ui/src/components/SignUp/index.tsxpackages/ui/src/components/UserProfile/index.tsxpackages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsxpackages/ui/src/components/SignIn/index.tsx
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/ui/src/components/SignUp/index.tsxpackages/ui/src/components/UserProfile/index.tsxpackages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsxpackages/ui/src/components/SignIn/index.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.tsx: Use error boundaries in React components
Minimize re-renders in React components
**/*.tsx: Use proper type definitions for props and state in React components
Leverage TypeScript's type inference where possible in React components
Use proper event types for handlers in React components
Implement proper generic types for reusable React components
Use proper type guards for conditional rendering in React components
Files:
packages/ui/src/components/SignUp/index.tsxpackages/ui/src/components/UserProfile/index.tsxpackages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsxpackages/ui/src/components/SignIn/index.tsx
**/*.{md,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Update documentation for API changes
Files:
packages/ui/src/components/SignUp/index.tsxpackages/ui/src/components/UserProfile/index.tsxpackages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsxpackages/ui/src/components/SignIn/index.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.{jsx,tsx}: Always use functional components with hooks instead of class components
Follow PascalCase naming for components (e.g.,UserProfile,NavigationMenu)
Keep components focused on a single responsibility - split large components
Limit component size to 150-200 lines; extract logic into custom hooks
Use composition over inheritance - prefer smaller, composable components
Export components as named exports for better tree-shaking
One component per file with matching filename and component name
Separate UI components from business logic components
Use useState for simple state management in React components
Use useReducer for complex state logic in React components
Implement proper state initialization in React components
Use proper state updates with callbacks in React components
Implement proper state cleanup in React components
Use Context API for theme/authentication state management
Implement proper state persistence in React applications
Use React.memo for expensive components
Implement proper useCallback for handlers in React components
Use proper useMemo for expensive computations in React components
Implement proper virtualization for lists in React components
Use proper code splitting with React.lazy in React applications
Implement proper cleanup in useEffect hooks
Use proper refs for DOM access in React components
Implement proper event listener cleanup in React components
Use proper abort controllers for fetch in React components
Implement proper subscription cleanup in React components
Use proper HTML elements for semantic HTML in React components
Implement proper ARIA attributes for accessibility in React components
Use proper heading hierarchy in React components
Implement proper form labels in React components
Use proper button types in React components
Implement proper focus management for keyboard navigation in React components
Use proper keyboard shortcuts in React components
Implement proper tab order in React components
Use proper ...
Files:
packages/ui/src/components/SignUp/index.tsxpackages/ui/src/components/UserProfile/index.tsxpackages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsxpackages/ui/src/components/SignIn/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Implement type guards forunknowntypes using the patternfunction isType(value: unknown): value is Type
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details in classes
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Use mixins for shared behavior across unrelated classes in TypeScript
Use generic constraints with bounded type parameters like<T extends { id: string }>
Use utility types likeOmit,Partial, andPickfor data transformation instead of manual type construction
Use discriminated unions instead of boolean flags for state management and API responses
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation at the type level
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document functions with JSDoc comments including @param, @returns, @throws, and @example tags
Create custom error classes that extend Error for specific error types
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining (?.) and nullish coalescing (??) operators for safe property access
Let TypeScript infer obvious types to reduce verbosity
Useconst assertionswithas constfor literal types
Usesatisfiesoperator for type checking without widening types
Declare readonly arrays and objects for immutable data structures
Use spread operator and array spread for immutable updates instead of mutations
Use lazy loading for large types...
Files:
packages/ui/src/components/SignUp/index.tsxpackages/ui/src/components/UserProfile/index.tsxpackages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsxpackages/ui/src/components/SignIn/index.tsx
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use ESLint with custom configurations tailored for different package types
Files:
packages/ui/src/components/SignUp/index.tsxpackages/ui/src/components/UserProfile/index.tsxpackages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsxpackages/ui/src/components/SignIn/index.tsx
**/*.{js,ts,jsx,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use Prettier for code formatting across all packages
Files:
packages/ui/src/components/SignUp/index.tsxpackages/ui/src/components/UserProfile/index.tsxpackages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsxpackages/ui/src/components/SignIn/index.tsx
🧬 Code graph analysis (3)
packages/ui/src/components/SignUp/index.tsx (4)
packages/ui/src/internal/routing.ts (1)
WithInternalRouting(3-5)packages/ui/src/internal/index.ts (1)
WithInternalRouting(4-4)packages/ui/src/types.ts (1)
SignUpProps(40-40)packages/shared/src/types/clerk.ts (1)
SignUpProps(1512-1567)
packages/ui/src/components/UserProfile/index.tsx (5)
packages/ui/src/types.ts (1)
UserProfileProps(43-43)packages/shared/src/types/clerk.ts (1)
UserProfileProps(1571-1609)packages/ui/src/elements/contexts/index.tsx (1)
withCardStateProvider(72-81)packages/ui/src/internal/routing.ts (1)
WithInternalRouting(3-5)packages/ui/src/internal/index.ts (1)
WithInternalRouting(4-4)
packages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsx (1)
packages/shared/src/types/signInCommon.ts (1)
SignInSecondFactor(86-86)
⏰ 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). (25)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 16, RQ)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (quickstart, chrome, 15)
- GitHub Check: Integration Tests (quickstart, chrome, 16)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (machine, chrome, RQ)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (billing, chrome, RQ)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (7)
packages/ui/src/components/SignIn/index.tsx (2)
5-19: LGTM! Import path refactoring is consistent.The centralization of import paths from
@/ui/*to@/*improves the module structure. The addition ofWithInternalRoutingtype properly supports the internal routing functionality while hiding it from the public API.
197-197: Public API correctly preserved.The public
SignIncomponent export remains unchanged, maintaining backward compatibility while the virtual routing option is being moved to internal use only.packages/ui/src/components/SignUp/index.tsx (2)
105-105: Well-designed internal wrapper for virtual routing.The
InternalSignUpcomponent correctly usesWithInternalRouting<SignUpProps>to enable virtual routing for internal modal usage while the publicSignUpexport does not expose this option. TheWithInternalRoutingtype explicitly excludes 'virtual' from the publicRoutingOptionsbut reintroduces it only for internal use. The naming convention and type annotations are appropriate, and the usage pattern inSignUpModalis correct.
126-129: Correct migration to InternalSignUp for modal rendering.The modal correctly uses
InternalSignUpwith explicitrouting='virtual', which aligns with the PR objective of removing virtual routing from the public API while maintaining it for internal modal usage. The explicit routing prop override ensures the modal always uses virtual routing regardless of props passed in. Verified that no other components in the codebase are still using the publicSignUpcomponent withrouting='virtual'.packages/ui/src/components/UserProfile/index.tsx (3)
4-12: Imports align with internal routing separationNew imports for the card scrollbox ID, user profile context/guard, router,
WithInternalRouting, andUserProfileCtxare consistent with the new internal-routing model and keep type-only imports where appropriate. No issues here.
18-34: Arg‑less_UserProfilekeeps routing concerns in contextMaking
_UserProfilea no‑arg component and driving configuration viaUserProfileContextkeeps the component surface clean and decoupled from routing props, while preserving the existing Flow/Route structure.
58-75: UsingInternalUserProfilekeeps'virtual'routing internal to the modal implementationThe public
UserProfileModalPropstype (which usesWithoutRouting<UserProfileProps>) removes routing options entirely, preventing developers from accessing'virtual'routing. By passingrouting: 'virtual'internally viaUserProfileCtxtoInternalUserProfile, the modal preserves the necessary virtual routing behavior without exposing this implementation detail to public consumers. This pattern aligns with how other modals (SignInModal, SignUpModal, etc.) handle internal routing.
| @@ -1,21 +1,21 @@ | |||
| import type { SignInFactor } from '@clerk/shared/types'; | |||
| import type { SignInSecondFactor } from '@clerk/shared/types'; | |||
There was a problem hiding this comment.
Add JSDoc documentation to the exported getButtonLabel function.
The function at line 96 is exported and lacks JSDoc comments. Per the coding guidelines for packages/**/src/**/*.{ts,tsx}, all public APIs must be documented with @param, @returns, and other relevant tags.
Additionally, verify that all components calling this component with onFactorSelected or invoking getButtonLabel pass instances compatible with SignInSecondFactor (PhoneCodeFactor, TOTPFactor, BackupCodeFactor, EmailCodeFactor, or EmailLinkFactor), since this narrows the accepted type from the previous SignInFactor.
🤖 Prompt for AI Agents
packages/ui/src/components/SignIn/SignInFactorTwoAlternativeMethods.tsx around
line 96: the exported function getButtonLabel is missing JSDoc and the review
requires documenting public APIs and tightening the accepted factor type to
SignInSecondFactor; add a JSDoc block above getButtonLabel that includes a brief
description, an @param tag describing the parameter as SignInSecondFactor
(enumerating allowed variants: PhoneCodeFactor, TOTPFactor, BackupCodeFactor,
EmailCodeFactor, EmailLinkFactor) and what each branch yields, and an @returns
tag describing the returned string; then update the function signature (and any
exported type annotations) to accept SignInSecondFactor instead of the broader
SignInFactor and run a quick grep/TS check to find all call sites using
onFactorSelected or calling getButtonLabel and ensure they pass only the allowed
second-factor variants, updating their types or narrow-casting where necessary
to satisfy the new signature.
2 participants
Description
Fixes: USER-2645
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
Breaking Changes
API / Types
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.