-
Couldn't load subscription status.
- Fork 403
feat(clerk-js): Introduce redirect rules #7039
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 158177d The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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.
|
WalkthroughThis PR centralizes redirect logic into a hook-based system: adds redirect rule types and evaluator, implements a generic useRedirectEngine hook, and provides useSignInRedirect/useSignUpRedirect. SignInStart now uses useSignInRedirect (removing HOC wrappers). Tests and integration scenarios for single-/multi-session redirects were added. Changes
Sequence Diagram(s)sequenceDiagram
participant SignInStart
participant useSignInRedirect
participant useRedirectEngine
participant evaluateRedirectRules
participant signInRedirectRules
SignInStart->>useSignInRedirect: mount (afterSignInUrl?, organizationTicket?)
Note over useSignInRedirect: prepare additionalContext (queryParams, hasInitializedRef)
useSignInRedirect->>useRedirectEngine: call with signInRedirectRules + additionalContext
rect rgb(235, 245, 255)
Note over useRedirectEngine: on deps/auth change → evaluate rules
useRedirectEngine->>evaluateRedirectRules: evaluate(rules, redirectContext)
evaluateRedirectRules->>signInRedirectRules: test rules sequentially
alt match found
signInRedirectRules-->>evaluateRedirectRules: RedirectResult
evaluateRedirectRules->>useRedirectEngine: return RedirectResult
useRedirectEngine->>useRedirectEngine: set isRedirecting = true
useRedirectEngine->>window.history: replaceState (cleanupQueryParams?)
useRedirectEngine->>window.location: navigate(destination) unless skipNavigation
else no match
signInRedirectRules-->>evaluateRedirectRules: null
evaluateRedirectRules->>useRedirectEngine: return null
useRedirectEngine->>useRedirectEngine: set isRedirecting = false
end
end
useRedirectEngine-->>useSignInRedirect: { isRedirecting }
useSignInRedirect-->>SignInStart: { isRedirecting }
alt isRedirecting
SignInStart->>SignInStart: render LoadingCard
else
SignInStart->>SignInStart: render sign-in UI
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
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: 3
🧹 Nitpick comments (10)
packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx (2)
7-26: Ensure mocks apply before module under test is evaluated.useAuthRedirect is imported (Line 7) before the vi.mock declarations. Vitest hoists mocks, but ordering can be brittle across ESM/CJS configs. Prefer mocking first, then dynamically importing the SUT.
Example pattern:
vi.mock('@clerk/shared/react', () => ({ useClerk: vi.fn() })); vi.mock('../../contexts', () => ({ useEnvironment: vi.fn() })); vi.mock('../../router', () => ({ useRouter: vi.fn() })); vi.mock('../../utils/redirectRules', () => ({ evaluateRedirectRules: vi.fn(), isDevelopmentMode: vi.fn(() => false) })); const { useAuthRedirect } = await import('../useAuthRedirect');
53-63: Add negative assertion: no navigation when rules return null.In the “no rules match” test, also assert that navigate was not called to prevent silent regressions.
expect(result.current.isRedirecting).toBe(false); +expect(mockNavigate).not.toHaveBeenCalled();packages/clerk-js/src/ui/hooks/useAuthRedirect.ts (1)
21-26: Prevent first‑paint flicker by deriving initial state.Currently, isRedirecting is false until the effect runs, which can flash the form before redirect. Precompute initial redirect match and seed state.
-export function useAuthRedirect(options: UseAuthRedirectOptions): UseAuthRedirectReturn { +export function useAuthRedirect<TExtra extends Record<string, unknown> = Record<string, never>>( + options: UseAuthRedirectOptions<TExtra>, +): UseAuthRedirectReturn { const clerk = useClerk(); const environment = useEnvironment(); const { navigate, currentPath } = useRouter(); - const [isRedirecting, setIsRedirecting] = React.useState(false); + const initialMatch = evaluateRedirectRules( + options.rules, + { clerk, currentPath, environment, ...options.additionalContext }, + isDevelopmentMode(clerk), + ); + const [isRedirecting, setIsRedirecting] = React.useState<boolean>(!!initialMatch);This keeps side effects (navigate) in the effect but avoids UI flash. Based on learnings.
Also applies to: 35-46
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx (2)
512-517: Good integration; consider synchronous initial redirect to avoid flash.With the current hook, first paint can show the form before isRedirecting flips. Once the hook seeds initial state (see hook comment), this will render LoadingCard immediately and prevent flicker.
518-522: Gate also on card loading when SSO kicks in.You already guard on status.isLoading and sign_up handoff. Consider also card.isLoading to keep parity with SSO redirect paths that set the card loading state.
-if (isRedirecting || status.isLoading || clerkStatus === 'sign_up') { +if (isRedirecting || status.isLoading || card.isLoading || clerkStatus === 'sign_up') { return <LoadingCard />; }Based on learnings.
packages/clerk-js/src/ui/utils/redirectRules.ts (5)
12-15: Add JSDoc for public API interface.This interface is exported but lacks documentation explaining the purpose of
destinationandreason.+/** + * Result of evaluating a redirect rule + * @property destination - The URL path to redirect to + * @property reason - Human-readable explanation for the redirect (used in debug logging) + */ export interface RedirectResult { readonly destination: string; readonly reason: string; }As per coding guidelines.
17-17: Add JSDoc for public API type.This exported type lacks documentation explaining when a rule should return
nullvs aRedirectResult.+/** + * A redirect rule function that evaluates whether a redirect should occur + * @param context - The current authentication and navigation context + * @returns A RedirectResult if the rule matches, or null to continue evaluating subsequent rules + */ export type RedirectRule = (context: RedirectContext) => RedirectResult | null;As per coding guidelines.
19-37: Consider expanding JSDoc with parameter descriptions.The JSDoc is present but minimal. Adding parameter and return descriptions would improve API documentation.
/** * Evaluates redirect rules in order, returning the first match + * @param rules - Array of redirect rules to evaluate in order + * @param context - The current authentication and navigation context + * @param debug - When true, logs matched rule information to console + * @returns The first matching RedirectResult, or null if no rules match */ export function evaluateRedirectRules(As per coding guidelines.
42-68: Add JSDoc for exported redirect rules.This exported constant lacks documentation explaining the purpose and order of the rules.
/** - * Redirect rules for SignIn component + * Redirect rules for SignIn component, evaluated in order + * 1. Single session mode: Redirect already-signed-in users to afterSignInUrl + * 2. Multi-session mode: Redirect to account switcher if at sign-in root with active sessions */ export const signInRedirectRules: RedirectRule[] = [As per coding guidelines.
73-99: Add JSDoc for exported redirect rules.This exported constant lacks documentation explaining the purpose and order of the rules.
/** - * Redirect rules for SignUp component + * Redirect rules for SignUp component, evaluated in order + * 1. Single session mode: Redirect already-signed-in users to afterSignUpUrl + * 2. Multi-session mode: Redirect to account switcher if at sign-up root with active sessions */ export const signUpRedirectRules: RedirectRule[] = [As per coding guidelines.
📜 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 (6)
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx(4 hunks)packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx(1 hunks)packages/clerk-js/src/ui/hooks/index.ts(1 hunks)packages/clerk-js/src/ui/hooks/useAuthRedirect.ts(1 hunks)packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts(1 hunks)packages/clerk-js/src/ui/utils/redirectRules.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (15)
packages/clerk-js/src/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/clerk-js-ui.mdc)
packages/clerk-js/src/ui/**/*.{ts,tsx}: Element descriptors should always be camelCase
Use element descriptors in UI components to enable consistent theming and styling via appearance.elements
Element descriptors should generate unique, stable CSS classes for theming
Element descriptors should handle state classes (e.g., cl-loading, cl-active, cl-error, cl-open) automatically based on component state
Do not render hard-coded values; all user-facing strings must be localized using provided localization methods
Use the useLocalizations hook and localizationKeys utility for all text and error messages
Use the styled system (sx prop, theme tokens, responsive values) for custom component styling
Use useCardState for card-level state, useFormState for form-level state, and useLoadingStatus for loading states
Always use handleError utility for API errors and use translateError for localized error messages
Use useFormControl for form field state, implement proper validation, and handle loading and error states in forms
Use localization keys for all form labels and placeholders
Use element descriptors for consistent styling and follow the theme token system
Use the Card and FormContainer patterns for consistent UI structure
Files:
packages/clerk-js/src/ui/hooks/index.tspackages/clerk-js/src/ui/hooks/useAuthRedirect.tspackages/clerk-js/src/ui/utils/redirectRules.tspackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
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
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/clerk-js/src/ui/hooks/index.tspackages/clerk-js/src/ui/hooks/useAuthRedirect.tspackages/clerk-js/src/ui/utils/redirectRules.tspackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/clerk-js/src/ui/hooks/index.tspackages/clerk-js/src/ui/hooks/useAuthRedirect.tspackages/clerk-js/src/ui/utils/redirectRules.tspackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/ui/hooks/index.tspackages/clerk-js/src/ui/hooks/useAuthRedirect.tspackages/clerk-js/src/ui/utils/redirectRules.tspackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/clerk-js/src/ui/hooks/index.tspackages/clerk-js/src/ui/hooks/useAuthRedirect.tspackages/clerk-js/src/ui/utils/redirectRules.tspackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx
packages/**/index.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use tree-shaking friendly exports
Files:
packages/clerk-js/src/ui/hooks/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{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
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/clerk-js/src/ui/hooks/index.tspackages/clerk-js/src/ui/hooks/useAuthRedirect.tspackages/clerk-js/src/ui/utils/redirectRules.tspackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/clerk-js/src/ui/hooks/index.tspackages/clerk-js/src/ui/hooks/useAuthRedirect.tspackages/clerk-js/src/ui/utils/redirectRules.tspackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx
**/index.ts
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
Use index.ts files for clean imports but avoid deep barrel exports
Avoid barrel files (index.ts re-exports) as they can cause circular dependencies
Files:
packages/clerk-js/src/ui/hooks/index.ts
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{jsx,tsx}: Use error boundaries in React components
Minimize re-renders in React components
**/*.{jsx,tsx}: Always use functional components with hooks instead of class components
Follow PascalCase naming for components: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
Use useState for simple state management
Use useReducer for complex state logic
Implement proper state initialization
Use proper state updates with callbacks
Implement proper state cleanup
Use Context API for theme/authentication
Implement proper state selectors
Use proper state normalization
Implement proper state persistence
Use React.memo for expensive components
Implement proper useCallback for handlers
Use proper useMemo for expensive computations
Implement proper virtualization for lists
Use proper code splitting with React.lazy
Implement proper cleanup in useEffect
Use proper refs for DOM access
Implement proper event listener cleanup
Use proper abort controllers for fetch
Implement proper subscription cleanup
Use proper HTML elements
Implement proper ARIA attributes
Use proper heading hierarchy
Implement proper form labels
Use proper button types
Implement proper focus management
Use proper keyboard shortcuts
Implement proper tab order
Use proper skip links
Implement proper focus traps
Implement proper error boundaries
Use proper error logging
Implement proper error recovery
Use proper error messages
Implement proper error fallbacks
Use proper form validation
Implement proper error states
Use proper error messages
Implement proper form submission
Use proper form reset
Use proper component naming
Implement proper file naming
Use proper prop naming
Implement proper...
Files:
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.tsx: Use proper type definitions for props and state
Leverage TypeScript's type inference where possible
Use proper event types for handlers
Implement proper generic types for reusable components
Use proper type guards for conditional rendering
Files:
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx
packages/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Unit tests should use Jest or Vitest as the test runner.
Files:
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx
packages/{clerk-js,elements,themes}/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Visual regression testing should be performed for UI components.
Files:
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/__tests__/**/*.{ts,tsx}: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces
Files:
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx
**/*.test.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.test.{jsx,tsx}: Use React Testing Library
Test component behavior, not implementation
Use proper test queries
Implement proper test isolation
Use proper test coverage
Test component interactions
Use proper test data
Implement proper test setup
Use proper test cleanup
Implement proper test assertions
Use proper test structure
Files:
packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx
🧬 Code graph analysis (5)
packages/clerk-js/src/ui/hooks/useAuthRedirect.ts (1)
packages/clerk-js/src/ui/utils/redirectRules.ts (3)
RedirectRule(17-17)evaluateRedirectRules(22-37)isDevelopmentMode(104-106)
packages/clerk-js/src/ui/utils/redirectRules.ts (2)
packages/types/src/environment.ts (1)
EnvironmentResource(10-24)packages/shared/src/keys.ts (1)
isDevelopmentFromPublishableKey(209-211)
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx (3)
packages/clerk-js/src/ui/hooks/useAuthRedirect.ts (1)
useAuthRedirect(21-46)packages/clerk-js/src/ui/utils/redirectRules.ts (1)
signInRedirectRules(42-68)packages/clerk-js/src/ui/elements/contexts/index.tsx (1)
withCardStateProvider(72-81)
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts (2)
packages/clerk-js/src/ui/utils/redirectRules.ts (4)
RedirectContext(5-10)evaluateRedirectRules(22-37)signInRedirectRules(42-68)signUpRedirectRules(73-99)packages/types/src/environment.ts (1)
EnvironmentResource(10-24)
packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx (3)
packages/types/src/environment.ts (1)
EnvironmentResource(10-24)packages/clerk-js/src/ui/utils/redirectRules.ts (2)
evaluateRedirectRules(22-37)RedirectRule(17-17)packages/clerk-js/src/ui/hooks/useAuthRedirect.ts (1)
useAuthRedirect(21-46)
⏰ 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). (4)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx (1)
111-129: Re-evaluation triggers covered — nice.Good assertions for isSignedIn, session count, singleSessionMode, and currentPath changes.
Also applies to: 131-149, 151-168, 170-188
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts (1)
8-49: Coverage is solid and well-structured.Tests validate order, single/multi-session, and root path handling for both sign-in and sign-up. Nice work.
Also applies to: 51-217, 220-301
packages/clerk-js/src/ui/hooks/index.ts (1)
1-1: No circular dependencies detected; export is safe.Verification confirms that useAuthRedirect does not import from the hooks barrel, and none of its dependencies (contexts, router, redirectRules utils) import from hooks/index. The barrel export pattern is already established in this file with 20 existing exports, and the new export follows the same pattern without introducing cycles.
packages/clerk-js/src/ui/utils/redirectRules.ts (2)
101-106: LGTM!The helper function correctly delegates to the existing
isDevelopmentFromPublishableKeyutility, has an explicit return type, and includes documentation.
87-98: ****The redirect to
/sign-in/choosein the sign-up flow is intentional and validated by tests. The test suite explicitly confirms this behavior: when a user is already signed in with active sessions in multi-session mode, they should be redirected to the account switcher rather than allowed to sign up again. This is consistent withsignInRedirectRulesand makes sense from a UX perspective.Likely an incorrect or invalid review comment.
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: 4
♻️ Duplicate comments (1)
packages/clerk-js/src/ui/utils/redirectRules.ts (1)
5-13: Replace[key: string]: anywith explicit fields orunknown.Use explicit optional properties for known fields and avoid
anyto restore type‑safety.Apply:
export interface RedirectContext { readonly clerk: Clerk; readonly currentPath: string; readonly environment: EnvironmentResource; readonly hasInitialized?: boolean; readonly organizationTicket?: string; readonly queryParams?: URLSearchParams; - readonly [key: string]: any; + readonly afterSignInUrl?: string; + readonly afterSignUpUrl?: string; + // If extensibility is required: + // readonly extras?: Record<string, unknown>; }
🧹 Nitpick comments (3)
integration/tests/session-tasks-multi-session.test.ts (1)
64-69: Harden locators and URL expectations.
- Use role‑based locator for the “Add account” button to avoid i18n flakiness.
- Make URL match tolerant to query strings.
- await u.page.getByText('Add account').click(); - await u.page.waitForURL(/sign-in$/); + await u.page.getByRole('button', { name: /add account/i }).click(); + await u.page.waitForURL(/\/sign-in(\/)?(\?.*)?$/);Apply at both occurrences.
Also applies to: 76-81
integration/tests/sign-in-single-session-mode.test.ts (1)
65-66: Avoid brittle text regex for form mount.Prefer accessible roles/labels or testids for the identifier field.
- await u.page.waitForSelector('text=/email address|username|phone/i'); + await u.page.getByRole('textbox', { name: /email address|username|phone/i }).waitFor({ state: 'visible' });Or use your page object’s
getIdentifierInput().packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts (1)
51-220: Add tests for “add account” and org‑ticket short‑circuit rules.Ensure Rule 2 (add account) and Rule 3 (org ticket) prevent later redirects.
+ it('skips navigation and cleans param when __clerk_add_account is present', () => { + const qp = new URLSearchParams('__clerk_add_account=1&x=1'); + const context: RedirectContext = { + clerk: { isSignedIn: true, client: { signedInSessions: [{ id: '1' }] } } as any, + currentPath: '/sign-in', + environment: { authConfig: { singleSessionMode: false } } as any, + queryParams: qp, + }; + const result = evaluateRedirectRules(signInRedirectRules, context); + expect(result).toEqual(expect.objectContaining({ skipNavigation: true, destination: '/sign-in' })); + // Simulate side-effect + result?.onRedirect?.(); + expect(qp.has('__clerk_add_account')).toBe(false); + }); + + it('short-circuits when organizationTicket is present', () => { + const context: RedirectContext = { + clerk: { isSignedIn: true, client: { signedInSessions: [{ id: '1' }] } } as any, + currentPath: '/sign-in', + environment: { authConfig: { singleSessionMode: false } } as any, + organizationTicket: 'ticket_123', + }; + const result = evaluateRedirectRules(signInRedirectRules, context); + expect(result).toEqual(expect.objectContaining({ skipNavigation: true })); + });
📜 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 (6)
integration/tests/session-tasks-multi-session.test.ts(1 hunks)integration/tests/sign-in-single-session-mode.test.ts(1 hunks)packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx(5 hunks)packages/clerk-js/src/ui/hooks/useAuthRedirect.ts(1 hunks)packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts(1 hunks)packages/clerk-js/src/ui/utils/redirectRules.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/clerk-js/src/ui/hooks/useAuthRedirect.ts
🧰 Additional context used
📓 Path-based instructions (15)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
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
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
integration/tests/session-tasks-multi-session.test.tspackages/clerk-js/src/ui/utils/redirectRules.tspackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tsintegration/tests/sign-in-single-session-mode.test.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
integration/tests/session-tasks-multi-session.test.tspackages/clerk-js/src/ui/utils/redirectRules.tspackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tsintegration/tests/sign-in-single-session-mode.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{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
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
integration/tests/session-tasks-multi-session.test.tspackages/clerk-js/src/ui/utils/redirectRules.tspackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tsintegration/tests/sign-in-single-session-mode.test.ts
integration/**
📄 CodeRabbit inference engine (.cursor/rules/global.mdc)
Framework integration templates and E2E tests should be placed under the integration/ directory
Files:
integration/tests/session-tasks-multi-session.test.tsintegration/tests/sign-in-single-session-mode.test.ts
integration/**/*
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
End-to-end tests and integration templates must be located in the 'integration/' directory.
Files:
integration/tests/session-tasks-multi-session.test.tsintegration/tests/sign-in-single-session-mode.test.ts
integration/**/*.{test,spec}.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Integration tests should use Playwright.
Files:
integration/tests/session-tasks-multi-session.test.tsintegration/tests/sign-in-single-session-mode.test.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
integration/tests/session-tasks-multi-session.test.tspackages/clerk-js/src/ui/utils/redirectRules.tspackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tsintegration/tests/sign-in-single-session-mode.test.ts
packages/clerk-js/src/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/clerk-js-ui.mdc)
packages/clerk-js/src/ui/**/*.{ts,tsx}: Element descriptors should always be camelCase
Use element descriptors in UI components to enable consistent theming and styling via appearance.elements
Element descriptors should generate unique, stable CSS classes for theming
Element descriptors should handle state classes (e.g., cl-loading, cl-active, cl-error, cl-open) automatically based on component state
Do not render hard-coded values; all user-facing strings must be localized using provided localization methods
Use the useLocalizations hook and localizationKeys utility for all text and error messages
Use the styled system (sx prop, theme tokens, responsive values) for custom component styling
Use useCardState for card-level state, useFormState for form-level state, and useLoadingStatus for loading states
Always use handleError utility for API errors and use translateError for localized error messages
Use useFormControl for form field state, implement proper validation, and handle loading and error states in forms
Use localization keys for all form labels and placeholders
Use element descriptors for consistent styling and follow the theme token system
Use the Card and FormContainer patterns for consistent UI structure
Files:
packages/clerk-js/src/ui/utils/redirectRules.tspackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/ui/utils/redirectRules.tspackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/clerk-js/src/ui/utils/redirectRules.tspackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{jsx,tsx}: Use error boundaries in React components
Minimize re-renders in React components
**/*.{jsx,tsx}: Always use functional components with hooks instead of class components
Follow PascalCase naming for components: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
Use useState for simple state management
Use useReducer for complex state logic
Implement proper state initialization
Use proper state updates with callbacks
Implement proper state cleanup
Use Context API for theme/authentication
Implement proper state selectors
Use proper state normalization
Implement proper state persistence
Use React.memo for expensive components
Implement proper useCallback for handlers
Use proper useMemo for expensive computations
Implement proper virtualization for lists
Use proper code splitting with React.lazy
Implement proper cleanup in useEffect
Use proper refs for DOM access
Implement proper event listener cleanup
Use proper abort controllers for fetch
Implement proper subscription cleanup
Use proper HTML elements
Implement proper ARIA attributes
Use proper heading hierarchy
Implement proper form labels
Use proper button types
Implement proper focus management
Use proper keyboard shortcuts
Implement proper tab order
Use proper skip links
Implement proper focus traps
Implement proper error boundaries
Use proper error logging
Implement proper error recovery
Use proper error messages
Implement proper error fallbacks
Use proper form validation
Implement proper error states
Use proper error messages
Implement proper form submission
Use proper form reset
Use proper component naming
Implement proper file naming
Use proper prop naming
Implement proper...
Files:
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.tsx: Use proper type definitions for props and state
Leverage TypeScript's type inference where possible
Use proper event types for handlers
Implement proper generic types for reusable components
Use proper type guards for conditional rendering
Files:
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx
packages/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Unit tests should use Jest or Vitest as the test runner.
Files:
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts
packages/{clerk-js,elements,themes}/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Visual regression testing should be performed for UI components.
Files:
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/__tests__/**/*.{ts,tsx}: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces
Files:
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts
🧬 Code graph analysis (4)
packages/clerk-js/src/ui/utils/redirectRules.ts (2)
packages/types/src/environment.ts (1)
EnvironmentResource(10-24)packages/shared/src/keys.ts (1)
isDevelopmentFromPublishableKey(209-211)
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx (2)
packages/clerk-js/src/ui/hooks/useAuthRedirect.ts (1)
useAuthRedirect(21-58)packages/clerk-js/src/ui/utils/redirectRules.ts (1)
signInRedirectRules(47-106)
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts (2)
packages/clerk-js/src/ui/utils/redirectRules.ts (4)
RedirectContext(5-13)evaluateRedirectRules(27-42)signInRedirectRules(47-106)signUpRedirectRules(114-140)packages/types/src/environment.ts (1)
EnvironmentResource(10-24)
integration/tests/sign-in-single-session-mode.test.ts (2)
integration/testUtils/index.ts (1)
createTestUtils(24-86)integration/presets/index.ts (1)
appConfigs(15-32)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx (1)
713-713: The review comment is based on an incorrect assumption about scope.The
withRedirectToAfterSignInandwithRedirectToSignInTaskHOCs are still actively used in multiple SignIn components (SignInFactorOne, SignInFactorTwo, SignInAccountSwitcher, SignInSSOCallback) and remain defined inpackages/clerk-js/src/ui/common/withRedirect.tsx.The change to SignInStart is a local modification to that component's export—not a codebase-wide removal. Each component independently wraps itself; removing these HOCs from SignInStart's export does not create dependencies elsewhere that need to be addressed.
Likely an incorrect or invalid review comment.
@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: 1
♻️ Duplicate comments (1)
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx (1)
513-522: Guard window access to prevent SSR breakage.The direct access to
window.location.searchon line 520 will cause runtime errors during server-side rendering.Apply this diff:
const { isRedirecting } = useAuthRedirect({ rules: signInRedirectRules, additionalContext: { afterSignInUrl, hasInitialized: hasInitializedRef.current, organizationTicket, - queryParams: new URLSearchParams(window.location.search), + queryParams: typeof window !== 'undefined' ? new URLSearchParams(window.location.search) : undefined, }, });
📜 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 (3)
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx(5 hunks)packages/clerk-js/src/ui/hooks/useAuthRedirect.ts(1 hunks)packages/clerk-js/src/ui/utils/redirectRules.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/clerk-js/src/ui/hooks/useAuthRedirect.ts
🧰 Additional context used
📓 Path-based instructions (9)
packages/clerk-js/src/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/clerk-js-ui.mdc)
packages/clerk-js/src/ui/**/*.{ts,tsx}: Element descriptors should always be camelCase
Use element descriptors in UI components to enable consistent theming and styling via appearance.elements
Element descriptors should generate unique, stable CSS classes for theming
Element descriptors should handle state classes (e.g., cl-loading, cl-active, cl-error, cl-open) automatically based on component state
Do not render hard-coded values; all user-facing strings must be localized using provided localization methods
Use the useLocalizations hook and localizationKeys utility for all text and error messages
Use the styled system (sx prop, theme tokens, responsive values) for custom component styling
Use useCardState for card-level state, useFormState for form-level state, and useLoadingStatus for loading states
Always use handleError utility for API errors and use translateError for localized error messages
Use useFormControl for form field state, implement proper validation, and handle loading and error states in forms
Use localization keys for all form labels and placeholders
Use element descriptors for consistent styling and follow the theme token system
Use the Card and FormContainer patterns for consistent UI structure
Files:
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/utils/redirectRules.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
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
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/utils/redirectRules.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/utils/redirectRules.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/utils/redirectRules.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/utils/redirectRules.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{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
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/utils/redirectRules.ts
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{jsx,tsx}: Use error boundaries in React components
Minimize re-renders in React components
**/*.{jsx,tsx}: Always use functional components with hooks instead of class components
Follow PascalCase naming for components: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
Use useState for simple state management
Use useReducer for complex state logic
Implement proper state initialization
Use proper state updates with callbacks
Implement proper state cleanup
Use Context API for theme/authentication
Implement proper state selectors
Use proper state normalization
Implement proper state persistence
Use React.memo for expensive components
Implement proper useCallback for handlers
Use proper useMemo for expensive computations
Implement proper virtualization for lists
Use proper code splitting with React.lazy
Implement proper cleanup in useEffect
Use proper refs for DOM access
Implement proper event listener cleanup
Use proper abort controllers for fetch
Implement proper subscription cleanup
Use proper HTML elements
Implement proper ARIA attributes
Use proper heading hierarchy
Implement proper form labels
Use proper button types
Implement proper focus management
Use proper keyboard shortcuts
Implement proper tab order
Use proper skip links
Implement proper focus traps
Implement proper error boundaries
Use proper error logging
Implement proper error recovery
Use proper error messages
Implement proper error fallbacks
Use proper form validation
Implement proper error states
Use proper error messages
Implement proper form submission
Use proper form reset
Use proper component naming
Implement proper file naming
Use proper prop naming
Implement proper...
Files:
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/utils/redirectRules.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.tsx: Use proper type definitions for props and state
Leverage TypeScript's type inference where possible
Use proper event types for handlers
Implement proper generic types for reusable components
Use proper type guards for conditional rendering
Files:
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx
🧬 Code graph analysis (2)
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx (3)
packages/clerk-js/src/ui/hooks/useAuthRedirect.ts (1)
useAuthRedirect(21-58)packages/clerk-js/src/ui/utils/redirectRules.ts (1)
signInRedirectRules(47-106)packages/clerk-js/src/ui/elements/contexts/index.tsx (1)
withCardStateProvider(72-81)
packages/clerk-js/src/ui/utils/redirectRules.ts (2)
packages/types/src/environment.ts (1)
EnvironmentResource(10-24)packages/shared/src/keys.ts (1)
isDevelopmentFromPublishableKey(209-211)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (7)
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx (5)
13-43: LGTM!The import additions properly support the new redirect functionality, including
useAuthRedirect,signInRedirectRules, and related utilities.
107-107: LGTM!The initialization tracking ref is correctly implemented and prevents redirect loops by allowing rules to distinguish the first render.
524-527: LGTM!The initialization effect correctly sets the ref after the first render with an empty dependency array, enabling redirect rules to distinguish initial load from subsequent renders.
529-529: LGTM!Adding
isRedirectingto the loading condition correctly prevents rendering the form during redirect navigation, improving UX.
713-713: LGTM!The simplified export correctly reflects the refactored architecture where redirect logic is now handled internally via the
useAuthRedirecthook, eliminating the need for the previous HOC wrappers.packages/clerk-js/src/ui/utils/redirectRules.ts (2)
27-31: Add explicit return type annotation.Per coding guidelines, always define explicit return types for functions, especially public APIs.
Apply this diff:
export function evaluateRedirectRules( rules: RedirectRule[], context: RedirectContext, debug = false, -): RedirectResult | null { +): RedirectResult | null {Wait, the return type is already there. Let me re-examine the code...
Actually, looking at the code again, the return type IS present on line 31. No issue here.
145-147: Add explicit return type annotation.Per coding guidelines, always define explicit return types for functions, especially public APIs.
Apply this diff:
-export function isDevelopmentMode(clerk: Clerk): boolean { +export function isDevelopmentMode(clerk: Clerk): boolean { return isDevelopmentFromPublishableKey(clerk.publishableKey); }As per coding guidelines.
Likely an incorrect or invalid review comment.
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: 0
🧹 Nitpick comments (1)
.changeset/plenty-dolls-pick.md (1)
5-5: Description could be more user-centric.The changeset description "Centralize redirect concerns for SignIn" is technically accurate but lacks context for changelog consumers. Consider expanding it to explain the user-facing benefit or the scope of the refactoring.
Example: "Centralize SignIn redirect handling via a new
useAuthRedirecthook, improving consistency and maintainability."
📜 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/plenty-dolls-pick.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.changeset/**
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Automated releases must use Changesets.
Files:
.changeset/plenty-dolls-pick.md
🪛 LanguageTool
.changeset/plenty-dolls-pick.md
[grammar] ~5-~5: Ensure spelling is correct
Context: ...r --- Centralize redirect concerns for SignIn
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (29)
- GitHub Check: Integration Tests (quickstart, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Static analysis
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Unit Tests (18, --filter=@clerk/astro --filter=@clerk/backend --filter=@clerk/express --filter=@c...
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
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
♻️ Duplicate comments (5)
packages/clerk-js/src/ui/utils/redirectRules.ts (4)
67-74: Guard window access for SSR safety.Wrap
windowusage to avoid SSR crashes.onRedirect: () => { - // Clean up query param + if (typeof window === 'undefined') return; + // Clean up query param ctx.queryParams?.delete('__clerk_add_account'); const newSearch = ctx.queryParams?.toString(); const newUrl = window.location.pathname + (newSearch ? `?${newSearch}` : ''); window.history.replaceState({}, '', newUrl); },As per coding guidelines.
79-85: Org-ticket rule is a no‑op; short‑circuit with non‑navigating result.Ensure later rules don’t run when handling organization tickets.
// Rule 3: Skip redirect if handling organization ticket ctx => { - if (ctx.organizationTicket) { - return null; - } - return null; + if (ctx.organizationTicket) { + return { + destination: ctx.currentPath, + reason: 'Organization ticket present', + skipNavigation: true, + }; + } + return null; },
97-101: Use absolute path for destination.Use '/sign-in/choose' for consistency with router patterns and other rules.
- return { - destination: 'choose', + return { + destination: '/sign-in/choose', reason: 'Active sessions detected (multi-session mode)', };#!/bin/bash # Verify no remaining relative 'choose' destinations and find navigate path patterns rg -nP --type=ts --type=tsx -C2 "destination:\s*['\"]choose['\"]|['\"]/sign-in/choose['\"]" packages
4-12: Replace[key: string]: anywith explicit fields orunknown.The permissive index signature weakens type safety for known keys (afterSignInUrl, afterSignUpUrl, hasInitialized, organizationTicket, queryParams).
export interface RedirectContext { readonly clerk: Clerk; readonly currentPath: string; readonly environment: EnvironmentResource; readonly hasInitialized?: boolean; readonly organizationTicket?: string; readonly queryParams?: URLSearchParams; - readonly [key: string]: any; + readonly afterSignInUrl?: string; + readonly afterSignUpUrl?: string; + readonly [key: string]: unknown; }As per coding guidelines.
packages/clerk-js/src/ui/hooks/useAuthRedirect.ts (1)
31-59: Add missing effect deps and guard against navigating to the same path.Without deps on options and publishableKey, evaluations can go stale; navigating to currentPath may loop.
React.useEffect(() => { const context = { clerk, currentPath, environment, ...options.additionalContext, }; - const result = evaluateRedirectRules(options.rules, context, isDevelopmentMode(clerk)); + const result = evaluateRedirectRules(options.rules, context, isDevelopmentMode(clerk)); if (result) { // Execute any side effects result.onRedirect?.(); // Only navigate if not explicitly skipped - if (!result.skipNavigation) { + if (!result.skipNavigation && result.destination !== currentPath) { setIsRedirecting(true); void navigate(result.destination); } } else { setIsRedirecting(false); } - }, [ + }, [ clerk.isSignedIn, clerk.client?.sessions?.length, clerk.client?.signedInSessions?.length, + clerk.publishableKey, environment.authConfig.singleSessionMode, currentPath, + options.rules, + options.additionalContext, ]);Based on coding guidelines.
🧹 Nitpick comments (4)
packages/clerk-js/src/ui/hooks/useAuthRedirect.ts (1)
9-11: Make rules readonly to avoid accidental mutation.Public options should prefer ReadonlyArray.
-export interface UseAuthRedirectOptions<C extends Record<string, unknown> = Record<string, unknown>> { - rules: RedirectRule[]; +export interface UseAuthRedirectOptions<C extends Record<string, unknown> = Record<string, unknown>> { + rules: readonly RedirectRule[];As per coding guidelines.
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts (1)
169-184: Add a test for organization ticket short‑circuit.Ensure later rules don’t fire when organizationTicket is present.
+ it('short-circuits when organization ticket is present', () => { + const context: RedirectContext = { + clerk: { isSignedIn: true, client: { signedInSessions: [{ id: '1' }] } } as any, + currentPath: '/sign-in', + environment: { authConfig: { singleSessionMode: false } } as EnvironmentResource, + organizationTicket: 'otk_123', + }; + const result = evaluateRedirectRules(signInRedirectRules, context); + expect(result).toEqual({ + destination: '/sign-in', + reason: 'Organization ticket present', + skipNavigation: true, + }); + });packages/clerk-js/src/ui/utils/redirectRules.ts (2)
26-31: Accept ReadonlyArray for rules.Avoid mutation and allow const rules.
-export function evaluateRedirectRules( - rules: RedirectRule[], +export function evaluateRedirectRules( + rules: ReadonlyArray<RedirectRule>, context: RedirectContext, debug = false, ): RedirectResult | null {
46-56: Export rules as readonly.Prevent accidental push/splice on shared rule sets.
-export const signInRedirectRules: RedirectRule[] = [ +export const signInRedirectRules: ReadonlyArray<RedirectRule> = [
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
.changeset/plenty-dolls-pick.md(1 hunks)packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx(1 hunks)packages/clerk-js/src/ui/hooks/useAuthRedirect.ts(1 hunks)packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts(1 hunks)packages/clerk-js/src/ui/utils/redirectRules.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/clerk-js/src/ui/hooks/tests/useAuthRedirect.test.tsx
🧰 Additional context used
📓 Path-based instructions (11)
packages/clerk-js/src/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/clerk-js-ui.mdc)
packages/clerk-js/src/ui/**/*.{ts,tsx}: Element descriptors should always be camelCase
Use element descriptors in UI components to enable consistent theming and styling via appearance.elements
Element descriptors should generate unique, stable CSS classes for theming
Element descriptors should handle state classes (e.g., cl-loading, cl-active, cl-error, cl-open) automatically based on component state
Do not render hard-coded values; all user-facing strings must be localized using provided localization methods
Use the useLocalizations hook and localizationKeys utility for all text and error messages
Use the styled system (sx prop, theme tokens, responsive values) for custom component styling
Use useCardState for card-level state, useFormState for form-level state, and useLoadingStatus for loading states
Always use handleError utility for API errors and use translateError for localized error messages
Use useFormControl for form field state, implement proper validation, and handle loading and error states in forms
Use localization keys for all form labels and placeholders
Use element descriptors for consistent styling and follow the theme token system
Use the Card and FormContainer patterns for consistent UI structure
Files:
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/useAuthRedirect.tspackages/clerk-js/src/ui/utils/redirectRules.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
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
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/useAuthRedirect.tspackages/clerk-js/src/ui/utils/redirectRules.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/useAuthRedirect.tspackages/clerk-js/src/ui/utils/redirectRules.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/useAuthRedirect.tspackages/clerk-js/src/ui/utils/redirectRules.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/useAuthRedirect.tspackages/clerk-js/src/ui/utils/redirectRules.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{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
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/useAuthRedirect.tspackages/clerk-js/src/ui/utils/redirectRules.ts
packages/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Unit tests should use Jest or Vitest as the test runner.
Files:
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts
packages/{clerk-js,elements,themes}/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Visual regression testing should be performed for UI components.
Files:
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/useAuthRedirect.tspackages/clerk-js/src/ui/utils/redirectRules.ts
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/__tests__/**/*.{ts,tsx}: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces
Files:
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts
.changeset/**
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Automated releases must use Changesets.
Files:
.changeset/plenty-dolls-pick.md
🧬 Code graph analysis (3)
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts (2)
packages/clerk-js/src/ui/utils/redirectRules.ts (3)
RedirectContext(4-12)evaluateRedirectRules(26-41)signInRedirectRules(46-105)packages/types/src/environment.ts (1)
EnvironmentResource(10-24)
packages/clerk-js/src/ui/hooks/useAuthRedirect.ts (1)
packages/clerk-js/src/ui/utils/redirectRules.ts (3)
RedirectRule(21-21)evaluateRedirectRules(26-41)isDevelopmentMode(110-112)
packages/clerk-js/src/ui/utils/redirectRules.ts (2)
packages/types/src/environment.ts (1)
EnvironmentResource(10-24)packages/shared/src/keys.ts (1)
isDevelopmentFromPublishableKey(209-211)
🪛 LanguageTool
.changeset/plenty-dolls-pick.md
[grammar] ~5-~5: Ensure spelling is correct
Context: ...r --- Centralize redirect concerns for SignIn
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (28)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (quickstart, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Static analysis
- GitHub Check: Unit Tests (18, --filter=@clerk/astro --filter=@clerk/backend --filter=@clerk/express --filter=@c...
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts (1)
50-124: Core rule tests look solid.Good coverage for first‑match and single‑session behavior.
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: 0
♻️ Duplicate comments (3)
packages/clerk-js/src/ui/utils/redirectRules.ts (3)
103-110: Guardwindowaccess for SSR safety.Wrap
onRedirectbody with atypeof window === 'undefined'guard to avoid SSR breakage.Apply:
onRedirect: () => { + if (typeof window === 'undefined') return; // Clean up query param ctx.queryParams?.delete('__clerk_add_account'); const newSearch = ctx.queryParams?.toString(); const newUrl = window.location.pathname + (newSearch ? `?${newSearch}` : ''); window.history.replaceState({}, '', newUrl); },
125-129: Use absolute destination for the chooser route.Return
'/sign-in/choose'for consistency and to avoid router ambiguity.Apply:
- return { - destination: 'choose', + return { + destination: '/sign-in/choose', reason: 'Active sessions detected (multi-session mode)', };#!/bin/bash # Confirm how routes are resolved and whether 'choose' exists as a relative segment. rg -nP --type=ts --type=tsx -C2 "/sign-in/choose|destination:\s*'choose'" packages
4-12: Remove[key: string]: any; model known fields or useunknown.This index signature defeats type safety and violates the “no any” guideline. Specify known optional fields (e.g., afterSignInUrl, afterSignUpUrl, hasInitialized, organizationTicket, queryParams) or switch to
unknownand narrow at use sites.Apply one of:
export interface RedirectContext { readonly clerk: Clerk; readonly currentPath: string; readonly environment: EnvironmentResource; readonly hasInitialized?: boolean; readonly organizationTicket?: string; readonly queryParams?: URLSearchParams; - readonly [key: string]: any; + readonly afterSignInUrl?: string; + readonly afterSignUpUrl?: string; }or
- readonly [key: string]: any; + readonly [key: string]: unknown;As per coding guidelines.
🧹 Nitpick comments (3)
packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx (2)
53-62: Also assert no navigation occurs.Strengthen the “no redirect” test by asserting that navigate was not called.
Apply:
expect(result.current.isRedirecting).toBe(false); + expect(mockNavigate).not.toHaveBeenCalled();
149-170: Add coverage for other effect deps (sessions, mode, path).The hook re-evaluates on sessions length, singleSessionMode, and currentPath changes; add tests mirroring this to prevent regressions.
I can draft tests that mutate:
- clerk.client.signedInSessions length,
- environment.authConfig.singleSessionMode,
- router.currentPath,
and assert evaluateRedirectRules call count increments. Want a patch?packages/clerk-js/src/ui/utils/redirectRules.ts (1)
85-91: Prefer Environment API (isSingleSession()) over internal shape.Avoid reaching into
authConfig.singleSessionMode; use the publicenvironment.isSingleSession()helper for stability and clearer intent.Apply:
- if (ctx.clerk.isSignedIn && ctx.environment.authConfig.singleSessionMode) { + if (ctx.clerk.isSignedIn && ctx.environment.isSingleSession()) {- const isMultiSessionMode = !ctx.environment.authConfig.singleSessionMode; + const isMultiSessionMode = !ctx.environment.isSingleSession();Also applies to: 122-129
📜 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 (3)
packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx(1 hunks)packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts(1 hunks)packages/clerk-js/src/ui/utils/redirectRules.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/clerk-js/src/ui/utils/tests/redirectRules.test.ts
🧰 Additional context used
📓 Path-based instructions (13)
packages/clerk-js/src/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/clerk-js-ui.mdc)
packages/clerk-js/src/ui/**/*.{ts,tsx}: Element descriptors should always be camelCase
Use element descriptors in UI components to enable consistent theming and styling via appearance.elements
Element descriptors should generate unique, stable CSS classes for theming
Element descriptors should handle state classes (e.g., cl-loading, cl-active, cl-error, cl-open) automatically based on component state
Do not render hard-coded values; all user-facing strings must be localized using provided localization methods
Use the useLocalizations hook and localizationKeys utility for all text and error messages
Use the styled system (sx prop, theme tokens, responsive values) for custom component styling
Use useCardState for card-level state, useFormState for form-level state, and useLoadingStatus for loading states
Always use handleError utility for API errors and use translateError for localized error messages
Use useFormControl for form field state, implement proper validation, and handle loading and error states in forms
Use localization keys for all form labels and placeholders
Use element descriptors for consistent styling and follow the theme token system
Use the Card and FormContainer patterns for consistent UI structure
Files:
packages/clerk-js/src/ui/utils/redirectRules.tspackages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
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
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/clerk-js/src/ui/utils/redirectRules.tspackages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/clerk-js/src/ui/utils/redirectRules.tspackages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/ui/utils/redirectRules.tspackages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/clerk-js/src/ui/utils/redirectRules.tspackages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{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
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/clerk-js/src/ui/utils/redirectRules.tspackages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/clerk-js/src/ui/utils/redirectRules.tspackages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{jsx,tsx}: Use error boundaries in React components
Minimize re-renders in React components
**/*.{jsx,tsx}: Always use functional components with hooks instead of class components
Follow PascalCase naming for components: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
Use useState for simple state management
Use useReducer for complex state logic
Implement proper state initialization
Use proper state updates with callbacks
Implement proper state cleanup
Use Context API for theme/authentication
Implement proper state selectors
Use proper state normalization
Implement proper state persistence
Use React.memo for expensive components
Implement proper useCallback for handlers
Use proper useMemo for expensive computations
Implement proper virtualization for lists
Use proper code splitting with React.lazy
Implement proper cleanup in useEffect
Use proper refs for DOM access
Implement proper event listener cleanup
Use proper abort controllers for fetch
Implement proper subscription cleanup
Use proper HTML elements
Implement proper ARIA attributes
Use proper heading hierarchy
Implement proper form labels
Use proper button types
Implement proper focus management
Use proper keyboard shortcuts
Implement proper tab order
Use proper skip links
Implement proper focus traps
Implement proper error boundaries
Use proper error logging
Implement proper error recovery
Use proper error messages
Implement proper error fallbacks
Use proper form validation
Implement proper error states
Use proper error messages
Implement proper form submission
Use proper form reset
Use proper component naming
Implement proper file naming
Use proper prop naming
Implement proper...
Files:
packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx
packages/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Unit tests should use Jest or Vitest as the test runner.
Files:
packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx
packages/{clerk-js,elements,themes}/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Visual regression testing should be performed for UI components.
Files:
packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.tsx: Use proper type definitions for props and state
Leverage TypeScript's type inference where possible
Use proper event types for handlers
Implement proper generic types for reusable components
Use proper type guards for conditional rendering
Files:
packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx
**/*.test.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.test.{jsx,tsx}: Use React Testing Library
Test component behavior, not implementation
Use proper test queries
Implement proper test isolation
Use proper test coverage
Test component interactions
Use proper test data
Implement proper test setup
Use proper test cleanup
Implement proper test assertions
Use proper test structure
Files:
packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/__tests__/**/*.{ts,tsx}: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces
Files:
packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx
🧬 Code graph analysis (2)
packages/clerk-js/src/ui/utils/redirectRules.ts (2)
packages/types/src/environment.ts (1)
EnvironmentResource(10-24)packages/shared/src/keys.ts (1)
isDevelopmentFromPublishableKey(209-211)
packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx (3)
packages/types/src/environment.ts (1)
EnvironmentResource(10-24)packages/clerk-js/src/ui/utils/redirectRules.ts (1)
evaluateRedirectRules(43-69)packages/clerk-js/src/ui/hooks/useAuthRedirect.ts (1)
useAuthRedirect(23-62)
⏰ 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: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (2)
packages/clerk-js/src/ui/utils/redirectRules.ts (1)
86-86: No issues found —buildAfterSignInUrl()is a public method on the Clerk client.The method is defined as
public buildAfterSignInUrl({ params }: { params?: URLSearchParams } = {}): stringin the Clerk class and is part of the Clerk interface with documentation stating it "Returns the configured afterSignInUrl of the instance." The code at line 86 correctly uses this established helper for computing the afterSignIn URL when none is explicitly provided.packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx (1)
2-2: No issues found—import is compatible with installed RTL version.The codebase has React Testing Library v16 installed, which exports renderHook directly from @testing-library/react. The import statement
import { renderHook, waitFor } from '@testing-library/react';is correct and follows the current RTL standard. No changes are required.
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: 0
♻️ Duplicate comments (2)
packages/clerk-js/src/ui/utils/redirectRules.ts (1)
5-14: Remove[key: string]: any; make context explicit or useunknown.
anydefeats type safety for public API. Add explicit optionals (e.g.,afterSignInUrl, etc.) and drop the index signature, or change it tounknownif extensibility is required.export interface RedirectContext { readonly afterSignInUrl?: string; readonly clerk: Clerk; readonly currentPath: string; readonly environment: EnvironmentResource; readonly hasInitialized?: boolean; readonly organizationTicket?: string; readonly queryParams?: Record<string, string>; - readonly [key: string]: any; }If arbitrary keys are needed in future, prefer
readonly [key: string]: unknown.
As per coding guidelines.packages/clerk-js/src/ui/hooks/useAuthRedirect.ts (1)
31-59: Effect misses option deps; guard against no-op navigations.
- Add
options.rulesandoptions.additionalContext(and stablenavigate) to deps to avoid stale evaluations.- Skip navigating to the same path to prevent redundant transitions.
React.useEffect(() => { const context = { clerk, currentPath, environment, ...options.additionalContext, }; const result = evaluateRedirectRules(options.rules, context); if (result) { // Execute any side effects result.onRedirect?.(); // Only navigate if not explicitly skipped - if (!result.skipNavigation) { + if (!result.skipNavigation && result.destination !== currentPath) { setIsRedirecting(true); void navigate(result.destination); } } else { setIsRedirecting(false); } - }, [ + }, [ clerk.isSignedIn, clerk.client?.sessions?.length, clerk.client?.signedInSessions?.length, environment.authConfig.singleSessionMode, currentPath, + navigate, + options.rules, + options.additionalContext, ]);As per coding guidelines.
🧹 Nitpick comments (8)
packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx (4)
20-27: Remove unused mock export.
isDevelopmentModeis mocked but never used in the hook or tests; drop it to reduce noise.
83-101: Assert redirecting state when navigation is skipped.Add an assertion that
isRedirectingstaysfalsewhenskipNavigationis true to lock behavior.renderHook(() => useAuthRedirect({ rules: [], additionalContext: {}, }), ); await waitFor(() => { expect(mockNavigate).not.toHaveBeenCalled(); + // Ensure we don't show a loading state when navigation is skipped + // by checking the latest hook state. + // Re-render with access to result if needed: + // const { result } = renderHook(...); + // expect(result.current.isRedirecting).toBe(false); });
148-169: Add tests for option changes (rules/context) to prevent stale evaluations.Currently only auth state changes are covered. Add cases that re-run the effect when
rulesoradditionalContextchange.- const { rerender } = renderHook(() => - useAuthRedirect({ - rules: [], - additionalContext: {}, - }), - ); + const { rerender } = renderHook( + (opts: Parameters<typeof useAuthRedirect>[0]) => useAuthRedirect(opts), + { initialProps: { rules: [], additionalContext: {} } }, + ); expect(evaluateRedirectRules).toHaveBeenCalledTimes(1); // Change auth state (useClerk as any).mockReturnValue({ ...mockClerk, isSignedIn: true, }); rerender({ rules: [], additionalContext: {} }); await waitFor(() => { expect(evaluateRedirectRules).toHaveBeenCalledTimes(2); }); + // Change rules + rerender({ rules: [() => ({ destination: '/x', reason: 'r' })], additionalContext: {} }); + await waitFor(() => { + expect(evaluateRedirectRules).toHaveBeenCalledTimes(3); + }); + + // Change additionalContext + rerender({ rules: [], additionalContext: { afterSignInUrl: '/y' } }); + await waitFor(() => { + expect(evaluateRedirectRules).toHaveBeenCalledTimes(4); + });As per coding guidelines.
29-51: Prefer typed mocks overas any.Use
vi.mocked(...)to avoidanycasts and get type inference on mock returns.- (useClerk as any).mockReturnValue(mockClerk); - (useEnvironment as any).mockReturnValue(mockEnvironment); - (useRouter as any).mockReturnValue({ navigate: mockNavigate, currentPath: '/sign-in' }); - (evaluateRedirectRules as any).mockReturnValue(null); + vi.mocked(useClerk).mockReturnValue(mockClerk); + vi.mocked(useEnvironment).mockReturnValue(mockEnvironment); + vi.mocked(useRouter).mockReturnValue({ navigate: mockNavigate, currentPath: '/sign-in' } as any); + vi.mocked(evaluateRedirectRules).mockReturnValue(null as any);Also applies to: 45-51
packages/clerk-js/src/ui/hooks/useAuthRedirect.ts (1)
9-12: Prefer readonly inputs for rules; tighten API surface.Mark
rulesas readonly to prevent accidental mutation from consumers.-export interface UseAuthRedirectOptions<C extends Record<string, unknown> = Record<string, unknown>> { - rules: RedirectRule[]; +export interface UseAuthRedirectOptions<C extends Record<string, unknown> = Record<string, unknown>> { + readonly rules: ReadonlyArray<RedirectRule>; additionalContext?: C; }Also applies to: 23-26
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx (1)
524-528: Comment and deps mismatch; initialize once or update comment.The effect runs on every
currentPathchange but the comment says “after first render.” Make it run once, or update the comment.-// Mark as initialized after first render, reset when path changes back to root -useEffect(() => { - hasInitializedRef.current = true; -}, [currentPath]); +// Mark as initialized after first render +useEffect(() => { + hasInitializedRef.current = true; + // eslint-disable-next-line react-hooks/exhaustive-deps +}, []);packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts (1)
293-316: ExerciseonRedirectcleanup for add-account flow.Add an assertion that invoking
onRedirectremoves__clerk_add_accountfrom the URL.const result = evaluateRedirectRules(signInRedirectRules, context); expect(result).toMatchObject({ destination: '/sign-in', reason: 'User is adding account', skipNavigation: true, }); expect(result?.onRedirect).toBeDefined(); + + // Simulate client env and run cleanup + const origLoc = window.location.href; + window.history.pushState({}, '', '/sign-in?__clerk_add_account=true&x=1'); + result?.onRedirect?.(); + expect(window.location.search).toBe('?x=1'); + window.history.replaceState({}, '', origLoc);packages/clerk-js/src/ui/utils/redirectRules.ts (1)
44-69: Use readonly arrays; lock rule table.Prevent accidental mutation by using
ReadonlyArray<RedirectRule>in signatures and const assertions onsignInRedirectRules.-export function evaluateRedirectRules(rules: RedirectRule[], context: RedirectContext): RedirectResult | null { +export function evaluateRedirectRules(rules: ReadonlyArray<RedirectRule>, context: RedirectContext): RedirectResult | null { ... } -export const signInRedirectRules: RedirectRule[] = [ +export const signInRedirectRules = [ // ... -]; +] as const satisfies ReadonlyArray<RedirectRule>;Also applies to: 74-136
📜 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/clerk-js/src/ui/components/SignIn/SignInStart.tsx(5 hunks)packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx(1 hunks)packages/clerk-js/src/ui/hooks/useAuthRedirect.ts(1 hunks)packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts(1 hunks)packages/clerk-js/src/ui/utils/redirectRules.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
packages/clerk-js/src/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/clerk-js-ui.mdc)
packages/clerk-js/src/ui/**/*.{ts,tsx}: Element descriptors should always be camelCase
Use element descriptors in UI components to enable consistent theming and styling via appearance.elements
Element descriptors should generate unique, stable CSS classes for theming
Element descriptors should handle state classes (e.g., cl-loading, cl-active, cl-error, cl-open) automatically based on component state
Do not render hard-coded values; all user-facing strings must be localized using provided localization methods
Use the useLocalizations hook and localizationKeys utility for all text and error messages
Use the styled system (sx prop, theme tokens, responsive values) for custom component styling
Use useCardState for card-level state, useFormState for form-level state, and useLoadingStatus for loading states
Always use handleError utility for API errors and use translateError for localized error messages
Use useFormControl for form field state, implement proper validation, and handle loading and error states in forms
Use localization keys for all form labels and placeholders
Use element descriptors for consistent styling and follow the theme token system
Use the Card and FormContainer patterns for consistent UI structure
Files:
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsxpackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/hooks/useAuthRedirect.tspackages/clerk-js/src/ui/utils/redirectRules.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
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
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsxpackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/hooks/useAuthRedirect.tspackages/clerk-js/src/ui/utils/redirectRules.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsxpackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/hooks/useAuthRedirect.tspackages/clerk-js/src/ui/utils/redirectRules.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsxpackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/hooks/useAuthRedirect.tspackages/clerk-js/src/ui/utils/redirectRules.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsxpackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/hooks/useAuthRedirect.tspackages/clerk-js/src/ui/utils/redirectRules.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{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
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsxpackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/hooks/useAuthRedirect.tspackages/clerk-js/src/ui/utils/redirectRules.ts
packages/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Unit tests should use Jest or Vitest as the test runner.
Files:
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx
packages/{clerk-js,elements,themes}/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Visual regression testing should be performed for UI components.
Files:
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsxpackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/hooks/useAuthRedirect.tspackages/clerk-js/src/ui/utils/redirectRules.ts
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/__tests__/**/*.{ts,tsx}: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces
Files:
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{jsx,tsx}: Use error boundaries in React components
Minimize re-renders in React components
**/*.{jsx,tsx}: Always use functional components with hooks instead of class components
Follow PascalCase naming for components: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
Use useState for simple state management
Use useReducer for complex state logic
Implement proper state initialization
Use proper state updates with callbacks
Implement proper state cleanup
Use Context API for theme/authentication
Implement proper state selectors
Use proper state normalization
Implement proper state persistence
Use React.memo for expensive components
Implement proper useCallback for handlers
Use proper useMemo for expensive computations
Implement proper virtualization for lists
Use proper code splitting with React.lazy
Implement proper cleanup in useEffect
Use proper refs for DOM access
Implement proper event listener cleanup
Use proper abort controllers for fetch
Implement proper subscription cleanup
Use proper HTML elements
Implement proper ARIA attributes
Use proper heading hierarchy
Implement proper form labels
Use proper button types
Implement proper focus management
Use proper keyboard shortcuts
Implement proper tab order
Use proper skip links
Implement proper focus traps
Implement proper error boundaries
Use proper error logging
Implement proper error recovery
Use proper error messages
Implement proper error fallbacks
Use proper form validation
Implement proper error states
Use proper error messages
Implement proper form submission
Use proper form reset
Use proper component naming
Implement proper file naming
Use proper prop naming
Implement proper...
Files:
packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsxpackages/clerk-js/src/ui/components/SignIn/SignInStart.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.tsx: Use proper type definitions for props and state
Leverage TypeScript's type inference where possible
Use proper event types for handlers
Implement proper generic types for reusable components
Use proper type guards for conditional rendering
Files:
packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsxpackages/clerk-js/src/ui/components/SignIn/SignInStart.tsx
**/*.test.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.test.{jsx,tsx}: Use React Testing Library
Test component behavior, not implementation
Use proper test queries
Implement proper test isolation
Use proper test coverage
Test component interactions
Use proper test data
Implement proper test setup
Use proper test cleanup
Implement proper test assertions
Use proper test structure
Files:
packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx
🧬 Code graph analysis (5)
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts (2)
packages/clerk-js/src/ui/utils/redirectRules.ts (4)
RedirectContext(5-14)evaluateRedirectRules(44-69)StopRedirectEvaluation(29-34)signInRedirectRules(74-136)packages/types/src/environment.ts (1)
EnvironmentResource(10-24)
packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx (3)
packages/types/src/environment.ts (1)
EnvironmentResource(10-24)packages/clerk-js/src/ui/utils/redirectRules.ts (1)
evaluateRedirectRules(44-69)packages/clerk-js/src/ui/hooks/useAuthRedirect.ts (1)
useAuthRedirect(23-62)
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx (5)
packages/clerk-js/src/ui/router/RouteContext.tsx (1)
useRouter(33-39)packages/clerk-js/src/ui/contexts/components/SignIn.ts (1)
useSignInContext(38-169)packages/clerk-js/src/ui/hooks/useAuthRedirect.ts (1)
useAuthRedirect(23-62)packages/clerk-js/src/ui/utils/redirectRules.ts (1)
signInRedirectRules(74-136)packages/clerk-js/src/ui/elements/contexts/index.tsx (1)
withCardStateProvider(72-81)
packages/clerk-js/src/ui/hooks/useAuthRedirect.ts (1)
packages/clerk-js/src/ui/utils/redirectRules.ts (2)
RedirectRule(23-23)evaluateRedirectRules(44-69)
packages/clerk-js/src/ui/utils/redirectRules.ts (2)
packages/types/src/environment.ts (1)
EnvironmentResource(10-24)packages/clerk-js/src/utils/debug.ts (1)
debugLogger(150-179)
⏰ 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). (4)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (2)
packages/clerk-js/src/ui/utils/redirectRules.ts (1)
95-116: SSR safety already guarded—nice.The
typeof window !== 'undefined' && window.historycheck aroundonRedirectis correct.packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx (1)
513-522: No action needed—queryParams type is already correct.The code properly types
queryParamsasRecord<string, string>throughout the entire chain:
getQueryParamsreturnsRecord<string, string>(explicitly cast)RedirectContextinterface definesqueryParams?: Record<string, string>signInRedirectRulesaccesses it as an object (ctx.queryParams?.['__clerk_add_account'])- SignInStart passes
queryParamsfromuseRouter(), which is alreadyRecord<string, string>The internal conversion from
URLSearchParamsto a plain object happens insidegetQueryParams, not at the call site. NoObject.fromEntries()is needed.
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: 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 (3)
integration/templates/next-app-router/next.config.js(1 hunks)packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx(5 hunks)packages/clerk-js/src/ui/hooks/useAuthRedirect.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/clerk-js/src/ui/hooks/useAuthRedirect.ts
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
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
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
integration/templates/next-app-router/next.config.jspackages/clerk-js/src/ui/components/SignIn/SignInStart.tsx
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
integration/templates/next-app-router/next.config.jspackages/clerk-js/src/ui/components/SignIn/SignInStart.tsx
integration/**
📄 CodeRabbit inference engine (.cursor/rules/global.mdc)
Framework integration templates and E2E tests should be placed under the integration/ directory
Files:
integration/templates/next-app-router/next.config.js
integration/**/*
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
End-to-end tests and integration templates must be located in the 'integration/' directory.
Files:
integration/templates/next-app-router/next.config.js
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
integration/templates/next-app-router/next.config.jspackages/clerk-js/src/ui/components/SignIn/SignInStart.tsx
packages/clerk-js/src/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/clerk-js-ui.mdc)
packages/clerk-js/src/ui/**/*.{ts,tsx}: Element descriptors should always be camelCase
Use element descriptors in UI components to enable consistent theming and styling via appearance.elements
Element descriptors should generate unique, stable CSS classes for theming
Element descriptors should handle state classes (e.g., cl-loading, cl-active, cl-error, cl-open) automatically based on component state
Do not render hard-coded values; all user-facing strings must be localized using provided localization methods
Use the useLocalizations hook and localizationKeys utility for all text and error messages
Use the styled system (sx prop, theme tokens, responsive values) for custom component styling
Use useCardState for card-level state, useFormState for form-level state, and useLoadingStatus for loading states
Always use handleError utility for API errors and use translateError for localized error messages
Use useFormControl for form field state, implement proper validation, and handle loading and error states in forms
Use localization keys for all form labels and placeholders
Use element descriptors for consistent styling and follow the theme token system
Use the Card and FormContainer patterns for consistent UI structure
Files:
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{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
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{jsx,tsx}: Use error boundaries in React components
Minimize re-renders in React components
**/*.{jsx,tsx}: Always use functional components with hooks instead of class components
Follow PascalCase naming for components: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
Use useState for simple state management
Use useReducer for complex state logic
Implement proper state initialization
Use proper state updates with callbacks
Implement proper state cleanup
Use Context API for theme/authentication
Implement proper state selectors
Use proper state normalization
Implement proper state persistence
Use React.memo for expensive components
Implement proper useCallback for handlers
Use proper useMemo for expensive computations
Implement proper virtualization for lists
Use proper code splitting with React.lazy
Implement proper cleanup in useEffect
Use proper refs for DOM access
Implement proper event listener cleanup
Use proper abort controllers for fetch
Implement proper subscription cleanup
Use proper HTML elements
Implement proper ARIA attributes
Use proper heading hierarchy
Implement proper form labels
Use proper button types
Implement proper focus management
Use proper keyboard shortcuts
Implement proper tab order
Use proper skip links
Implement proper focus traps
Implement proper error boundaries
Use proper error logging
Implement proper error recovery
Use proper error messages
Implement proper error fallbacks
Use proper form validation
Implement proper error states
Use proper error messages
Implement proper form submission
Use proper form reset
Use proper component naming
Implement proper file naming
Use proper prop naming
Implement proper...
Files:
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.tsx: Use proper type definitions for props and state
Leverage TypeScript's type inference where possible
Use proper event types for handlers
Implement proper generic types for reusable components
Use proper type guards for conditional rendering
Files:
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx
🧬 Code graph analysis (2)
integration/templates/next-app-router/next.config.js (1)
scripts/renovate-config-generator.mjs (1)
__dirname(8-8)
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx (5)
packages/clerk-js/src/ui/router/RouteContext.tsx (1)
useRouter(33-39)packages/clerk-js/src/ui/contexts/components/SignIn.ts (1)
useSignInContext(38-169)packages/clerk-js/src/ui/hooks/useAuthRedirect.ts (1)
useAuthRedirect(25-69)packages/clerk-js/src/ui/utils/redirectRules.ts (1)
signInRedirectRules(74-136)packages/clerk-js/src/ui/elements/contexts/index.tsx (1)
withCardStateProvider(72-81)
⏰ 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 (5)
integration/templates/next-app-router/next.config.js (3)
1-2: LGTM! Path import correctly added.The
pathmodule import is necessary for thepath.resolve()call on line 8 and follows proper CommonJS syntax.
8-8: Good improvement for monorepo file tracing.Changing from a hardcoded
'/'topath.resolve(__dirname, '../../../')correctly points to the monorepo root, enabling Next.js to properly trace dependencies across workspace packages.
9-16: LGTM! Webpack watch configuration improves stability.The custom
watchOptionsappropriately exclude directories that can cause permission errors or unnecessary watch overhead. The spread operator correctly preserves existing watch settings, and the explanatory comment is helpful.Note: The
**/.Trash/**pattern is macOS-specific but won't cause issues on other platforms.packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx (2)
513-528: LGTM! Redirect integration is clean and SSR-safe.The
useAuthRedirecthook integration correctly replaces the previous HOC wrappers. The memoizedredirectAdditionalContextprevents unnecessary re-evaluations, and usingqueryParamsfromuseRouter(rather than directwindowaccess) ensures SSR compatibility.
719-719: LGTM! Export simplification aligns with centralized redirect approach.Moving from a complex HOC chain to a single
withCardStateProviderwrapper is a solid refactor. The redirect logic is now centralized in theuseAuthRedirecthook, making the flow easier to reason about and test.
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 (4)
packages/clerk-js/src/ui/utils/redirectRules.ts (2)
28-36: Consider edge case: empty string URL.The
isValidRedirectUrlhelper correctly validates relative paths and same-origin URLs, but an empty string""will returntrue(since it starts with... well, nothing matches/, butnew URL("", origin)resolves to origin). While this may be intentional, consider explicitly handling empty strings if they should be invalid.Apply this diff if empty URLs should be rejected:
function isValidRedirectUrl(url: string): boolean { + if (!url) return false; try { if (url.startsWith('/')) return true; const parsed = new URL(url, window.location.origin); return parsed.origin === window.location.origin; } catch { return false; } }
75-141: Verify 'choose' destination consistency.Line 133 uses
destination: 'choose'(a relative path). While this may be resolved by the router relative to the current sign-in path, using an absolute path like'/sign-in/choose'would be more explicit and less dependent on router context resolution logic.If absolute paths are preferred for clarity, apply:
return [ { - destination: 'choose', + destination: '/sign-in/choose', reason: 'Active sessions detected (multi-session mode)', }, false, ];packages/clerk-js/src/ui/hooks/useRedirectEngine.ts (2)
18-21: Enhance JSDoc documentation.While marked as
@internal, the documentation should still explain what this hook does, its parameters, return value, and the generic type parameter. This helps maintainers understand the implementation.Consider expanding the JSDoc:
/** - * Internal redirect engine - use dedicated hooks like useSignInRedirect instead + * Internal redirect engine that evaluates redirect rules and performs navigation. + * Use dedicated hooks like useSignInRedirect or useSignUpRedirect instead of calling this directly. + * + * @template C - Additional context type extending Record<string, unknown> + * @param options - Configuration object with redirect rules and optional additional context + * @param options.rules - Array of redirect rules to evaluate in order + * @param options.additionalContext - Additional context data to merge with base redirect context + * @returns Object containing isRedirecting state indicating whether a redirect is in progress * @internal */
30-65: Consider adding cleanup to prevent state updates after unmount.The effect performs asynchronous navigation without tracking whether the component is still mounted. This can cause React warnings about state updates on unmounted components.
Add a cleanup function to track mounted state:
React.useEffect(() => { + let isMounted = true; + const context = { clerk, currentPath, environment, ...options.additionalContext, } as RedirectContext & C; const result = evaluateRedirectRules(options.rules, context); if (result) { if (result.cleanupQueryParams && typeof window !== 'undefined' && window.history) { const params = new URLSearchParams(window.location.search); result.cleanupQueryParams.forEach(param => params.delete(param)); const newSearch = params.toString(); const newUrl = window.location.pathname + (newSearch ? `?${newSearch}` : ''); window.history.replaceState({}, '', newUrl); } if (!result.skipNavigation) { - setIsRedirecting(true); - void navigate(result.destination); + if (isMounted) setIsRedirecting(true); + navigate(result.destination).catch(err => { + console.error('Navigation failed:', err); + if (isMounted) setIsRedirecting(false); + }); } } else { - setIsRedirecting(false); + if (isMounted) setIsRedirecting(false); } + + return () => { + isMounted = false; + }; }, [ // ... dependencies ]);
📜 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 (10)
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx(4 hunks)packages/clerk-js/src/ui/hooks/__tests__/useRedirectEngine.test.tsx(1 hunks)packages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsx(1 hunks)packages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsx(1 hunks)packages/clerk-js/src/ui/hooks/index.ts(1 hunks)packages/clerk-js/src/ui/hooks/useRedirectEngine.ts(1 hunks)packages/clerk-js/src/ui/hooks/useSignInRedirect.ts(1 hunks)packages/clerk-js/src/ui/hooks/useSignUpRedirect.ts(1 hunks)packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts(1 hunks)packages/clerk-js/src/ui/utils/redirectRules.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/clerk-js/src/ui/hooks/index.ts
🧰 Additional context used
📓 Path-based instructions (13)
packages/clerk-js/src/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/clerk-js-ui.mdc)
packages/clerk-js/src/ui/**/*.{ts,tsx}: Element descriptors should always be camelCase
Use element descriptors in UI components to enable consistent theming and styling via appearance.elements
Element descriptors should generate unique, stable CSS classes for theming
Element descriptors should handle state classes (e.g., cl-loading, cl-active, cl-error, cl-open) automatically based on component state
Do not render hard-coded values; all user-facing strings must be localized using provided localization methods
Use the useLocalizations hook and localizationKeys utility for all text and error messages
Use the styled system (sx prop, theme tokens, responsive values) for custom component styling
Use useCardState for card-level state, useFormState for form-level state, and useLoadingStatus for loading states
Always use handleError utility for API errors and use translateError for localized error messages
Use useFormControl for form field state, implement proper validation, and handle loading and error states in forms
Use localization keys for all form labels and placeholders
Use element descriptors for consistent styling and follow the theme token system
Use the Card and FormContainer patterns for consistent UI structure
Files:
packages/clerk-js/src/ui/hooks/useSignUpRedirect.tspackages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsxpackages/clerk-js/src/ui/hooks/useSignInRedirect.tspackages/clerk-js/src/ui/hooks/useRedirectEngine.tspackages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsxpackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/hooks/__tests__/useRedirectEngine.test.tsxpackages/clerk-js/src/ui/utils/redirectRules.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
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
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/clerk-js/src/ui/hooks/useSignUpRedirect.tspackages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsxpackages/clerk-js/src/ui/hooks/useSignInRedirect.tspackages/clerk-js/src/ui/hooks/useRedirectEngine.tspackages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsxpackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/hooks/__tests__/useRedirectEngine.test.tsxpackages/clerk-js/src/ui/utils/redirectRules.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/clerk-js/src/ui/hooks/useSignUpRedirect.tspackages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsxpackages/clerk-js/src/ui/hooks/useSignInRedirect.tspackages/clerk-js/src/ui/hooks/useRedirectEngine.tspackages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsxpackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/hooks/__tests__/useRedirectEngine.test.tsxpackages/clerk-js/src/ui/utils/redirectRules.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/ui/hooks/useSignUpRedirect.tspackages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsxpackages/clerk-js/src/ui/hooks/useSignInRedirect.tspackages/clerk-js/src/ui/hooks/useRedirectEngine.tspackages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsxpackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/hooks/__tests__/useRedirectEngine.test.tsxpackages/clerk-js/src/ui/utils/redirectRules.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/clerk-js/src/ui/hooks/useSignUpRedirect.tspackages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsxpackages/clerk-js/src/ui/hooks/useSignInRedirect.tspackages/clerk-js/src/ui/hooks/useRedirectEngine.tspackages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsxpackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/hooks/__tests__/useRedirectEngine.test.tsxpackages/clerk-js/src/ui/utils/redirectRules.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{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
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/clerk-js/src/ui/hooks/useSignUpRedirect.tspackages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsxpackages/clerk-js/src/ui/hooks/useSignInRedirect.tspackages/clerk-js/src/ui/hooks/useRedirectEngine.tspackages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsxpackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/hooks/__tests__/useRedirectEngine.test.tsxpackages/clerk-js/src/ui/utils/redirectRules.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/clerk-js/src/ui/hooks/useSignUpRedirect.tspackages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsxpackages/clerk-js/src/ui/hooks/useSignInRedirect.tspackages/clerk-js/src/ui/hooks/useRedirectEngine.tspackages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsxpackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/hooks/__tests__/useRedirectEngine.test.tsxpackages/clerk-js/src/ui/utils/redirectRules.ts
packages/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Unit tests should use Jest or Vitest as the test runner.
Files:
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useRedirectEngine.test.tsx
packages/{clerk-js,elements,themes}/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Visual regression testing should be performed for UI components.
Files:
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useRedirectEngine.test.tsx
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/__tests__/**/*.{ts,tsx}: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces
Files:
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.tspackages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useRedirectEngine.test.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{jsx,tsx}: Use error boundaries in React components
Minimize re-renders in React components
**/*.{jsx,tsx}: Always use functional components with hooks instead of class components
Follow PascalCase naming for components: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
Use useState for simple state management
Use useReducer for complex state logic
Implement proper state initialization
Use proper state updates with callbacks
Implement proper state cleanup
Use Context API for theme/authentication
Implement proper state selectors
Use proper state normalization
Implement proper state persistence
Use React.memo for expensive components
Implement proper useCallback for handlers
Use proper useMemo for expensive computations
Implement proper virtualization for lists
Use proper code splitting with React.lazy
Implement proper cleanup in useEffect
Use proper refs for DOM access
Implement proper event listener cleanup
Use proper abort controllers for fetch
Implement proper subscription cleanup
Use proper HTML elements
Implement proper ARIA attributes
Use proper heading hierarchy
Implement proper form labels
Use proper button types
Implement proper focus management
Use proper keyboard shortcuts
Implement proper tab order
Use proper skip links
Implement proper focus traps
Implement proper error boundaries
Use proper error logging
Implement proper error recovery
Use proper error messages
Implement proper error fallbacks
Use proper form validation
Implement proper error states
Use proper error messages
Implement proper form submission
Use proper form reset
Use proper component naming
Implement proper file naming
Use proper prop naming
Implement proper...
Files:
packages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsxpackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/hooks/__tests__/useRedirectEngine.test.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.tsx: Use proper type definitions for props and state
Leverage TypeScript's type inference where possible
Use proper event types for handlers
Implement proper generic types for reusable components
Use proper type guards for conditional rendering
Files:
packages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsxpackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/ui/hooks/__tests__/useRedirectEngine.test.tsx
**/*.test.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.test.{jsx,tsx}: Use React Testing Library
Test component behavior, not implementation
Use proper test queries
Implement proper test isolation
Use proper test coverage
Test component interactions
Use proper test data
Implement proper test setup
Use proper test cleanup
Implement proper test assertions
Use proper test structure
Files:
packages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useRedirectEngine.test.tsx
🧬 Code graph analysis (9)
packages/clerk-js/src/ui/hooks/useSignUpRedirect.ts (2)
packages/clerk-js/src/ui/hooks/useRedirectEngine.ts (1)
useRedirectEngine(22-68)packages/clerk-js/src/ui/utils/redirectRules.ts (1)
signUpRedirectRules(146-166)
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts (2)
packages/clerk-js/src/ui/utils/redirectRules.ts (3)
RedirectContext(6-15)evaluateRedirectRules(45-70)signInRedirectRules(75-141)packages/types/src/environment.ts (1)
EnvironmentResource(10-24)
packages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsx (2)
packages/clerk-js/src/ui/hooks/useSignInRedirect.ts (1)
useSignInRedirect(16-38)packages/clerk-js/src/ui/hooks/useRedirectEngine.ts (1)
useRedirectEngine(22-68)
packages/clerk-js/src/ui/hooks/useSignInRedirect.ts (2)
packages/clerk-js/src/ui/hooks/useRedirectEngine.ts (1)
useRedirectEngine(22-68)packages/clerk-js/src/ui/utils/redirectRules.ts (1)
signInRedirectRules(75-141)
packages/clerk-js/src/ui/hooks/useRedirectEngine.ts (1)
packages/clerk-js/src/ui/utils/redirectRules.ts (3)
RedirectRule(24-26)RedirectContext(6-15)evaluateRedirectRules(45-70)
packages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsx (2)
packages/clerk-js/src/ui/hooks/useSignUpRedirect.ts (1)
useSignUpRedirect(15-30)packages/clerk-js/src/ui/hooks/useRedirectEngine.ts (1)
useRedirectEngine(22-68)
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx (2)
packages/clerk-js/src/ui/hooks/useSignInRedirect.ts (1)
useSignInRedirect(16-38)packages/clerk-js/src/ui/elements/contexts/index.tsx (1)
withCardStateProvider(72-81)
packages/clerk-js/src/ui/hooks/__tests__/useRedirectEngine.test.tsx (3)
packages/types/src/environment.ts (1)
EnvironmentResource(10-24)packages/clerk-js/src/ui/utils/redirectRules.ts (1)
evaluateRedirectRules(45-70)packages/clerk-js/src/ui/hooks/useRedirectEngine.ts (1)
useRedirectEngine(22-68)
packages/clerk-js/src/ui/utils/redirectRules.ts (2)
packages/types/src/environment.ts (1)
EnvironmentResource(10-24)packages/clerk-js/src/utils/debug.ts (1)
debugLogger(150-179)
⏰ 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). (4)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (14)
packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts (2)
1-53: LGTM! Clean evaluation engine tests.The tests properly validate the core evaluation behavior: no match returns null, first matching rule is returned, and short-circuit evaluation via
shouldStopflag works correctly.
175-216: The review comment is incorrect; no changes needed.The tests correctly expect
destination: 'choose'because that's what the actual redirect rule implementation produces. Rule 3 insignInRedirectRulesexplicitly setsdestination: 'choose', and the SignIn component defines a matching route:<Route path='choose'><SignInAccountSwitcher /></Route>. The relative path is intentional—the router resolves it within the component context. The tests accurately validate the correct behavior.Likely an incorrect or invalid review comment.
packages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsx (1)
1-65: LGTM! Clean hook integration tests.The tests properly verify that
useSignUpRedirectdelegates touseRedirectEnginewith the correct rules and context structure, and that the sign-up flow correctly omitshasInitializedRef(unlike sign-in).packages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsx (1)
58-71: LGTM! Initialization tracking correctly tested.The test properly validates that
hasInitializedRefis set totrueafter the first render by capturing the ref from the first call and checking its value after rerender. This aligns with the hook's implementation.packages/clerk-js/src/ui/hooks/useSignUpRedirect.ts (1)
15-30: LGTM! Clean and focused hook.The hook correctly memoizes the additional context and delegates to
useRedirectEngine. The absence ofhasInitializedRefis intentional and appropriate for the sign-up flow, distinguishing it from the sign-in flow.packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx (2)
511-520: LGTM! Clean integration of redirect hook.The
useSignInRedirecthook is properly integrated with the correct options, and the loading state correctly gates rendering when redirecting. This replaces the previous HOC-based redirect pattern with a cleaner hook-based approach.
700-700: LGTM! Simplified export pattern.The export now directly uses
withCardStateProviderinstead of multiple wrapper composition, aligning with the new hook-based redirect architecture. This improves code clarity and maintainability.packages/clerk-js/src/ui/hooks/__tests__/useRedirectEngine.test.tsx (2)
101-129: LGTM! Thorough query param cleanup test.The test properly mocks
window.historyandwindow.location, verifies thatcleanupQueryParamstriggersreplaceState, and confirms the cleaned URL structure. This validates the declarative cleanup feature.
157-178: LGTM! Auth state reactivity validated.The test correctly verifies that the redirect engine re-evaluates rules when clerk auth state changes, ensuring the effect's dependency array is properly configured.
packages/clerk-js/src/ui/hooks/useSignInRedirect.ts (1)
16-38: LGTM! Well-structured sign-in redirect hook.The hook correctly:
- Tracks initialization with a ref set on mount
- Memoizes the additional context with proper dependencies
- Delegates to the redirect engine with sign-in-specific rules
The initialization tracking via
hasInitializedRefenables the multi-session redirect rule to distinguish first render from subsequent renders.packages/clerk-js/src/ui/utils/redirectRules.ts (2)
84-102: LGTM! Single-session redirect with URL validation.The rule correctly:
- Checks if user is signed in AND in single-session mode
- Falls back to
buildAfterSignInUrl()ifafterSignInUrlis invalid- Returns appropriate destination and reason
104-119: LGTM! Add account flow with query param cleanup.The rule properly detects the
__clerk_add_accountquery parameter, returns the current path withskipNavigation: true, and declares the param for cleanup. This preserves the add-account flow while cleaning up the URL.packages/clerk-js/src/ui/hooks/useRedirectEngine.ts (2)
9-16: LGTM!The type definitions are well-structured, use appropriate generic constraints, and follow TypeScript best practices with named exports.
56-65: The review comment is incorrect.The callers of
useRedirectEngineproperly memoize the dependencies. InuseSignUpRedirect,additionalContextis memoized withuseMemowith dependencies[options.afterSignUpUrl, queryParams], and inuseSignInRedirect,additionalContextis also memoized withuseMemowith dependencies[options.afterSignInUrl, options.organizationTicket, queryParams]. Additionally, therulespassed are constants imported at the top level (signUpRedirectRulesandsignInRedirectRules), which are stable references. The dependency array inuseRedirectEngineis therefore safe and will not cause infinite re-renders.Likely an incorrect or invalid review comment.
| const context = { | ||
| clerk, | ||
| currentPath, | ||
| environment, | ||
| ...options.additionalContext, | ||
| } as RedirectContext & C; |
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.
Type assertion may allow unsafe key collisions.
The type assertion at line 36 combined with spreading additionalContext could allow runtime issues if additionalContext contains keys that overlap with RedirectContext properties (e.g., clerk, currentPath, environment). TypeScript won't catch this due to the assertion.
Consider either:
- Document that
additionalContextmust not contain base context keys - Use a more defensive merge that preserves base context:
- const context = {
- clerk,
- currentPath,
- environment,
- ...options.additionalContext,
- } as RedirectContext & C;
+ const baseContext: RedirectContext = {
+ clerk,
+ currentPath,
+ environment,
+ };
+ const context = {
+ ...options.additionalContext,
+ ...baseContext,
+ } as RedirectContext & C;Or add a runtime check in development mode to warn about key collisions.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const context = { | |
| clerk, | |
| currentPath, | |
| environment, | |
| ...options.additionalContext, | |
| } as RedirectContext & C; | |
| const baseContext: RedirectContext = { | |
| clerk, | |
| currentPath, | |
| environment, | |
| }; | |
| const context = { | |
| ...options.additionalContext, | |
| ...baseContext, | |
| } as RedirectContext & C; |
| if (result) { | ||
| if (result.cleanupQueryParams && typeof window !== 'undefined' && window.history) { | ||
| const params = new URLSearchParams(window.location.search); | ||
| result.cleanupQueryParams.forEach(param => params.delete(param)); | ||
| const newSearch = params.toString(); | ||
| const newUrl = window.location.pathname + (newSearch ? `?${newSearch}` : ''); | ||
| window.history.replaceState({}, '', newUrl); | ||
| } | ||
|
|
||
| if (!result.skipNavigation) { | ||
| setIsRedirecting(true); | ||
| void navigate(result.destination); | ||
| } | ||
| } else { | ||
| setIsRedirecting(false); | ||
| } |
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.
Fix state management and add error handling for navigation.
Two issues:
-
State management: When
result.skipNavigationis true,isRedirectingremains in its previous state instead of being set tofalse. This can cause stale loading indicators. -
No error handling: Navigation failures are silently ignored due to the
voidoperator. Users won't be notified if navigation fails.
Apply this diff to fix both issues:
if (result) {
if (result.cleanupQueryParams && typeof window !== 'undefined' && window.history) {
const params = new URLSearchParams(window.location.search);
result.cleanupQueryParams.forEach(param => params.delete(param));
const newSearch = params.toString();
const newUrl = window.location.pathname + (newSearch ? `?${newSearch}` : '');
window.history.replaceState({}, '', newUrl);
}
if (!result.skipNavigation) {
setIsRedirecting(true);
- void navigate(result.destination);
+ navigate(result.destination).catch(err => {
+ console.error('Navigation failed:', err);
+ setIsRedirecting(false);
+ });
+ } else {
+ setIsRedirecting(false);
}
} else {
setIsRedirecting(false);
}🤖 Prompt for AI Agents
In packages/clerk-js/src/ui/hooks/useRedirectEngine.ts around lines 40 to 55,
the code leaves isRedirecting unchanged when result.skipNavigation is true and
swallows navigation errors via the void operator; update logic so that if
result.skipNavigation is true you explicitly call setIsRedirecting(false), and
replace the void navigate(...) with promise handling that sets
setIsRedirecting(false) on both success and failure (e.g., navigate(...).then(()
=> setIsRedirecting(false)).catch(err => { setIsRedirecting(false); /* log or
surface err */ })), and ensure the else branch for a falsy result still sets
isRedirecting(false).
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: 1
🧹 Nitpick comments (1)
packages/clerk-js/src/ui/utils/redirectRules.ts (1)
24-26: Add JSDoc to document the tuple structure.The
RedirectRuletype returns a tuple[RedirectResult | null, boolean]where the boolean indicates whether to stop further rule evaluation. This is not immediately clear from the type signature alone.Apply this diff:
+/** + * A redirect rule that evaluates context and returns a result with a stop flag. + * + * @param context - The redirect context including clerk instance, path, and additional properties + * @returns A tuple of [result, shouldStop] where: + * - result: The redirect result if the rule matches, null otherwise + * - shouldStop: If true, stops evaluation of subsequent rules regardless of result + */ export type RedirectRule<T extends Record<string, unknown> = Record<string, unknown>> = ( context: RedirectContext & T, ) => readonly [RedirectResult | null, boolean];As per coding guidelines.
📜 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 (3)
packages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsx(1 hunks)packages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsx(1 hunks)packages/clerk-js/src/ui/utils/redirectRules.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
packages/clerk-js/src/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/clerk-js-ui.mdc)
packages/clerk-js/src/ui/**/*.{ts,tsx}: Element descriptors should always be camelCase
Use element descriptors in UI components to enable consistent theming and styling via appearance.elements
Element descriptors should generate unique, stable CSS classes for theming
Element descriptors should handle state classes (e.g., cl-loading, cl-active, cl-error, cl-open) automatically based on component state
Do not render hard-coded values; all user-facing strings must be localized using provided localization methods
Use the useLocalizations hook and localizationKeys utility for all text and error messages
Use the styled system (sx prop, theme tokens, responsive values) for custom component styling
Use useCardState for card-level state, useFormState for form-level state, and useLoadingStatus for loading states
Always use handleError utility for API errors and use translateError for localized error messages
Use useFormControl for form field state, implement proper validation, and handle loading and error states in forms
Use localization keys for all form labels and placeholders
Use element descriptors for consistent styling and follow the theme token system
Use the Card and FormContainer patterns for consistent UI structure
Files:
packages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsxpackages/clerk-js/src/ui/utils/redirectRules.tspackages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
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
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsxpackages/clerk-js/src/ui/utils/redirectRules.tspackages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsx
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsxpackages/clerk-js/src/ui/utils/redirectRules.tspackages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsx
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsxpackages/clerk-js/src/ui/utils/redirectRules.tspackages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsx
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsxpackages/clerk-js/src/ui/utils/redirectRules.tspackages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{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
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsxpackages/clerk-js/src/ui/utils/redirectRules.tspackages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{jsx,tsx}: Use error boundaries in React components
Minimize re-renders in React components
**/*.{jsx,tsx}: Always use functional components with hooks instead of class components
Follow PascalCase naming for components: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
Use useState for simple state management
Use useReducer for complex state logic
Implement proper state initialization
Use proper state updates with callbacks
Implement proper state cleanup
Use Context API for theme/authentication
Implement proper state selectors
Use proper state normalization
Implement proper state persistence
Use React.memo for expensive components
Implement proper useCallback for handlers
Use proper useMemo for expensive computations
Implement proper virtualization for lists
Use proper code splitting with React.lazy
Implement proper cleanup in useEffect
Use proper refs for DOM access
Implement proper event listener cleanup
Use proper abort controllers for fetch
Implement proper subscription cleanup
Use proper HTML elements
Implement proper ARIA attributes
Use proper heading hierarchy
Implement proper form labels
Use proper button types
Implement proper focus management
Use proper keyboard shortcuts
Implement proper tab order
Use proper skip links
Implement proper focus traps
Implement proper error boundaries
Use proper error logging
Implement proper error recovery
Use proper error messages
Implement proper error fallbacks
Use proper form validation
Implement proper error states
Use proper error messages
Implement proper form submission
Use proper form reset
Use proper component naming
Implement proper file naming
Use proper prop naming
Implement proper...
Files:
packages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsx
packages/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Unit tests should use Jest or Vitest as the test runner.
Files:
packages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsx
packages/{clerk-js,elements,themes}/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Visual regression testing should be performed for UI components.
Files:
packages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsx
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsxpackages/clerk-js/src/ui/utils/redirectRules.tspackages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.tsx: Use proper type definitions for props and state
Leverage TypeScript's type inference where possible
Use proper event types for handlers
Implement proper generic types for reusable components
Use proper type guards for conditional rendering
Files:
packages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsx
**/*.test.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.test.{jsx,tsx}: Use React Testing Library
Test component behavior, not implementation
Use proper test queries
Implement proper test isolation
Use proper test coverage
Test component interactions
Use proper test data
Implement proper test setup
Use proper test cleanup
Implement proper test assertions
Use proper test structure
Files:
packages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsx
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/__tests__/**/*.{ts,tsx}: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces
Files:
packages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsxpackages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsx
🧬 Code graph analysis (3)
packages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsx (2)
packages/clerk-js/src/ui/hooks/useSignUpRedirect.ts (1)
useSignUpRedirect(15-30)packages/clerk-js/src/ui/hooks/useRedirectEngine.ts (1)
useRedirectEngine(22-68)
packages/clerk-js/src/ui/utils/redirectRules.ts (2)
packages/types/src/environment.ts (1)
EnvironmentResource(10-24)packages/clerk-js/src/utils/debug.ts (1)
debugLogger(150-179)
packages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsx (2)
packages/clerk-js/src/ui/hooks/useSignInRedirect.ts (1)
useSignInRedirect(16-38)packages/clerk-js/src/ui/hooks/useRedirectEngine.ts (1)
useRedirectEngine(22-68)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (16)
packages/clerk-js/src/ui/utils/redirectRules.ts (6)
1-4: LGTM: Clean imports with proper type-only usage.The imports follow TypeScript best practices with type-only imports where appropriate.
6-15: LGTM: Well-typed context interface.The interface now uses explicit optional properties instead of an index signature, improving type safety. The use of
readonlyfollows best practices for immutable data structures.
17-22: LGTM: Clear result interface.The interface clearly defines the shape of redirect results with appropriate optional flags for special behaviors.
40-72: LGTM: Well-implemented rule evaluator.The function correctly implements sequential rule evaluation with early stopping, includes comprehensive JSDoc, and uses appropriate logging for debugging.
148-168: LGTM: Sign-up redirect rules are appropriate.The sign-up rules correctly implement single-session redirect with URL validation, mirroring the sign-in flow where applicable. The absence of multi-session and organization ticket rules is appropriate for the sign-up context.
124-142: No issues found. The relative path'choose'is correct and intentionally designed.The
destination: 'choose'on line 135 is the correct relative path format. This is confirmed by:
- The route definition in
SignIn/index.tsxline 77 defines<Route path='choose'>for the account switcher- Tests in
redirectRules.test.tslines 192 and 213 verify this exact path works as expected- The router handles relative paths within the flow context correctly
The previous review comment suggesting a change to
'/sign-in/choose'appears to have been based on outdated or incorrect assumptions. The current implementation is consistent and properly tested.packages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsx (5)
1-19: LGTM: Proper test setup with clean mocks.The test correctly mocks dependencies before importing the module under test, following Vitest best practices. Using an empty array for
signUpRedirectRulesappropriately isolates the hook's behavior from the rule implementation.
21-25: LGTM: Proper test isolation setup.The
beforeEachhook correctly resets mocks and configures test data, ensuring each test runs in isolation.
27-41: LGTM: Thorough verification of hook delegation.The test correctly verifies that the hook passes the appropriate rules and context to
useRedirectEngine, validating the hook's core responsibility.
43-53: LGTM: Return value delegation is tested.The test verifies that the hook correctly exposes the
isRedirectingstate from the redirect engine.
55-64: LGTM: Documents SignUp-specific behavior.This test validates an important distinction: unlike SignIn, the SignUp flow doesn't include
hasInitializedRefbecause it doesn't need to track initialization for multi-session account switching.packages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsx (5)
1-19: LGTM: Consistent test setup pattern.The test setup follows the same clean mocking pattern as the SignUp redirect tests, properly isolating the hook's behavior.
21-25: LGTM: Proper test isolation.The setup correctly ensures test independence with mock cleanup and configuration.
27-44: LGTM: Complete context validation.The test correctly verifies that the SignIn-specific context properties (including
hasInitializedRef,organizationTicket) are passed to the redirect engine.
46-56: LGTM: Return value verified.The test confirms the hook properly exposes the
isRedirectingstate.
58-71: LGTM: Validates initialization timing.This test correctly verifies that
hasInitializedRefis set totrueafter the first render viauseEffect, which is crucial for preventing premature multi-session redirects.
| function isValidRedirectUrl(url: string): boolean { | ||
| try { | ||
| if (url.startsWith('/')) { | ||
| return true; | ||
| } | ||
| const parsed = new URL(url, window.location.origin); | ||
| return parsed.origin === window.location.origin; | ||
| } catch { | ||
| return false; | ||
| } | ||
| } |
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.
Add SSR safety guard for window access.
The function directly accesses window.location.origin without checking if window exists, which will throw a ReferenceError in SSR environments. While the current usage context (inside useEffect) is client-side, the function is exported and could be called from other contexts.
Apply this diff:
function isValidRedirectUrl(url: string): boolean {
try {
if (url.startsWith('/')) {
return true;
}
+ if (typeof window === 'undefined') {
+ return false;
+ }
const parsed = new URL(url, window.location.origin);
return parsed.origin === window.location.origin;
} catch {
return false;
}
}As per coding guidelines.
🤖 Prompt for AI Agents
In packages/clerk-js/src/ui/utils/redirectRules.ts around lines 28 to 38, the
function reads window.location.origin directly which will throw in SSR; update
it to first check typeof window !== 'undefined' before accessing
window.location.origin, allow relative paths (url.startsWith('/')) in any
environment, and if running in SSR return false for absolute URLs (or fall back
to a provided base if you add one); keep the try/catch and ensure URL is
constructed only when window is defined so no ReferenceError occurs.
Description
Proof-of-concept centralization of redirect rules for the sign in component
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Tests