From 5cb418995ef33e4cf1a84e7bb3cbbc7adc603504 Mon Sep 17 00:00:00 2001 From: Jacek Date: Mon, 20 Oct 2025 21:05:42 -0500 Subject: [PATCH 01/13] poc: redirect rules --- .../src/ui/components/SignIn/SignInStart.tsx | 61 ++-- .../hooks/__tests__/useAuthRedirect.test.tsx | 189 +++++++++++ packages/clerk-js/src/ui/hooks/index.ts | 1 + .../clerk-js/src/ui/hooks/useAuthRedirect.ts | 47 +++ .../ui/utils/__tests__/redirectRules.test.ts | 301 ++++++++++++++++++ .../clerk-js/src/ui/utils/redirectRules.ts | 107 +++++++ 6 files changed, 675 insertions(+), 31 deletions(-) create mode 100644 packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx create mode 100644 packages/clerk-js/src/ui/hooks/useAuthRedirect.ts create mode 100644 packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts create mode 100644 packages/clerk-js/src/ui/utils/redirectRules.ts diff --git a/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx b/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx index 6b0dd0cc8a9..7944ce236a3 100644 --- a/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx +++ b/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx @@ -10,43 +10,37 @@ import type { } from '@clerk/types'; import { useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react'; +import { ERROR_CODES, SIGN_UP_MODES } from '@/core/constants'; +import { clerkInvalidFAPIResponse } from '@/core/errors'; +import type { SignInStartIdentifier } from '@/ui/common'; +import { buildSSOCallbackURL, getIdentifierControlDisplayValues, groupIdentifiers } from '@/ui/common'; +import { handleCombinedFlowTransfer } from '@/ui/components/SignIn/handleCombinedFlowTransfer'; +import { hasMultipleEnterpriseConnections, useHandleAuthenticateWithPasskey } from '@/ui/components/SignIn/shared'; +import { SignInAlternativePhoneCodePhoneNumberCard } from '@/ui/components/SignIn/SignInAlternativePhoneCodePhoneNumberCard'; +import { SignInSocialButtons } from '@/ui/components/SignIn/SignInSocialButtons'; +import { + getPreferredAlternativePhoneChannel, + getPreferredAlternativePhoneChannelForCombinedFlow, + getSignUpAttributeFromIdentifier, +} from '@/ui/components/SignIn/utils'; +import { useCoreSignIn, useEnvironment, useSignInContext } from '@/ui/contexts'; +import { Col, descriptors, Flow, localizationKeys } from '@/ui/customizables'; +import { CaptchaElement } from '@/ui/elements/CaptchaElement'; import { Card } from '@/ui/elements/Card'; import { useCardState, withCardStateProvider } from '@/ui/elements/contexts'; import { Form } from '@/ui/elements/Form'; import { Header } from '@/ui/elements/Header'; import { LoadingCard } from '@/ui/elements/LoadingCard'; import { SocialButtonsReversibleContainerWithDivider } from '@/ui/elements/ReversibleContainer'; +import { useAuthRedirect, useLoadingStatus } from '@/ui/hooks'; +import { useSupportEmail } from '@/ui/hooks/useSupportEmail'; +import { useRouter } from '@/ui/router'; import { handleError } from '@/ui/utils/errorHandler'; import { isMobileDevice } from '@/ui/utils/isMobileDevice'; +import { signInRedirectRules } from '@/ui/utils/redirectRules'; import type { FormControlState } from '@/ui/utils/useFormControl'; import { buildRequest, useFormControl } from '@/ui/utils/useFormControl'; - -import { ERROR_CODES, SIGN_UP_MODES } from '../../../core/constants'; -import { clerkInvalidFAPIResponse } from '../../../core/errors'; -import { getClerkQueryParam, removeClerkQueryParam } from '../../../utils'; -import type { SignInStartIdentifier } from '../../common'; -import { - buildSSOCallbackURL, - getIdentifierControlDisplayValues, - groupIdentifiers, - withRedirectToAfterSignIn, - withRedirectToSignInTask, -} from '../../common'; -import { useCoreSignIn, useEnvironment, useSignInContext } from '../../contexts'; -import { Col, descriptors, Flow, localizationKeys } from '../../customizables'; -import { CaptchaElement } from '../../elements/CaptchaElement'; -import { useLoadingStatus } from '../../hooks'; -import { useSupportEmail } from '../../hooks/useSupportEmail'; -import { useRouter } from '../../router'; -import { handleCombinedFlowTransfer } from './handleCombinedFlowTransfer'; -import { hasMultipleEnterpriseConnections, useHandleAuthenticateWithPasskey } from './shared'; -import { SignInAlternativePhoneCodePhoneNumberCard } from './SignInAlternativePhoneCodePhoneNumberCard'; -import { SignInSocialButtons } from './SignInSocialButtons'; -import { - getPreferredAlternativePhoneChannel, - getPreferredAlternativePhoneChannelForCombinedFlow, - getSignUpAttributeFromIdentifier, -} from './utils'; +import { getClerkQueryParam, removeClerkQueryParam } from '@/utils'; const useAutoFillPasskey = () => { const [isSupported, setIsSupported] = useState(false); @@ -87,6 +81,7 @@ function SignInStartInternal(): JSX.Element { const ctx = useSignInContext(); const { afterSignInUrl, signUpUrl, waitlistUrl, isCombinedFlow, navigateOnSetActive } = ctx; const supportEmail = useSupportEmail(); + const identifierAttributes = useMemo( () => groupIdentifiers(userSettings.enabledFirstFactorIdentifiers), [userSettings.enabledFirstFactorIdentifiers], @@ -514,7 +509,13 @@ function SignInStartInternal(): JSX.Element { return components[identifierField.type as keyof typeof components]; }, [identifierField.type]); - if (status.isLoading || clerkStatus === 'sign_up') { + // Handle redirect scenarios - must be after all hooks + const { isRedirecting } = useAuthRedirect({ + rules: signInRedirectRules, + additionalContext: { afterSignInUrl }, + }); + + if (isRedirecting || status.isLoading || clerkStatus === 'sign_up') { // clerkStatus being sign_up will trigger a navigation to the sign up flow, so show a loading card instead of // rendering the sign in flow. return ; @@ -698,6 +699,4 @@ const InstantPasswordRow = ({ field }: { field?: FormControlState<'password'> }) ); }; -export const SignInStart = withRedirectToSignInTask( - withRedirectToAfterSignIn(withCardStateProvider(SignInStartInternal)), -); +export const SignInStart = withCardStateProvider(SignInStartInternal); diff --git a/packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx b/packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx new file mode 100644 index 00000000000..a777deadc26 --- /dev/null +++ b/packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx @@ -0,0 +1,189 @@ +import { renderHook, waitFor } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import type { Clerk, EnvironmentResource } from '@clerk/types'; + +import type { RedirectRule } from '../../utils/redirectRules'; +import { useAuthRedirect } from '../useAuthRedirect'; + +// Mock dependencies +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), +})); + +import { useClerk } from '@clerk/shared/react'; + +import { useEnvironment } from '../../contexts'; +import { useRouter } from '../../router'; +import { evaluateRedirectRules } from '../../utils/redirectRules'; + +describe('useAuthRedirect', () => { + const mockNavigate = vi.fn(); + const mockClerk = { + isSignedIn: false, + publishableKey: 'pk_test_xxx', + } as Clerk; + const mockEnvironment = { + authConfig: { singleSessionMode: false }, + } as EnvironmentResource; + + beforeEach(() => { + vi.clearAllMocks(); + (useClerk as any).mockReturnValue(mockClerk); + (useEnvironment as any).mockReturnValue(mockEnvironment); + (useRouter as any).mockReturnValue({ + currentPath: '/sign-in', + navigate: mockNavigate, + }); + }); + + it('returns isRedirecting: false when no rules match', () => { + (evaluateRedirectRules as any).mockReturnValue(null); + + const { result } = renderHook(() => + useAuthRedirect({ + rules: [], + }), + ); + + expect(result.current.isRedirecting).toBe(false); + }); + + it('returns isRedirecting: true and calls navigate when rule matches', async () => { + const mockRule: RedirectRule = vi.fn(() => ({ + destination: '/dashboard', + reason: 'Test redirect', + })); + + (evaluateRedirectRules as any).mockReturnValue({ + destination: '/dashboard', + reason: 'Test redirect', + }); + + const { result } = renderHook(() => + useAuthRedirect({ + rules: [mockRule], + }), + ); + + await waitFor(() => { + expect(result.current.isRedirecting).toBe(true); + }); + + expect(mockNavigate).toHaveBeenCalledWith('/dashboard'); + }); + + it('passes additional context to rules', () => { + const additionalContext = { afterSignInUrl: '/custom-dashboard' }; + + renderHook(() => + useAuthRedirect({ + rules: [], + additionalContext, + }), + ); + + expect(evaluateRedirectRules).toHaveBeenCalledWith( + [], + expect.objectContaining({ + clerk: mockClerk, + currentPath: '/sign-in', + environment: mockEnvironment, + afterSignInUrl: '/custom-dashboard', + }), + false, + ); + }); + + it('re-evaluates when isSignedIn changes', () => { + const { rerender } = renderHook(() => + useAuthRedirect({ + rules: [], + }), + ); + + expect(evaluateRedirectRules).toHaveBeenCalledTimes(1); + + // Change isSignedIn + (useClerk as any).mockReturnValue({ + ...mockClerk, + isSignedIn: true, + }); + + rerender(); + + expect(evaluateRedirectRules).toHaveBeenCalledTimes(2); + }); + + it('re-evaluates when session count changes', () => { + const { rerender } = renderHook(() => + useAuthRedirect({ + rules: [], + }), + ); + + expect(evaluateRedirectRules).toHaveBeenCalledTimes(1); + + // Change session count + (useClerk as any).mockReturnValue({ + ...mockClerk, + client: { sessions: [{ id: '1' }] }, + }); + + rerender(); + + expect(evaluateRedirectRules).toHaveBeenCalledTimes(2); + }); + + it('re-evaluates when singleSessionMode changes', () => { + const { rerender } = renderHook(() => + useAuthRedirect({ + rules: [], + }), + ); + + expect(evaluateRedirectRules).toHaveBeenCalledTimes(1); + + // Change singleSessionMode + (useEnvironment as any).mockReturnValue({ + authConfig: { singleSessionMode: true }, + }); + + rerender(); + + expect(evaluateRedirectRules).toHaveBeenCalledTimes(2); + }); + + it('re-evaluates when currentPath changes', () => { + const { rerender } = renderHook(() => + useAuthRedirect({ + rules: [], + }), + ); + + expect(evaluateRedirectRules).toHaveBeenCalledTimes(1); + + // Change currentPath + (useRouter as any).mockReturnValue({ + currentPath: '/sign-in/factor-one', + navigate: mockNavigate, + }); + + rerender(); + + expect(evaluateRedirectRules).toHaveBeenCalledTimes(2); + }); +}); diff --git a/packages/clerk-js/src/ui/hooks/index.ts b/packages/clerk-js/src/ui/hooks/index.ts index b456a3dd943..4324869cc4c 100644 --- a/packages/clerk-js/src/ui/hooks/index.ts +++ b/packages/clerk-js/src/ui/hooks/index.ts @@ -1,3 +1,4 @@ +export * from './useAuthRedirect'; export * from './useClerkModalStateParams'; export * from './useClipboard'; export * from './useDebounce'; diff --git a/packages/clerk-js/src/ui/hooks/useAuthRedirect.ts b/packages/clerk-js/src/ui/hooks/useAuthRedirect.ts new file mode 100644 index 00000000000..d1081f30afe --- /dev/null +++ b/packages/clerk-js/src/ui/hooks/useAuthRedirect.ts @@ -0,0 +1,47 @@ +import { useClerk } from '@clerk/shared/react'; +import React from 'react'; + +import { useEnvironment } from '../contexts'; +import { useRouter } from '../router'; +import type { RedirectResult, RedirectRule } from '../utils/redirectRules'; +import { evaluateRedirectRules, isDevelopmentMode } from '../utils/redirectRules'; + +export interface UseAuthRedirectOptions { + rules: RedirectRule[]; + additionalContext?: Record; +} + +export interface UseAuthRedirectReturn { + isRedirecting: boolean; +} + +/** + * Hook to handle authentication redirects based on rules + */ +export function useAuthRedirect(options: UseAuthRedirectOptions): UseAuthRedirectReturn { + const clerk = useClerk(); + const environment = useEnvironment(); + const { navigate, currentPath } = useRouter(); + const [isRedirecting, setIsRedirecting] = React.useState(false); + + React.useEffect(() => { + const context = { + clerk, + currentPath, + environment, + ...options.additionalContext, + }; + + const result = evaluateRedirectRules(options.rules, context, isDevelopmentMode(clerk)); + + if (result) { + setIsRedirecting(true); + void navigate(result.destination); + } else { + setIsRedirecting(false); + } + }, [clerk.isSignedIn, clerk.client?.sessions?.length, environment.authConfig.singleSessionMode, currentPath]); + + return { isRedirecting }; +} + diff --git a/packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts b/packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts new file mode 100644 index 00000000000..6dc21bf98f9 --- /dev/null +++ b/packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts @@ -0,0 +1,301 @@ +import { describe, expect, it } from 'vitest'; + +import type { Clerk, EnvironmentResource } from '@clerk/types'; + +import type { RedirectContext } from '../redirectRules'; +import { evaluateRedirectRules, signInRedirectRules, signUpRedirectRules } from '../redirectRules'; + +describe('evaluateRedirectRules', () => { + it('returns null when no rules match', () => { + const context: RedirectContext = { + clerk: { isSignedIn: false } as Clerk, + currentPath: '/sign-in', + environment: { authConfig: { singleSessionMode: false } } as EnvironmentResource, + }; + + const result = evaluateRedirectRules([], context); + expect(result).toBeNull(); + }); + + it('returns first matching rule result', () => { + const rules = [ + () => null, + () => ({ destination: '/page2', reason: 'Rule 2' }), + () => ({ destination: '/page3', reason: 'Rule 3' }), + ]; + + const context: RedirectContext = { + clerk: {} as Clerk, + currentPath: '/test', + environment: {} as EnvironmentResource, + }; + + const result = evaluateRedirectRules(rules, context); + expect(result).toEqual({ destination: '/page2', reason: 'Rule 2' }); + }); + + it('evaluates all rules until a match is found', () => { + const rules = [() => null, () => null, () => ({ destination: '/match', reason: 'Found' })]; + + const context: RedirectContext = { + clerk: {} as Clerk, + currentPath: '/test', + environment: {} as EnvironmentResource, + }; + + const result = evaluateRedirectRules(rules, context); + expect(result).toEqual({ destination: '/match', reason: 'Found' }); + }); +}); + +describe('signInRedirectRules', () => { + describe('single session mode redirect', () => { + it('redirects to afterSignInUrl when already signed in', () => { + const context: RedirectContext = { + clerk: { + buildAfterSignInUrl: () => '/default-dashboard', + isSignedIn: true, + } as Clerk, + currentPath: '/sign-in', + environment: { + authConfig: { singleSessionMode: true }, + } as EnvironmentResource, + afterSignInUrl: '/custom-dashboard', + }; + + const result = evaluateRedirectRules(signInRedirectRules, context); + + expect(result).toEqual({ + destination: '/custom-dashboard', + reason: 'User already signed in (single session mode)', + }); + }); + + it('uses clerk.buildAfterSignInUrl when afterSignInUrl not provided', () => { + const context: RedirectContext = { + clerk: { + buildAfterSignInUrl: () => '/default-dashboard', + isSignedIn: true, + } as Clerk, + currentPath: '/sign-in', + environment: { + authConfig: { singleSessionMode: true }, + } as EnvironmentResource, + }; + + const result = evaluateRedirectRules(signInRedirectRules, context); + + expect(result).toEqual({ + destination: '/default-dashboard', + reason: 'User already signed in (single session mode)', + }); + }); + + it('does not redirect when not signed in', () => { + const context: RedirectContext = { + clerk: { + buildAfterSignInUrl: () => '/dashboard', + isSignedIn: false, + } as Clerk, + currentPath: '/sign-in', + environment: { + authConfig: { singleSessionMode: true }, + } as EnvironmentResource, + }; + + const result = evaluateRedirectRules(signInRedirectRules, context); + expect(result).toBeNull(); + }); + + it('does not redirect in multi-session mode', () => { + const context: RedirectContext = { + clerk: { + buildAfterSignInUrl: () => '/dashboard', + isSignedIn: true, + } as Clerk, + currentPath: '/sign-in', + environment: { + authConfig: { singleSessionMode: false }, + } as EnvironmentResource, + }; + + const result = evaluateRedirectRules(signInRedirectRules, context); + expect(result).not.toEqual(expect.objectContaining({ reason: 'User already signed in (single session mode)' })); + }); + }); + + describe('multi-session mode account switcher redirect', () => { + it('redirects to /sign-in/choose at root with active sessions', () => { + const context: RedirectContext = { + clerk: { + client: { sessions: [{ id: '1' }, { id: '2' }] }, + isSignedIn: true, + } as any, + currentPath: '/sign-in', + environment: { + authConfig: { singleSessionMode: false }, + } as EnvironmentResource, + }; + + const result = evaluateRedirectRules(signInRedirectRules, context); + + expect(result).toEqual({ + destination: '/sign-in/choose', + reason: 'Active sessions detected (multi-session mode)', + }); + }); + + it('redirects to /sign-in/choose at root with trailing slash', () => { + const context: RedirectContext = { + clerk: { + client: { sessions: [{ id: '1' }] }, + isSignedIn: true, + } as any, + currentPath: '/sign-in/', + environment: { + authConfig: { singleSessionMode: false }, + } as EnvironmentResource, + }; + + const result = evaluateRedirectRules(signInRedirectRules, context); + + expect(result).toEqual({ + destination: '/sign-in/choose', + reason: 'Active sessions detected (multi-session mode)', + }); + }); + + it('does not redirect at non-root paths', () => { + const context: RedirectContext = { + clerk: { + client: { sessions: [{ id: '1' }] }, + isSignedIn: true, + } as any, + currentPath: '/sign-in/factor-one', + environment: { + authConfig: { singleSessionMode: false }, + } as EnvironmentResource, + }; + + const result = evaluateRedirectRules(signInRedirectRules, context); + expect(result).toBeNull(); + }); + + it('does not redirect when no active sessions', () => { + const context: RedirectContext = { + clerk: { + client: { sessions: [] }, + isSignedIn: false, + } as any, + currentPath: '/sign-in', + environment: { + authConfig: { singleSessionMode: false }, + } as EnvironmentResource, + }; + + const result = evaluateRedirectRules(signInRedirectRules, context); + expect(result).toBeNull(); + }); + + it('does not redirect in single-session mode', () => { + const context: RedirectContext = { + clerk: { + buildAfterSignInUrl: () => '/dashboard', + client: { sessions: [{ id: '1' }] }, + isSignedIn: true, + } as any, + currentPath: '/sign-in', + environment: { + authConfig: { singleSessionMode: true }, + } as EnvironmentResource, + }; + + const result = evaluateRedirectRules(signInRedirectRules, context); + // Should match first rule instead (single session redirect) + expect(result?.reason).toBe('User already signed in (single session mode)'); + }); + }); +}); + +describe('signUpRedirectRules', () => { + describe('single session mode redirect', () => { + it('redirects to afterSignUpUrl when already signed in', () => { + const context: RedirectContext = { + clerk: { + buildAfterSignUpUrl: () => '/default-onboarding', + isSignedIn: true, + } as Clerk, + currentPath: '/sign-up', + environment: { + authConfig: { singleSessionMode: true }, + } as EnvironmentResource, + afterSignUpUrl: '/custom-onboarding', + }; + + const result = evaluateRedirectRules(signUpRedirectRules, context); + + expect(result).toEqual({ + destination: '/custom-onboarding', + reason: 'User already signed in (single session mode)', + }); + }); + + it('uses clerk.buildAfterSignUpUrl when afterSignUpUrl not provided', () => { + const context: RedirectContext = { + clerk: { + buildAfterSignUpUrl: () => '/default-onboarding', + isSignedIn: true, + } as Clerk, + currentPath: '/sign-up', + environment: { + authConfig: { singleSessionMode: true }, + } as EnvironmentResource, + }; + + const result = evaluateRedirectRules(signUpRedirectRules, context); + + expect(result).toEqual({ + destination: '/default-onboarding', + reason: 'User already signed in (single session mode)', + }); + }); + }); + + describe('multi-session mode account switcher redirect', () => { + it('redirects to /sign-in/choose at root with active sessions', () => { + const context: RedirectContext = { + clerk: { + client: { sessions: [{ id: '1' }] }, + isSignedIn: true, + } as any, + currentPath: '/sign-up', + environment: { + authConfig: { singleSessionMode: false }, + } as EnvironmentResource, + }; + + const result = evaluateRedirectRules(signUpRedirectRules, context); + + expect(result).toEqual({ + destination: '/sign-in/choose', + reason: 'Active sessions detected (multi-session mode)', + }); + }); + + it('does not redirect at non-root paths', () => { + const context: RedirectContext = { + clerk: { + client: { sessions: [{ id: '1' }] }, + isSignedIn: true, + } as any, + currentPath: '/sign-up/verify', + environment: { + authConfig: { singleSessionMode: false }, + } as EnvironmentResource, + }; + + const result = evaluateRedirectRules(signUpRedirectRules, context); + expect(result).toBeNull(); + }); + }); +}); diff --git a/packages/clerk-js/src/ui/utils/redirectRules.ts b/packages/clerk-js/src/ui/utils/redirectRules.ts new file mode 100644 index 00000000000..1a62567f3b5 --- /dev/null +++ b/packages/clerk-js/src/ui/utils/redirectRules.ts @@ -0,0 +1,107 @@ +import type { Clerk, ClerkOptions, EnvironmentResource } from '@clerk/types'; + +import { isDevelopmentFromPublishableKey } from '@clerk/shared/keys'; + +export interface RedirectContext { + readonly clerk: Clerk; + readonly currentPath: string; + readonly environment: EnvironmentResource; + readonly [key: string]: any; +} + +export interface RedirectResult { + readonly destination: string; + readonly reason: string; +} + +export type RedirectRule = (context: RedirectContext) => RedirectResult | null; + +/** + * Evaluates redirect rules in order, returning the first match + */ +export function evaluateRedirectRules( + rules: RedirectRule[], + context: RedirectContext, + debug = false, +): RedirectResult | null { + for (const rule of rules) { + const result = rule(context); + if (result) { + if (debug) { + console.info('[Clerk Redirect]', result.reason, '→', result.destination); + } + return result; + } + } + return null; +} + +/** + * Redirect rules for SignIn component + */ +export const signInRedirectRules: RedirectRule[] = [ + // Rule 1: Single session mode - user already signed in + ctx => { + if (ctx.clerk.isSignedIn && ctx.environment.authConfig.singleSessionMode) { + const destination = ctx.afterSignInUrl || ctx.clerk.buildAfterSignInUrl(); + return { + destination, + reason: 'User already signed in (single session mode)', + }; + } + return null; + }, + + // Rule 2: Multi-session mode - redirect to account switcher at root with active sessions + ctx => { + const isRoot = ctx.currentPath === '/sign-in' || ctx.currentPath === '/sign-in/'; + const hasActiveSessions = (ctx.clerk.client?.sessions?.length ?? 0) > 0; + + if (ctx.clerk.isSignedIn && !ctx.environment.authConfig.singleSessionMode && isRoot && hasActiveSessions) { + return { + destination: '/sign-in/choose', + reason: 'Active sessions detected (multi-session mode)', + }; + } + return null; + }, +]; + +/** + * Redirect rules for SignUp component + */ +export const signUpRedirectRules: RedirectRule[] = [ + // Rule 1: Single session mode - user already signed in + ctx => { + if (ctx.clerk.isSignedIn && ctx.environment.authConfig.singleSessionMode) { + const destination = ctx.afterSignUpUrl || ctx.clerk.buildAfterSignUpUrl(); + return { + destination, + reason: 'User already signed in (single session mode)', + }; + } + return null; + }, + + // Rule 2: Multi-session mode - redirect to account switcher at root with active sessions + ctx => { + const isRoot = ctx.currentPath === '/sign-up' || ctx.currentPath === '/sign-up/'; + const hasActiveSessions = (ctx.clerk.client?.sessions?.length ?? 0) > 0; + + if (ctx.clerk.isSignedIn && !ctx.environment.authConfig.singleSessionMode && isRoot && hasActiveSessions) { + return { + destination: '/sign-in/choose', + reason: 'Active sessions detected (multi-session mode)', + }; + } + return null; + }, +]; + +/** + * Helper to check if we're in development mode + */ +export function isDevelopmentMode(clerk: Clerk): boolean { + return isDevelopmentFromPublishableKey(clerk.publishableKey); +} + From 4c576c2eefeda5b01f69df02519e5c3f993964b1 Mon Sep 17 00:00:00 2001 From: Jacek Date: Tue, 21 Oct 2025 10:23:19 -0500 Subject: [PATCH 02/13] wip --- .../tests/session-tasks-multi-session.test.ts | 10 +++ .../tests/sign-in-single-session-mode.test.ts | 79 +++++++++++++++++++ .../src/ui/components/SignIn/SignInStart.tsx | 13 ++- .../clerk-js/src/ui/hooks/useAuthRedirect.ts | 18 ++++- .../ui/utils/__tests__/redirectRules.test.ts | 17 ++-- .../clerk-js/src/ui/utils/redirectRules.ts | 51 ++++++++++-- 6 files changed, 172 insertions(+), 16 deletions(-) create mode 100644 integration/tests/sign-in-single-session-mode.test.ts diff --git a/integration/tests/session-tasks-multi-session.test.ts b/integration/tests/session-tasks-multi-session.test.ts index 7e923f6f957..3235d99fec1 100644 --- a/integration/tests/session-tasks-multi-session.test.ts +++ b/integration/tests/session-tasks-multi-session.test.ts @@ -61,6 +61,11 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withSessionTasks] })( // Create second user, to initiate a pending session // Don't resolve task and switch to active session afterwards await u.po.signIn.goTo(); + await u.page.waitForURL(/sign-in\/choose/); + await u.page.getByText('Add account').click(); + await u.page.waitForURL(/sign-in$/); + await u.po.signIn.waitForMounted(); + await u.po.signIn.getIdentifierInput().waitFor({ state: 'visible' }); await u.po.signIn.setIdentifier(user2.email); await u.po.signIn.continue(); await u.po.signIn.setPassword(user2.password); @@ -68,6 +73,11 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withSessionTasks] })( // Sign-in again back with active session await u.po.signIn.goTo(); + await u.page.waitForURL(/sign-in\/choose/); + await u.page.getByText('Add account').click(); + await u.page.waitForURL(/sign-in$/); + await u.po.signIn.waitForMounted(); + await u.po.signIn.getIdentifierInput().waitFor({ state: 'visible' }); await u.po.signIn.setIdentifier(user1.email); await u.po.signIn.continue(); await u.po.signIn.setPassword(user1.password); diff --git a/integration/tests/sign-in-single-session-mode.test.ts b/integration/tests/sign-in-single-session-mode.test.ts new file mode 100644 index 00000000000..821027d021d --- /dev/null +++ b/integration/tests/sign-in-single-session-mode.test.ts @@ -0,0 +1,79 @@ +import type { FakeUser } from '@clerk/testing/playwright'; +import { test } from '@playwright/test'; + +import type { Application } from '../models/application'; +import { appConfigs } from '../presets'; +import { createTestUtils, testAgainstRunningApps } from '../testUtils'; + +/** + * Tests for single-session mode behavior using the withBilling environment + * which is configured for single-session mode in the Clerk Dashboard. + */ +testAgainstRunningApps({ withEnv: [appConfigs.envs.withBilling] })( + 'sign in with active session in single-session mode @generic @nextjs', + ({ app }) => { + test.describe.configure({ mode: 'serial' }); + + let fakeUser: FakeUser; + + test.beforeAll(async () => { + const u = createTestUtils({ app }); + fakeUser = u.services.users.createFakeUser({ + withPhoneNumber: true, + withUsername: true, + }); + await u.services.users.createBapiUser(fakeUser); + }); + + test.afterAll(async () => { + await fakeUser.deleteIfExists(); + }); + + test('redirects to afterSignIn URL when visiting /sign-in with active session in single-session mode', async ({ + page, + context, + }) => { + const u = createTestUtils({ app, page, context }); + + await u.po.signIn.goTo(); + await u.po.signIn.signInWithEmailAndInstantPassword({ email: fakeUser.email, password: fakeUser.password }); + await u.po.expect.toBeSignedIn(); + await u.page.waitForAppUrl('/'); + + await u.po.signIn.goTo(); + await u.page.waitForAppUrl('/'); + await u.po.expect.toBeSignedIn(); + }); + + test('does NOT show account switcher in single-session mode', async ({ page, context }) => { + const u = createTestUtils({ app, page, context }); + + await u.po.signIn.goTo(); + await u.po.signIn.signInWithEmailAndInstantPassword({ email: fakeUser.email, password: fakeUser.password }); + await u.po.expect.toBeSignedIn(); + + await u.page.goToRelative('/sign-in/choose'); + await u.page.waitForAppUrl('/'); + }); + + test('shows sign-in form when no active session in single-session mode', async ({ page, context }) => { + const u = createTestUtils({ app, page, context }); + + await u.page.context().clearCookies(); + await u.po.signIn.goTo(); + await u.po.signIn.waitForMounted(); + await u.page.waitForSelector('text=/email address|username|phone/i'); + await u.page.waitForURL(/sign-in$/); + }); + + test('can sign in normally when not already authenticated in single-session mode', async ({ page, context }) => { + const u = createTestUtils({ app, page, context }); + + await u.page.context().clearCookies(); + await u.po.signIn.goTo(); + await u.po.signIn.signInWithEmailAndInstantPassword({ email: fakeUser.email, password: fakeUser.password }); + await u.po.expect.toBeSignedIn(); + await u.page.waitForAppUrl('/'); + }); + }, +); diff --git a/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx b/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx index 7944ce236a3..a6056041a95 100644 --- a/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx +++ b/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx @@ -104,6 +104,7 @@ function SignInStartInternal(): JSX.Element { shouldStartWithPhoneNumberIdentifier ? 'phone_number' : identifierAttributes[0] || '', ); const [hasSwitchedByAutofill, setHasSwitchedByAutofill] = useState(false); + const hasInitializedRef = useRef(false); const organizationTicket = getClerkQueryParam('__clerk_ticket') || ''; const clerkStatus = getClerkQueryParam('__clerk_status') || ''; @@ -512,9 +513,19 @@ function SignInStartInternal(): JSX.Element { // Handle redirect scenarios - must be after all hooks const { isRedirecting } = useAuthRedirect({ rules: signInRedirectRules, - additionalContext: { afterSignInUrl }, + additionalContext: { + afterSignInUrl, + hasInitialized: hasInitializedRef.current, + organizationTicket, + queryParams: new URLSearchParams(window.location.search), + }, }); + // Mark as initialized after first render + React.useEffect(() => { + hasInitializedRef.current = true; + }, []); + if (isRedirecting || status.isLoading || clerkStatus === 'sign_up') { // clerkStatus being sign_up will trigger a navigation to the sign up flow, so show a loading card instead of // rendering the sign in flow. diff --git a/packages/clerk-js/src/ui/hooks/useAuthRedirect.ts b/packages/clerk-js/src/ui/hooks/useAuthRedirect.ts index d1081f30afe..991155cbe42 100644 --- a/packages/clerk-js/src/ui/hooks/useAuthRedirect.ts +++ b/packages/clerk-js/src/ui/hooks/useAuthRedirect.ts @@ -35,12 +35,24 @@ export function useAuthRedirect(options: UseAuthRedirectOptions): UseAuthRedirec const result = evaluateRedirectRules(options.rules, context, isDevelopmentMode(clerk)); if (result) { - setIsRedirecting(true); - void navigate(result.destination); + // Execute any side effects + result.onRedirect?.(); + + // Only navigate if not explicitly skipped + if (!result.skipNavigation) { + setIsRedirecting(true); + void navigate(result.destination); + } } else { setIsRedirecting(false); } - }, [clerk.isSignedIn, clerk.client?.sessions?.length, environment.authConfig.singleSessionMode, currentPath]); + }, [ + clerk.isSignedIn, + clerk.client?.sessions?.length, + clerk.client?.signedInSessions?.length, + environment.authConfig.singleSessionMode, + currentPath, + ]); return { isRedirecting }; } diff --git a/packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts b/packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts index 6dc21bf98f9..bd152f64fa9 100644 --- a/packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts +++ b/packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts @@ -128,19 +128,20 @@ describe('signInRedirectRules', () => { it('redirects to /sign-in/choose at root with active sessions', () => { const context: RedirectContext = { clerk: { - client: { sessions: [{ id: '1' }, { id: '2' }] }, + client: { sessions: [{ id: '1' }, { id: '2' }], signedInSessions: [{ id: '1' }, { id: '2' }] }, isSignedIn: true, } as any, currentPath: '/sign-in', environment: { authConfig: { singleSessionMode: false }, } as EnvironmentResource, + hasInitialized: false, }; const result = evaluateRedirectRules(signInRedirectRules, context); expect(result).toEqual({ - destination: '/sign-in/choose', + destination: 'choose', reason: 'Active sessions detected (multi-session mode)', }); }); @@ -148,33 +149,35 @@ describe('signInRedirectRules', () => { it('redirects to /sign-in/choose at root with trailing slash', () => { const context: RedirectContext = { clerk: { - client: { sessions: [{ id: '1' }] }, + client: { sessions: [{ id: '1' }], signedInSessions: [{ id: '1' }] }, isSignedIn: true, } as any, currentPath: '/sign-in/', environment: { authConfig: { singleSessionMode: false }, } as EnvironmentResource, + hasInitialized: false, }; const result = evaluateRedirectRules(signInRedirectRules, context); expect(result).toEqual({ - destination: '/sign-in/choose', + destination: 'choose', reason: 'Active sessions detected (multi-session mode)', }); }); - it('does not redirect at non-root paths', () => { + it('does not redirect when already initialized', () => { const context: RedirectContext = { clerk: { - client: { sessions: [{ id: '1' }] }, + client: { sessions: [{ id: '1' }], signedInSessions: [{ id: '1' }] }, isSignedIn: true, } as any, - currentPath: '/sign-in/factor-one', + currentPath: '/sign-in', environment: { authConfig: { singleSessionMode: false }, } as EnvironmentResource, + hasInitialized: true, }; const result = evaluateRedirectRules(signInRedirectRules, context); diff --git a/packages/clerk-js/src/ui/utils/redirectRules.ts b/packages/clerk-js/src/ui/utils/redirectRules.ts index 1a62567f3b5..f57e7705735 100644 --- a/packages/clerk-js/src/ui/utils/redirectRules.ts +++ b/packages/clerk-js/src/ui/utils/redirectRules.ts @@ -6,12 +6,17 @@ 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; } export interface RedirectResult { readonly destination: string; readonly reason: string; + readonly skipNavigation?: boolean; + readonly onRedirect?: () => void; } export type RedirectRule = (context: RedirectContext) => RedirectResult | null; @@ -52,14 +57,47 @@ export const signInRedirectRules: RedirectRule[] = [ return null; }, - // Rule 2: Multi-session mode - redirect to account switcher at root with active sessions + // Rule 2: Skip redirect if adding account (preserves add account flow) ctx => { - const isRoot = ctx.currentPath === '/sign-in' || ctx.currentPath === '/sign-in/'; - const hasActiveSessions = (ctx.clerk.client?.sessions?.length ?? 0) > 0; + const isAddingAccount = ctx.queryParams?.has('__clerk_add_account'); + if (isAddingAccount) { + return { + destination: ctx.currentPath, + reason: 'User is adding account', + skipNavigation: true, + onRedirect: () => { + // 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); + }, + }; + } + return null; + }, - if (ctx.clerk.isSignedIn && !ctx.environment.authConfig.singleSessionMode && isRoot && hasActiveSessions) { + // Rule 3: Skip redirect if handling organization ticket + ctx => { + if (ctx.organizationTicket) { + return null; + } + return null; + }, + + // Rule 4: Multi-session mode - redirect to account switcher with active sessions + ctx => { + // Only apply on first initialization to prevent redirect loops + if (ctx.hasInitialized) { + return null; + } + + const isMultiSessionMode = !ctx.environment.authConfig.singleSessionMode; + const hasActiveSessions = (ctx.clerk.client?.signedInSessions?.length ?? 0) > 0; + + if (hasActiveSessions && isMultiSessionMode) { return { - destination: '/sign-in/choose', + destination: 'choose', reason: 'Active sessions detected (multi-session mode)', }; } @@ -69,6 +107,9 @@ export const signInRedirectRules: RedirectRule[] = [ /** * Redirect rules for SignUp component + * + * NOTE: Currently not in use. Kept for future implementation. + * When implementing SignUp redirect logic, follow the same pattern as signInRedirectRules. */ export const signUpRedirectRules: RedirectRule[] = [ // Rule 1: Single session mode - user already signed in From c27ca98adcd9c0b0b2768114eda0b137e79821ad Mon Sep 17 00:00:00 2001 From: Jacek Date: Tue, 21 Oct 2025 11:13:29 -0500 Subject: [PATCH 03/13] wip --- packages/clerk-js/src/ui/utils/redirectRules.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/clerk-js/src/ui/utils/redirectRules.ts b/packages/clerk-js/src/ui/utils/redirectRules.ts index f57e7705735..be92cf83b4d 100644 --- a/packages/clerk-js/src/ui/utils/redirectRules.ts +++ b/packages/clerk-js/src/ui/utils/redirectRules.ts @@ -145,4 +145,3 @@ export const signUpRedirectRules: RedirectRule[] = [ export function isDevelopmentMode(clerk: Clerk): boolean { return isDevelopmentFromPublishableKey(clerk.publishableKey); } - From 33c6fd03ea0493d5e51ef4fd48482726c47c33bf Mon Sep 17 00:00:00 2001 From: Jacek Date: Tue, 21 Oct 2025 11:23:13 -0500 Subject: [PATCH 04/13] fix build --- packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx | 2 +- packages/clerk-js/src/ui/hooks/useAuthRedirect.ts | 3 +-- packages/clerk-js/src/ui/utils/redirectRules.ts | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx b/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx index a6056041a95..853e78ab125 100644 --- a/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx +++ b/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx @@ -522,7 +522,7 @@ function SignInStartInternal(): JSX.Element { }); // Mark as initialized after first render - React.useEffect(() => { + useEffect(() => { hasInitializedRef.current = true; }, []); diff --git a/packages/clerk-js/src/ui/hooks/useAuthRedirect.ts b/packages/clerk-js/src/ui/hooks/useAuthRedirect.ts index 991155cbe42..af4955a3fdd 100644 --- a/packages/clerk-js/src/ui/hooks/useAuthRedirect.ts +++ b/packages/clerk-js/src/ui/hooks/useAuthRedirect.ts @@ -3,7 +3,7 @@ import React from 'react'; import { useEnvironment } from '../contexts'; import { useRouter } from '../router'; -import type { RedirectResult, RedirectRule } from '../utils/redirectRules'; +import type { RedirectRule } from '../utils/redirectRules'; import { evaluateRedirectRules, isDevelopmentMode } from '../utils/redirectRules'; export interface UseAuthRedirectOptions { @@ -56,4 +56,3 @@ export function useAuthRedirect(options: UseAuthRedirectOptions): UseAuthRedirec return { isRedirecting }; } - diff --git a/packages/clerk-js/src/ui/utils/redirectRules.ts b/packages/clerk-js/src/ui/utils/redirectRules.ts index be92cf83b4d..4b2f0a6dc8c 100644 --- a/packages/clerk-js/src/ui/utils/redirectRules.ts +++ b/packages/clerk-js/src/ui/utils/redirectRules.ts @@ -1,4 +1,4 @@ -import type { Clerk, ClerkOptions, EnvironmentResource } from '@clerk/types'; +import type { Clerk, EnvironmentResource } from '@clerk/types'; import { isDevelopmentFromPublishableKey } from '@clerk/shared/keys'; From a87316f6c9d5723b928ec8891af2ac01d0de20cf Mon Sep 17 00:00:00 2001 From: Jacek Date: Tue, 21 Oct 2025 11:26:44 -0500 Subject: [PATCH 05/13] changeset --- .changeset/plenty-dolls-pick.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/plenty-dolls-pick.md diff --git a/.changeset/plenty-dolls-pick.md b/.changeset/plenty-dolls-pick.md new file mode 100644 index 00000000000..34031d577e6 --- /dev/null +++ b/.changeset/plenty-dolls-pick.md @@ -0,0 +1,5 @@ +--- +'@clerk/clerk-js': minor +--- + +Centralize redirect concerns for SignIn From 0510cf29b6f42e40815fe0904441fe2821a4dbad Mon Sep 17 00:00:00 2001 From: Jacek Date: Tue, 21 Oct 2025 11:40:31 -0500 Subject: [PATCH 06/13] remove sign up concerns --- .../hooks/__tests__/useAuthRedirect.test.tsx | 3 +- .../clerk-js/src/ui/hooks/useAuthRedirect.ts | 10 ++- .../ui/utils/__tests__/redirectRules.test.ts | 88 +------------------ .../clerk-js/src/ui/utils/redirectRules.ts | 37 +------- 4 files changed, 11 insertions(+), 127 deletions(-) diff --git a/packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx b/packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx index a777deadc26..45b8d7c4e18 100644 --- a/packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx +++ b/packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx @@ -1,8 +1,7 @@ +import type { Clerk, EnvironmentResource } from '@clerk/types'; import { renderHook, waitFor } from '@testing-library/react'; import { beforeEach, describe, expect, it, vi } from 'vitest'; -import type { Clerk, EnvironmentResource } from '@clerk/types'; - import type { RedirectRule } from '../../utils/redirectRules'; import { useAuthRedirect } from '../useAuthRedirect'; diff --git a/packages/clerk-js/src/ui/hooks/useAuthRedirect.ts b/packages/clerk-js/src/ui/hooks/useAuthRedirect.ts index af4955a3fdd..35210b10d08 100644 --- a/packages/clerk-js/src/ui/hooks/useAuthRedirect.ts +++ b/packages/clerk-js/src/ui/hooks/useAuthRedirect.ts @@ -6,9 +6,9 @@ import { useRouter } from '../router'; import type { RedirectRule } from '../utils/redirectRules'; import { evaluateRedirectRules, isDevelopmentMode } from '../utils/redirectRules'; -export interface UseAuthRedirectOptions { +export interface UseAuthRedirectOptions = Record> { rules: RedirectRule[]; - additionalContext?: Record; + additionalContext?: C; } export interface UseAuthRedirectReturn { @@ -17,8 +17,12 @@ export interface UseAuthRedirectReturn { /** * Hook to handle authentication redirects based on rules + * + * @template C - The type of additional context to pass to redirect rules */ -export function useAuthRedirect(options: UseAuthRedirectOptions): UseAuthRedirectReturn { +export function useAuthRedirect = Record>( + options: UseAuthRedirectOptions, +): UseAuthRedirectReturn { const clerk = useClerk(); const environment = useEnvironment(); const { navigate, currentPath } = useRouter(); diff --git a/packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts b/packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts index bd152f64fa9..270d1dba454 100644 --- a/packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts +++ b/packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts @@ -1,9 +1,8 @@ -import { describe, expect, it } from 'vitest'; - import type { Clerk, EnvironmentResource } from '@clerk/types'; +import { describe, expect, it } from 'vitest'; import type { RedirectContext } from '../redirectRules'; -import { evaluateRedirectRules, signInRedirectRules, signUpRedirectRules } from '../redirectRules'; +import { evaluateRedirectRules, signInRedirectRules } from '../redirectRules'; describe('evaluateRedirectRules', () => { it('returns null when no rules match', () => { @@ -219,86 +218,3 @@ describe('signInRedirectRules', () => { }); }); }); - -describe('signUpRedirectRules', () => { - describe('single session mode redirect', () => { - it('redirects to afterSignUpUrl when already signed in', () => { - const context: RedirectContext = { - clerk: { - buildAfterSignUpUrl: () => '/default-onboarding', - isSignedIn: true, - } as Clerk, - currentPath: '/sign-up', - environment: { - authConfig: { singleSessionMode: true }, - } as EnvironmentResource, - afterSignUpUrl: '/custom-onboarding', - }; - - const result = evaluateRedirectRules(signUpRedirectRules, context); - - expect(result).toEqual({ - destination: '/custom-onboarding', - reason: 'User already signed in (single session mode)', - }); - }); - - it('uses clerk.buildAfterSignUpUrl when afterSignUpUrl not provided', () => { - const context: RedirectContext = { - clerk: { - buildAfterSignUpUrl: () => '/default-onboarding', - isSignedIn: true, - } as Clerk, - currentPath: '/sign-up', - environment: { - authConfig: { singleSessionMode: true }, - } as EnvironmentResource, - }; - - const result = evaluateRedirectRules(signUpRedirectRules, context); - - expect(result).toEqual({ - destination: '/default-onboarding', - reason: 'User already signed in (single session mode)', - }); - }); - }); - - describe('multi-session mode account switcher redirect', () => { - it('redirects to /sign-in/choose at root with active sessions', () => { - const context: RedirectContext = { - clerk: { - client: { sessions: [{ id: '1' }] }, - isSignedIn: true, - } as any, - currentPath: '/sign-up', - environment: { - authConfig: { singleSessionMode: false }, - } as EnvironmentResource, - }; - - const result = evaluateRedirectRules(signUpRedirectRules, context); - - expect(result).toEqual({ - destination: '/sign-in/choose', - reason: 'Active sessions detected (multi-session mode)', - }); - }); - - it('does not redirect at non-root paths', () => { - const context: RedirectContext = { - clerk: { - client: { sessions: [{ id: '1' }] }, - isSignedIn: true, - } as any, - currentPath: '/sign-up/verify', - environment: { - authConfig: { singleSessionMode: false }, - } as EnvironmentResource, - }; - - const result = evaluateRedirectRules(signUpRedirectRules, context); - expect(result).toBeNull(); - }); - }); -}); diff --git a/packages/clerk-js/src/ui/utils/redirectRules.ts b/packages/clerk-js/src/ui/utils/redirectRules.ts index 4b2f0a6dc8c..bc392c699f9 100644 --- a/packages/clerk-js/src/ui/utils/redirectRules.ts +++ b/packages/clerk-js/src/ui/utils/redirectRules.ts @@ -1,6 +1,5 @@ -import type { Clerk, EnvironmentResource } from '@clerk/types'; - import { isDevelopmentFromPublishableKey } from '@clerk/shared/keys'; +import type { Clerk, EnvironmentResource } from '@clerk/types'; export interface RedirectContext { readonly clerk: Clerk; @@ -105,40 +104,6 @@ export const signInRedirectRules: RedirectRule[] = [ }, ]; -/** - * Redirect rules for SignUp component - * - * NOTE: Currently not in use. Kept for future implementation. - * When implementing SignUp redirect logic, follow the same pattern as signInRedirectRules. - */ -export const signUpRedirectRules: RedirectRule[] = [ - // Rule 1: Single session mode - user already signed in - ctx => { - if (ctx.clerk.isSignedIn && ctx.environment.authConfig.singleSessionMode) { - const destination = ctx.afterSignUpUrl || ctx.clerk.buildAfterSignUpUrl(); - return { - destination, - reason: 'User already signed in (single session mode)', - }; - } - return null; - }, - - // Rule 2: Multi-session mode - redirect to account switcher at root with active sessions - ctx => { - const isRoot = ctx.currentPath === '/sign-up' || ctx.currentPath === '/sign-up/'; - const hasActiveSessions = (ctx.clerk.client?.sessions?.length ?? 0) > 0; - - if (ctx.clerk.isSignedIn && !ctx.environment.authConfig.singleSessionMode && isRoot && hasActiveSessions) { - return { - destination: '/sign-in/choose', - reason: 'Active sessions detected (multi-session mode)', - }; - } - return null; - }, -]; - /** * Helper to check if we're in development mode */ From 944ece4196ee113dae9262dd2fbebc1668443c40 Mon Sep 17 00:00:00 2001 From: Jacek Date: Tue, 21 Oct 2025 12:04:35 -0500 Subject: [PATCH 07/13] introduce rule exceptions --- .../hooks/__tests__/useAuthRedirect.test.tsx | 162 +++++++++--------- .../ui/utils/__tests__/redirectRules.test.ts | 114 +++++++++--- .../clerk-js/src/ui/utils/redirectRules.ts | 62 +++++-- 3 files changed, 220 insertions(+), 118 deletions(-) diff --git a/packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx b/packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx index 45b8d7c4e18..2f0635612df 100644 --- a/packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx +++ b/packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx @@ -2,7 +2,6 @@ import type { Clerk, EnvironmentResource } from '@clerk/types'; import { renderHook, waitFor } from '@testing-library/react'; import { beforeEach, describe, expect, it, vi } from 'vitest'; -import type { RedirectRule } from '../../utils/redirectRules'; import { useAuthRedirect } from '../useAuthRedirect'; // Mock dependencies @@ -18,10 +17,14 @@ vi.mock('../../router', () => ({ useRouter: vi.fn(), })); -vi.mock('../../utils/redirectRules', () => ({ - evaluateRedirectRules: vi.fn(), - isDevelopmentMode: vi.fn(() => false), -})); +vi.mock('../../utils/redirectRules', async () => { + const actual = await vi.importActual('../../utils/redirectRules'); + return { + ...actual, + evaluateRedirectRules: vi.fn(), + isDevelopmentMode: vi.fn(() => false), + }; +}); import { useClerk } from '@clerk/shared/react'; @@ -33,40 +36,32 @@ describe('useAuthRedirect', () => { const mockNavigate = vi.fn(); const mockClerk = { isSignedIn: false, - publishableKey: 'pk_test_xxx', - } as Clerk; + client: { sessions: [], signedInSessions: [] }, + } as unknown as Clerk; const mockEnvironment = { - authConfig: { singleSessionMode: false }, + authConfig: { singleSessionMode: true }, } as EnvironmentResource; beforeEach(() => { vi.clearAllMocks(); (useClerk as any).mockReturnValue(mockClerk); (useEnvironment as any).mockReturnValue(mockEnvironment); - (useRouter as any).mockReturnValue({ - currentPath: '/sign-in', - navigate: mockNavigate, - }); - }); - - it('returns isRedirecting: false when no rules match', () => { + (useRouter as any).mockReturnValue({ navigate: mockNavigate, currentPath: '/sign-in' }); (evaluateRedirectRules as any).mockReturnValue(null); + }); + it('returns isRedirecting false when no redirect needed', () => { const { result } = renderHook(() => useAuthRedirect({ rules: [], + additionalContext: {}, }), ); expect(result.current.isRedirecting).toBe(false); }); - it('returns isRedirecting: true and calls navigate when rule matches', async () => { - const mockRule: RedirectRule = vi.fn(() => ({ - destination: '/dashboard', - reason: 'Test redirect', - })); - + it('navigates when redirect rule matches', async () => { (evaluateRedirectRules as any).mockReturnValue({ destination: '/dashboard', reason: 'Test redirect', @@ -74,115 +69,122 @@ describe('useAuthRedirect', () => { const { result } = renderHook(() => useAuthRedirect({ - rules: [mockRule], + rules: [], + additionalContext: {}, }), ); await waitFor(() => { expect(result.current.isRedirecting).toBe(true); + expect(mockNavigate).toHaveBeenCalledWith('/dashboard'); }); - - expect(mockNavigate).toHaveBeenCalledWith('/dashboard'); }); - it('passes additional context to rules', () => { - const additionalContext = { afterSignInUrl: '/custom-dashboard' }; + it('does not navigate when skipNavigation is true', async () => { + (evaluateRedirectRules as any).mockReturnValue({ + destination: '/current', + reason: 'Side effect only', + skipNavigation: true, + }); renderHook(() => useAuthRedirect({ rules: [], - additionalContext, + additionalContext: {}, }), ); - expect(evaluateRedirectRules).toHaveBeenCalledWith( - [], - expect.objectContaining({ - clerk: mockClerk, - currentPath: '/sign-in', - environment: mockEnvironment, - afterSignInUrl: '/custom-dashboard', - }), - false, - ); + await waitFor(() => { + expect(mockNavigate).not.toHaveBeenCalled(); + }); }); - it('re-evaluates when isSignedIn changes', () => { - const { rerender } = renderHook(() => + it('executes onRedirect callback when provided', async () => { + const onRedirect = vi.fn(); + (evaluateRedirectRules as any).mockReturnValue({ + destination: '/dashboard', + reason: 'Test redirect', + onRedirect, + }); + + renderHook(() => useAuthRedirect({ rules: [], + additionalContext: {}, }), ); - expect(evaluateRedirectRules).toHaveBeenCalledTimes(1); - - // Change isSignedIn - (useClerk as any).mockReturnValue({ - ...mockClerk, - isSignedIn: true, + await waitFor(() => { + expect(onRedirect).toHaveBeenCalled(); }); - - rerender(); - - expect(evaluateRedirectRules).toHaveBeenCalledTimes(2); }); - it('re-evaluates when session count changes', () => { - const { rerender } = renderHook(() => + it('passes additional context to evaluateRedirectRules', async () => { + const additionalContext = { + afterSignInUrl: '/custom', + organizationTicket: 'test_ticket', + }; + + renderHook(() => useAuthRedirect({ rules: [], + additionalContext, }), ); - expect(evaluateRedirectRules).toHaveBeenCalledTimes(1); - - // Change session count - (useClerk as any).mockReturnValue({ - ...mockClerk, - client: { sessions: [{ id: '1' }] }, + await waitFor(() => { + expect(evaluateRedirectRules).toHaveBeenCalledWith( + [], + expect.objectContaining({ + clerk: mockClerk, + currentPath: '/sign-in', + environment: mockEnvironment, + ...additionalContext, + }), + false, + ); }); - - rerender(); - - expect(evaluateRedirectRules).toHaveBeenCalledTimes(2); }); - it('re-evaluates when singleSessionMode changes', () => { + it('re-evaluates when auth state changes', async () => { const { rerender } = renderHook(() => useAuthRedirect({ rules: [], + additionalContext: {}, }), ); expect(evaluateRedirectRules).toHaveBeenCalledTimes(1); - // Change singleSessionMode - (useEnvironment as any).mockReturnValue({ - authConfig: { singleSessionMode: true }, + // Change auth state + (useClerk as any).mockReturnValue({ + ...mockClerk, + isSignedIn: true, }); rerender(); - expect(evaluateRedirectRules).toHaveBeenCalledTimes(2); + await waitFor(() => { + expect(evaluateRedirectRules).toHaveBeenCalledTimes(2); + }); }); - it('re-evaluates when currentPath changes', () => { - const { rerender } = renderHook(() => - useAuthRedirect({ + it('handles type-safe additional context', () => { + interface CustomContext { + customField: string; + optionalField?: number; + } + + const { result } = renderHook(() => + useAuthRedirect({ rules: [], + additionalContext: { + customField: 'test', + optionalField: 42, + }, }), ); - expect(evaluateRedirectRules).toHaveBeenCalledTimes(1); - - // Change currentPath - (useRouter as any).mockReturnValue({ - currentPath: '/sign-in/factor-one', - navigate: mockNavigate, - }); - - rerender(); - - expect(evaluateRedirectRules).toHaveBeenCalledTimes(2); + expect(result.current.isRedirecting).toBe(false); }); }); diff --git a/packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts b/packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts index 270d1dba454..61b7353e69b 100644 --- a/packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts +++ b/packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts @@ -2,70 +2,135 @@ import type { Clerk, EnvironmentResource } from '@clerk/types'; import { describe, expect, it } from 'vitest'; import type { RedirectContext } from '../redirectRules'; -import { evaluateRedirectRules, signInRedirectRules } from '../redirectRules'; +import { evaluateRedirectRules, signInRedirectRules, StopRedirectEvaluation } from '../redirectRules'; describe('evaluateRedirectRules', () => { it('returns null when no rules match', () => { const context: RedirectContext = { clerk: { isSignedIn: false } as Clerk, currentPath: '/sign-in', - environment: { authConfig: { singleSessionMode: false } } as EnvironmentResource, + environment: { + authConfig: { singleSessionMode: true }, + } as EnvironmentResource, }; const result = evaluateRedirectRules([], context); expect(result).toBeNull(); }); - it('returns first matching rule result', () => { + it('returns the first matching rule', () => { const rules = [ () => null, - () => ({ destination: '/page2', reason: 'Rule 2' }), - () => ({ destination: '/page3', reason: 'Rule 3' }), + () => ({ destination: '/first', reason: 'First rule' }), + () => ({ destination: '/second', reason: 'Second rule' }), ]; const context: RedirectContext = { clerk: {} as Clerk, - currentPath: '/test', + currentPath: '/sign-in', environment: {} as EnvironmentResource, }; const result = evaluateRedirectRules(rules, context); - expect(result).toEqual({ destination: '/page2', reason: 'Rule 2' }); + expect(result).toEqual({ destination: '/first', reason: 'First rule' }); }); - it('evaluates all rules until a match is found', () => { - const rules = [() => null, () => null, () => ({ destination: '/match', reason: 'Found' })]; + it('handles StopRedirectEvaluation exception and returns null', () => { + const rules = [ + () => { + throw new StopRedirectEvaluation('Guard triggered'); + }, + () => ({ destination: '/should-not-reach', reason: 'Should not execute' }), + ]; const context: RedirectContext = { clerk: {} as Clerk, - currentPath: '/test', + currentPath: '/sign-in', environment: {} as EnvironmentResource, }; const result = evaluateRedirectRules(rules, context); - expect(result).toEqual({ destination: '/match', reason: 'Found' }); + expect(result).toBeNull(); + }); + + it('re-throws unexpected errors', () => { + const rules = [ + () => { + throw new Error('Unexpected error'); + }, + ]; + + const context: RedirectContext = { + clerk: {} as Clerk, + currentPath: '/sign-in', + environment: {} as EnvironmentResource, + }; + + expect(() => evaluateRedirectRules(rules, context)).toThrow('Unexpected error'); }); }); describe('signInRedirectRules', () => { + describe('organization ticket guard', () => { + it('stops evaluation when organization ticket is present', () => { + const context: RedirectContext = { + clerk: { + buildAfterSignInUrl: () => '/dashboard', + isSignedIn: true, + } as Clerk, + currentPath: '/sign-in', + environment: { + authConfig: { singleSessionMode: true }, + } as EnvironmentResource, + organizationTicket: 'test_ticket', + afterSignInUrl: '/custom', + }; + + const result = evaluateRedirectRules(signInRedirectRules, context); + // Should return null because guard stops evaluation + expect(result).toBeNull(); + }); + + it('continues evaluation when no organization ticket', () => { + const context: RedirectContext = { + clerk: { + buildAfterSignInUrl: () => '/dashboard', + isSignedIn: true, + } as Clerk, + currentPath: '/sign-in', + environment: { + authConfig: { singleSessionMode: true }, + } as EnvironmentResource, + afterSignInUrl: '/custom', + }; + + const result = evaluateRedirectRules(signInRedirectRules, context); + // Should match single session rule + expect(result).toEqual({ + destination: '/custom', + reason: 'User already signed in (single session mode)', + }); + }); + }); + describe('single session mode redirect', () => { it('redirects to afterSignInUrl when already signed in', () => { const context: RedirectContext = { clerk: { - buildAfterSignInUrl: () => '/default-dashboard', + buildAfterSignInUrl: () => '/default', isSignedIn: true, } as Clerk, currentPath: '/sign-in', environment: { authConfig: { singleSessionMode: true }, } as EnvironmentResource, - afterSignInUrl: '/custom-dashboard', + afterSignInUrl: '/dashboard', }; const result = evaluateRedirectRules(signInRedirectRules, context); expect(result).toEqual({ - destination: '/custom-dashboard', + destination: '/dashboard', reason: 'User already signed in (single session mode)', }); }); @@ -73,7 +138,7 @@ describe('signInRedirectRules', () => { it('uses clerk.buildAfterSignInUrl when afterSignInUrl not provided', () => { const context: RedirectContext = { clerk: { - buildAfterSignInUrl: () => '/default-dashboard', + buildAfterSignInUrl: () => '/default', isSignedIn: true, } as Clerk, currentPath: '/sign-in', @@ -85,7 +150,7 @@ describe('signInRedirectRules', () => { const result = evaluateRedirectRules(signInRedirectRules, context); expect(result).toEqual({ - destination: '/default-dashboard', + destination: '/default', reason: 'User already signed in (single session mode)', }); }); @@ -93,7 +158,7 @@ describe('signInRedirectRules', () => { it('does not redirect when not signed in', () => { const context: RedirectContext = { clerk: { - buildAfterSignInUrl: () => '/dashboard', + buildAfterSignInUrl: () => '/default', isSignedIn: false, } as Clerk, currentPath: '/sign-in', @@ -109,9 +174,10 @@ describe('signInRedirectRules', () => { it('does not redirect in multi-session mode', () => { const context: RedirectContext = { clerk: { - buildAfterSignInUrl: () => '/dashboard', + buildAfterSignInUrl: () => '/default', isSignedIn: true, - } as Clerk, + client: { signedInSessions: [] }, + } as any, currentPath: '/sign-in', environment: { authConfig: { singleSessionMode: false }, @@ -119,6 +185,7 @@ describe('signInRedirectRules', () => { }; const result = evaluateRedirectRules(signInRedirectRules, context); + // Should not match single session rule, should evaluate other rules expect(result).not.toEqual(expect.objectContaining({ reason: 'User already signed in (single session mode)' })); }); }); @@ -193,23 +260,28 @@ describe('signInRedirectRules', () => { environment: { authConfig: { singleSessionMode: false }, } as EnvironmentResource, + hasInitialized: false, }; const result = evaluateRedirectRules(signInRedirectRules, context); expect(result).toBeNull(); }); + }); - it('does not redirect in single-session mode', () => { + describe('rule priority', () => { + it('single session mode takes precedence over multi-session when both conditions met', () => { const context: RedirectContext = { clerk: { buildAfterSignInUrl: () => '/dashboard', - client: { sessions: [{ id: '1' }] }, + client: { sessions: [{ id: '1' }], signedInSessions: [{ id: '1' }] }, isSignedIn: true, } as any, currentPath: '/sign-in', environment: { authConfig: { singleSessionMode: true }, } as EnvironmentResource, + hasInitialized: false, + afterSignInUrl: '/custom', }; const result = evaluateRedirectRules(signInRedirectRules, context); diff --git a/packages/clerk-js/src/ui/utils/redirectRules.ts b/packages/clerk-js/src/ui/utils/redirectRules.ts index bc392c699f9..67ad29a6efa 100644 --- a/packages/clerk-js/src/ui/utils/redirectRules.ts +++ b/packages/clerk-js/src/ui/utils/redirectRules.ts @@ -21,29 +21,65 @@ export interface RedirectResult { export type RedirectRule = (context: RedirectContext) => RedirectResult | null; /** - * Evaluates redirect rules in order, returning the first match + * Thrown by rules to stop evaluation without redirecting. + * Used for guard conditions that prevent redirects. + */ +export class StopRedirectEvaluation extends Error { + constructor(public readonly reason: string) { + super(reason); + this.name = 'StopRedirectEvaluation'; + } +} + +/** + * Evaluates redirect rules in order, returning the first match. + * Rules can throw StopRedirectEvaluation to short-circuit without redirecting. + * + * @param rules - Array of redirect rules to evaluate + * @param context - Context containing clerk instance, path, environment, etc. + * @param debug - Whether to log debug information (default: false) + * @returns The first matching redirect result, or null if no rules match */ export function evaluateRedirectRules( rules: RedirectRule[], context: RedirectContext, debug = false, ): RedirectResult | null { - for (const rule of rules) { - const result = rule(context); - if (result) { - if (debug) { - console.info('[Clerk Redirect]', result.reason, '→', result.destination); + try { + for (const rule of rules) { + const result = rule(context); + if (result) { + if (debug && isDevelopmentFromPublishableKey(context.clerk.publishableKey)) { + console.info('[Clerk Redirect]', result.reason, '→', result.destination); + } + return result; } - return result; } + return null; + } catch (error) { + if (error instanceof StopRedirectEvaluation) { + if (debug && isDevelopmentFromPublishableKey(context.clerk.publishableKey)) { + console.info('[Clerk Redirect]', 'Evaluation stopped:', error.reason); + } + return null; + } + // Re-throw unexpected errors + throw error; } - return null; } /** * Redirect rules for SignIn component */ export const signInRedirectRules: RedirectRule[] = [ + // Guard: Organization ticket flow is handled separately in component + ctx => { + if (ctx.organizationTicket) { + throw new StopRedirectEvaluation('Organization ticket flow is handled separately'); + } + return null; + }, + // Rule 1: Single session mode - user already signed in ctx => { if (ctx.clerk.isSignedIn && ctx.environment.authConfig.singleSessionMode) { @@ -76,15 +112,7 @@ export const signInRedirectRules: RedirectRule[] = [ return null; }, - // Rule 3: Skip redirect if handling organization ticket - ctx => { - if (ctx.organizationTicket) { - return null; - } - return null; - }, - - // Rule 4: Multi-session mode - redirect to account switcher with active sessions + // Rule 3: Multi-session mode - redirect to account switcher with active sessions ctx => { // Only apply on first initialization to prevent redirect loops if (ctx.hasInitialized) { From ac40dde80f2882ff8afc3d5a1f96cc337307e0e8 Mon Sep 17 00:00:00 2001 From: Jacek Date: Tue, 21 Oct 2025 21:52:22 -0500 Subject: [PATCH 08/13] wip --- .../src/ui/components/SignIn/SignInStart.tsx | 8 +-- .../hooks/__tests__/useAuthRedirect.test.tsx | 1 - .../clerk-js/src/ui/hooks/useAuthRedirect.ts | 4 +- .../ui/utils/__tests__/redirectRules.test.ts | 44 ++++++++++++++++ .../clerk-js/src/ui/utils/redirectRules.ts | 50 +++++++++---------- 5 files changed, 73 insertions(+), 34 deletions(-) diff --git a/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx b/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx index 853e78ab125..0cee6db9a8d 100644 --- a/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx +++ b/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx @@ -77,7 +77,7 @@ function SignInStartInternal(): JSX.Element { const status = useLoadingStatus(); const { displayConfig, userSettings, authConfig } = useEnvironment(); const signIn = useCoreSignIn(); - const { navigate } = useRouter(); + const { navigate, currentPath, queryParams } = useRouter(); const ctx = useSignInContext(); const { afterSignInUrl, signUpUrl, waitlistUrl, isCombinedFlow, navigateOnSetActive } = ctx; const supportEmail = useSupportEmail(); @@ -517,14 +517,14 @@ function SignInStartInternal(): JSX.Element { afterSignInUrl, hasInitialized: hasInitializedRef.current, organizationTicket, - queryParams: new URLSearchParams(window.location.search), + queryParams, }, }); - // Mark as initialized after first render + // Mark as initialized after first render, reset when path changes back to root useEffect(() => { hasInitializedRef.current = true; - }, []); + }, [currentPath]); if (isRedirecting || status.isLoading || clerkStatus === 'sign_up') { // clerkStatus being sign_up will trigger a navigation to the sign up flow, so show a loading card instead of diff --git a/packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx b/packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx index 2f0635612df..6ee156e8cab 100644 --- a/packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx +++ b/packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx @@ -141,7 +141,6 @@ describe('useAuthRedirect', () => { environment: mockEnvironment, ...additionalContext, }), - false, ); }); }); diff --git a/packages/clerk-js/src/ui/hooks/useAuthRedirect.ts b/packages/clerk-js/src/ui/hooks/useAuthRedirect.ts index 35210b10d08..d49018dad35 100644 --- a/packages/clerk-js/src/ui/hooks/useAuthRedirect.ts +++ b/packages/clerk-js/src/ui/hooks/useAuthRedirect.ts @@ -4,7 +4,7 @@ import React from 'react'; import { useEnvironment } from '../contexts'; import { useRouter } from '../router'; import type { RedirectRule } from '../utils/redirectRules'; -import { evaluateRedirectRules, isDevelopmentMode } from '../utils/redirectRules'; +import { evaluateRedirectRules } from '../utils/redirectRules'; export interface UseAuthRedirectOptions = Record> { rules: RedirectRule[]; @@ -36,7 +36,7 @@ export function useAuthRedirect = Record { expect(result?.reason).toBe('User already signed in (single session mode)'); }); }); + + describe('add account flow', () => { + it('returns skip navigation when __clerk_add_account query param is present', () => { + const context: RedirectContext = { + clerk: { + client: { sessions: [], signedInSessions: [] }, + isSignedIn: false, + } as any, + currentPath: '/sign-in', + environment: { + authConfig: { singleSessionMode: false }, + } as EnvironmentResource, + queryParams: { + __clerk_add_account: 'true', + }, + }; + + const result = evaluateRedirectRules(signInRedirectRules, context); + expect(result).toMatchObject({ + destination: '/sign-in', + reason: 'User is adding account', + skipNavigation: true, + }); + expect(result?.onRedirect).toBeDefined(); + }); + + it('does not skip navigation when __clerk_add_account query param is absent', () => { + const context: RedirectContext = { + clerk: { + client: { sessions: [], signedInSessions: [] }, + isSignedIn: false, + } as any, + currentPath: '/sign-in', + environment: { + authConfig: { singleSessionMode: false }, + } as EnvironmentResource, + queryParams: {}, + }; + + const result = evaluateRedirectRules(signInRedirectRules, context); + // Should evaluate other rules + expect(result?.reason).not.toBe('User is adding account'); + }); + }); }); diff --git a/packages/clerk-js/src/ui/utils/redirectRules.ts b/packages/clerk-js/src/ui/utils/redirectRules.ts index 67ad29a6efa..5dcda25667c 100644 --- a/packages/clerk-js/src/ui/utils/redirectRules.ts +++ b/packages/clerk-js/src/ui/utils/redirectRules.ts @@ -1,13 +1,15 @@ -import { isDevelopmentFromPublishableKey } from '@clerk/shared/keys'; import type { Clerk, EnvironmentResource } from '@clerk/types'; +import { debugLogger } from '../../utils/debug'; + export interface RedirectContext { + readonly afterSignInUrl?: string; readonly clerk: Clerk; readonly currentPath: string; readonly environment: EnvironmentResource; readonly hasInitialized?: boolean; readonly organizationTicket?: string; - readonly queryParams?: URLSearchParams; + readonly queryParams?: Record; readonly [key: string]: any; } @@ -37,30 +39,28 @@ export class StopRedirectEvaluation extends Error { * * @param rules - Array of redirect rules to evaluate * @param context - Context containing clerk instance, path, environment, etc. - * @param debug - Whether to log debug information (default: false) * @returns The first matching redirect result, or null if no rules match */ -export function evaluateRedirectRules( - rules: RedirectRule[], - context: RedirectContext, - debug = false, -): RedirectResult | null { +export function evaluateRedirectRules(rules: RedirectRule[], context: RedirectContext): RedirectResult | null { try { for (const rule of rules) { const result = rule(context); if (result) { - if (debug && isDevelopmentFromPublishableKey(context.clerk.publishableKey)) { - console.info('[Clerk Redirect]', result.reason, '→', result.destination); - } + debugLogger.info( + 'Redirect rule matched', + { + destination: result.destination, + reason: result.reason, + }, + 'redirect', + ); return result; } } return null; } catch (error) { if (error instanceof StopRedirectEvaluation) { - if (debug && isDevelopmentFromPublishableKey(context.clerk.publishableKey)) { - console.info('[Clerk Redirect]', 'Evaluation stopped:', error.reason); - } + debugLogger.info('Redirect evaluation stopped', { reason: error.reason }, 'redirect'); return null; } // Re-throw unexpected errors @@ -94,18 +94,21 @@ export const signInRedirectRules: RedirectRule[] = [ // Rule 2: Skip redirect if adding account (preserves add account flow) ctx => { - const isAddingAccount = ctx.queryParams?.has('__clerk_add_account'); + const isAddingAccount = ctx.queryParams?.['__clerk_add_account']; if (isAddingAccount) { return { destination: ctx.currentPath, reason: 'User is adding account', skipNavigation: true, onRedirect: () => { - // 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); + // Clean up query param (platform-specific) + if (typeof window !== 'undefined' && window.history) { + const params = new URLSearchParams(window.location.search); + params.delete('__clerk_add_account'); + const newSearch = params.toString(); + const newUrl = window.location.pathname + (newSearch ? `?${newSearch}` : ''); + window.history.replaceState({}, '', newUrl); + } }, }; } @@ -131,10 +134,3 @@ export const signInRedirectRules: RedirectRule[] = [ return null; }, ]; - -/** - * Helper to check if we're in development mode - */ -export function isDevelopmentMode(clerk: Clerk): boolean { - return isDevelopmentFromPublishableKey(clerk.publishableKey); -} From 9124a4fc909abc6b1533436494b4af80a9af93fe Mon Sep 17 00:00:00 2001 From: Jacek Date: Wed, 22 Oct 2025 14:20:05 -0500 Subject: [PATCH 09/13] wip --- .../templates/next-app-router/next.config.js | 12 +++++++++++- .../src/ui/components/SignIn/SignInStart.tsx | 16 +++++++++++----- .../clerk-js/src/ui/hooks/useAuthRedirect.ts | 11 +++++++++-- 3 files changed, 31 insertions(+), 8 deletions(-) diff --git a/integration/templates/next-app-router/next.config.js b/integration/templates/next-app-router/next.config.js index e47b0b46969..05a4a80e6ab 100644 --- a/integration/templates/next-app-router/next.config.js +++ b/integration/templates/next-app-router/next.config.js @@ -1,9 +1,19 @@ +const path = require('path'); + /** @type {import('next').NextConfig} */ const nextConfig = { eslint: { ignoreDuringBuilds: true, }, - outputFileTracingRoot: '/', + outputFileTracingRoot: path.resolve(__dirname, '../../../'), + webpack: config => { + // Exclude macOS .Trash directory and other system directories to prevent permission errors + config.watchOptions = { + ...config.watchOptions, + ignored: ['**/.Trash/**', '**/node_modules/**', '**/.git/**'], + }; + return config; + }, }; module.exports = nextConfig; diff --git a/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx b/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx index 0cee6db9a8d..c428095b1e2 100644 --- a/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx +++ b/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx @@ -510,15 +510,21 @@ function SignInStartInternal(): JSX.Element { return components[identifierField.type as keyof typeof components]; }, [identifierField.type]); - // Handle redirect scenarios - must be after all hooks - const { isRedirecting } = useAuthRedirect({ - rules: signInRedirectRules, - additionalContext: { + // Memoize additional context to prevent unnecessary re-evaluations + const redirectAdditionalContext = useMemo( + () => ({ afterSignInUrl, hasInitialized: hasInitializedRef.current, organizationTicket, queryParams, - }, + }), + [afterSignInUrl, organizationTicket, queryParams], + ); + + // Handle redirect scenarios - must be after all hooks + const { isRedirecting } = useAuthRedirect({ + rules: signInRedirectRules, + additionalContext: redirectAdditionalContext, }); // Mark as initialized after first render, reset when path changes back to root diff --git a/packages/clerk-js/src/ui/hooks/useAuthRedirect.ts b/packages/clerk-js/src/ui/hooks/useAuthRedirect.ts index d49018dad35..61cd2a08360 100644 --- a/packages/clerk-js/src/ui/hooks/useAuthRedirect.ts +++ b/packages/clerk-js/src/ui/hooks/useAuthRedirect.ts @@ -16,7 +16,9 @@ export interface UseAuthRedirectReturn { } /** - * Hook to handle authentication redirects based on rules + * Hook to handle authentication redirects based on rules. + * + * **Important**: The `additionalContext` object should be memoized by the caller * * @template C - The type of additional context to pass to redirect rules */ @@ -51,11 +53,16 @@ export function useAuthRedirect = Record Date: Wed, 22 Oct 2025 16:36:34 -0500 Subject: [PATCH 10/13] wip --- packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx b/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx index c428095b1e2..f352961bb8d 100644 --- a/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx +++ b/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx @@ -527,10 +527,10 @@ function SignInStartInternal(): JSX.Element { additionalContext: redirectAdditionalContext, }); - // Mark as initialized after first render, reset when path changes back to root + // Mark as initialized after first render useEffect(() => { hasInitializedRef.current = true; - }, [currentPath]); + }, []); if (isRedirecting || status.isLoading || clerkStatus === 'sign_up') { // clerkStatus being sign_up will trigger a navigation to the sign up flow, so show a loading card instead of From 63ba262af329d07032597badd46c86dd1d05db15 Mon Sep 17 00:00:00 2001 From: Jacek Date: Wed, 22 Oct 2025 22:04:07 -0500 Subject: [PATCH 11/13] wip --- .../src/ui/components/SignIn/SignInStart.tsx | 10 +++++----- .../UserButton/useMultisessionActions.tsx | 13 +++++++++++-- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx b/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx index f352961bb8d..e50ba03500d 100644 --- a/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx +++ b/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx @@ -77,7 +77,7 @@ function SignInStartInternal(): JSX.Element { const status = useLoadingStatus(); const { displayConfig, userSettings, authConfig } = useEnvironment(); const signIn = useCoreSignIn(); - const { navigate, currentPath, queryParams } = useRouter(); + const { navigate, queryParams } = useRouter(); const ctx = useSignInContext(); const { afterSignInUrl, signUpUrl, waitlistUrl, isCombinedFlow, navigateOnSetActive } = ctx; const supportEmail = useSupportEmail(); @@ -104,7 +104,7 @@ function SignInStartInternal(): JSX.Element { shouldStartWithPhoneNumberIdentifier ? 'phone_number' : identifierAttributes[0] || '', ); const [hasSwitchedByAutofill, setHasSwitchedByAutofill] = useState(false); - const hasInitializedRef = useRef(false); + const [hasInitialized, setHasInitialized] = useState(false); const organizationTicket = getClerkQueryParam('__clerk_ticket') || ''; const clerkStatus = getClerkQueryParam('__clerk_status') || ''; @@ -514,11 +514,11 @@ function SignInStartInternal(): JSX.Element { const redirectAdditionalContext = useMemo( () => ({ afterSignInUrl, - hasInitialized: hasInitializedRef.current, + hasInitialized, organizationTicket, queryParams, }), - [afterSignInUrl, organizationTicket, queryParams], + [afterSignInUrl, hasInitialized, organizationTicket, queryParams], ); // Handle redirect scenarios - must be after all hooks @@ -529,7 +529,7 @@ function SignInStartInternal(): JSX.Element { // Mark as initialized after first render useEffect(() => { - hasInitializedRef.current = true; + setHasInitialized(true); }, []); if (isRedirecting || status.isLoading || clerkStatus === 'sign_up') { diff --git a/packages/clerk-js/src/ui/components/UserButton/useMultisessionActions.tsx b/packages/clerk-js/src/ui/components/UserButton/useMultisessionActions.tsx index dd3d37e9c1f..e49dbddd90d 100644 --- a/packages/clerk-js/src/ui/components/UserButton/useMultisessionActions.tsx +++ b/packages/clerk-js/src/ui/components/UserButton/useMultisessionActions.tsx @@ -5,6 +5,7 @@ import { navigateIfTaskExists } from '@/core/sessionTasks'; import { useEnvironment } from '@/ui/contexts'; import { useCardState } from '@/ui/elements/contexts'; import { sleep } from '@/ui/utils/sleep'; +import { buildURL } from '@/utils/url'; import { windowNavigate } from '../../../utils/windowNavigate'; import { useMultipleSessions } from '../../hooks/useMultipleSessions'; @@ -25,7 +26,7 @@ export const useMultisessionActions = (opts: UseMultisessionActionsParams) => { const { setActive, signOut, openUserProfile } = useClerk(); const card = useCardState(); const { signedInSessions, otherSessions } = useMultipleSessions({ user: opts.user }); - const { navigate } = useRouter(); + const { navigate, queryParams } = useRouter(); const { displayConfig } = useEnvironment(); const handleSignOutSessionClicked = (session: SignedInSessionResource) => () => { @@ -99,7 +100,15 @@ export const useMultisessionActions = (opts: UseMultisessionActionsParams) => { }; const handleAddAccountClicked = () => { - windowNavigate(opts.signInUrl || window.location.href); + const baseUrl = opts.signInUrl || (typeof window !== 'undefined' ? window.location.href : ''); + const url = buildURL( + { + base: baseUrl, + searchParams: new URLSearchParams({ ...queryParams, __clerk_add_account: 'true' }), + }, + { stringify: true }, + ); + windowNavigate(url); return sleep(2000); }; From 4edf9d14f3799235db7decdbe4810af4950f4d6e Mon Sep 17 00:00:00 2001 From: Jacek Date: Mon, 27 Oct 2025 12:41:23 -0500 Subject: [PATCH 12/13] refactor --- .../src/ui/components/SignIn/SignInStart.tsx | 29 +--- ...ct.test.tsx => useRedirectEngine.test.tsx} | 41 +++-- .../__tests__/useSignInRedirect.test.tsx | 72 ++++++++ .../__tests__/useSignUpRedirect.test.tsx | 65 +++++++ packages/clerk-js/src/ui/hooks/index.ts | 3 +- ...seAuthRedirect.ts => useRedirectEngine.ts} | 35 ++-- .../src/ui/hooks/useSignInRedirect.ts | 38 ++++ .../src/ui/hooks/useSignUpRedirect.ts | 30 ++++ .../ui/utils/__tests__/redirectRules.test.ts | 44 ++--- .../clerk-js/src/ui/utils/redirectRules.ts | 162 +++++++++++------- 10 files changed, 363 insertions(+), 156 deletions(-) rename packages/clerk-js/src/ui/hooks/__tests__/{useAuthRedirect.test.tsx => useRedirectEngine.test.tsx} (81%) create mode 100644 packages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsx create mode 100644 packages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsx rename packages/clerk-js/src/ui/hooks/{useAuthRedirect.ts => useRedirectEngine.ts} (54%) create mode 100644 packages/clerk-js/src/ui/hooks/useSignInRedirect.ts create mode 100644 packages/clerk-js/src/ui/hooks/useSignUpRedirect.ts diff --git a/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx b/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx index e50ba03500d..1a54cda492c 100644 --- a/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx +++ b/packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx @@ -32,12 +32,11 @@ import { Form } from '@/ui/elements/Form'; import { Header } from '@/ui/elements/Header'; import { LoadingCard } from '@/ui/elements/LoadingCard'; import { SocialButtonsReversibleContainerWithDivider } from '@/ui/elements/ReversibleContainer'; -import { useAuthRedirect, useLoadingStatus } from '@/ui/hooks'; +import { useLoadingStatus, useSignInRedirect } from '@/ui/hooks'; import { useSupportEmail } from '@/ui/hooks/useSupportEmail'; import { useRouter } from '@/ui/router'; import { handleError } from '@/ui/utils/errorHandler'; import { isMobileDevice } from '@/ui/utils/isMobileDevice'; -import { signInRedirectRules } from '@/ui/utils/redirectRules'; import type { FormControlState } from '@/ui/utils/useFormControl'; import { buildRequest, useFormControl } from '@/ui/utils/useFormControl'; import { getClerkQueryParam, removeClerkQueryParam } from '@/utils'; @@ -77,7 +76,7 @@ function SignInStartInternal(): JSX.Element { const status = useLoadingStatus(); const { displayConfig, userSettings, authConfig } = useEnvironment(); const signIn = useCoreSignIn(); - const { navigate, queryParams } = useRouter(); + const { navigate } = useRouter(); const ctx = useSignInContext(); const { afterSignInUrl, signUpUrl, waitlistUrl, isCombinedFlow, navigateOnSetActive } = ctx; const supportEmail = useSupportEmail(); @@ -104,7 +103,6 @@ function SignInStartInternal(): JSX.Element { shouldStartWithPhoneNumberIdentifier ? 'phone_number' : identifierAttributes[0] || '', ); const [hasSwitchedByAutofill, setHasSwitchedByAutofill] = useState(false); - const [hasInitialized, setHasInitialized] = useState(false); const organizationTicket = getClerkQueryParam('__clerk_ticket') || ''; const clerkStatus = getClerkQueryParam('__clerk_status') || ''; @@ -510,28 +508,11 @@ function SignInStartInternal(): JSX.Element { return components[identifierField.type as keyof typeof components]; }, [identifierField.type]); - // Memoize additional context to prevent unnecessary re-evaluations - const redirectAdditionalContext = useMemo( - () => ({ - afterSignInUrl, - hasInitialized, - organizationTicket, - queryParams, - }), - [afterSignInUrl, hasInitialized, organizationTicket, queryParams], - ); - - // Handle redirect scenarios - must be after all hooks - const { isRedirecting } = useAuthRedirect({ - rules: signInRedirectRules, - additionalContext: redirectAdditionalContext, + const { isRedirecting } = useSignInRedirect({ + afterSignInUrl, + organizationTicket, }); - // Mark as initialized after first render - useEffect(() => { - setHasInitialized(true); - }, []); - if (isRedirecting || status.isLoading || clerkStatus === 'sign_up') { // clerkStatus being sign_up will trigger a navigation to the sign up flow, so show a loading card instead of // rendering the sign in flow. diff --git a/packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx b/packages/clerk-js/src/ui/hooks/__tests__/useRedirectEngine.test.tsx similarity index 81% rename from packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx rename to packages/clerk-js/src/ui/hooks/__tests__/useRedirectEngine.test.tsx index 6ee156e8cab..b9fd66e993e 100644 --- a/packages/clerk-js/src/ui/hooks/__tests__/useAuthRedirect.test.tsx +++ b/packages/clerk-js/src/ui/hooks/__tests__/useRedirectEngine.test.tsx @@ -2,7 +2,7 @@ import type { Clerk, EnvironmentResource } from '@clerk/types'; import { renderHook, waitFor } from '@testing-library/react'; import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { useAuthRedirect } from '../useAuthRedirect'; +import { useRedirectEngine } from '../useRedirectEngine'; // Mock dependencies vi.mock('@clerk/shared/react', () => ({ @@ -22,7 +22,6 @@ vi.mock('../../utils/redirectRules', async () => { return { ...actual, evaluateRedirectRules: vi.fn(), - isDevelopmentMode: vi.fn(() => false), }; }); @@ -32,7 +31,7 @@ import { useEnvironment } from '../../contexts'; import { useRouter } from '../../router'; import { evaluateRedirectRules } from '../../utils/redirectRules'; -describe('useAuthRedirect', () => { +describe('useRedirectEngine', () => { const mockNavigate = vi.fn(); const mockClerk = { isSignedIn: false, @@ -52,7 +51,7 @@ describe('useAuthRedirect', () => { it('returns isRedirecting false when no redirect needed', () => { const { result } = renderHook(() => - useAuthRedirect({ + useRedirectEngine({ rules: [], additionalContext: {}, }), @@ -68,7 +67,7 @@ describe('useAuthRedirect', () => { }); const { result } = renderHook(() => - useAuthRedirect({ + useRedirectEngine({ rules: [], additionalContext: {}, }), @@ -88,7 +87,7 @@ describe('useAuthRedirect', () => { }); renderHook(() => - useAuthRedirect({ + useRedirectEngine({ rules: [], additionalContext: {}, }), @@ -99,23 +98,33 @@ describe('useAuthRedirect', () => { }); }); - it('executes onRedirect callback when provided', async () => { - const onRedirect = vi.fn(); + it('handles cleanupQueryParams declaratively', async () => { + const mockReplaceState = vi.fn(); + Object.defineProperty(window, 'history', { + value: { replaceState: mockReplaceState }, + writable: true, + }); + Object.defineProperty(window, 'location', { + value: { search: '?__clerk_add_account=true&other=value', pathname: '/sign-in' }, + writable: true, + }); + (evaluateRedirectRules as any).mockReturnValue({ - destination: '/dashboard', - reason: 'Test redirect', - onRedirect, + destination: '/sign-in', + reason: 'Cleanup params', + skipNavigation: true, + cleanupQueryParams: ['__clerk_add_account'], }); renderHook(() => - useAuthRedirect({ + useRedirectEngine({ rules: [], additionalContext: {}, }), ); await waitFor(() => { - expect(onRedirect).toHaveBeenCalled(); + expect(mockReplaceState).toHaveBeenCalledWith({}, '', '/sign-in?other=value'); }); }); @@ -126,7 +135,7 @@ describe('useAuthRedirect', () => { }; renderHook(() => - useAuthRedirect({ + useRedirectEngine({ rules: [], additionalContext, }), @@ -147,7 +156,7 @@ describe('useAuthRedirect', () => { it('re-evaluates when auth state changes', async () => { const { rerender } = renderHook(() => - useAuthRedirect({ + useRedirectEngine({ rules: [], additionalContext: {}, }), @@ -175,7 +184,7 @@ describe('useAuthRedirect', () => { } const { result } = renderHook(() => - useAuthRedirect({ + useRedirectEngine({ rules: [], additionalContext: { customField: 'test', diff --git a/packages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsx b/packages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsx new file mode 100644 index 00000000000..b4e3db7d120 --- /dev/null +++ b/packages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsx @@ -0,0 +1,72 @@ +import { renderHook } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { useSignInRedirect } from '../useSignInRedirect'; + +vi.mock('../useRedirectEngine', () => ({ + useRedirectEngine: vi.fn(() => ({ isRedirecting: false })), +})); + +vi.mock('../../router', () => ({ + useRouter: vi.fn(() => ({ queryParams: {} })), +})); + +vi.mock('../../utils/redirectRules', () => ({ + signInRedirectRules: [], +})); + +import { useRedirectEngine } from '../useRedirectEngine'; +import { useRouter } from '../../router'; + +describe('useSignInRedirect', () => { + beforeEach(() => { + vi.clearAllMocks(); + (useRouter as any).mockReturnValue({ queryParams: { test: 'value' } }); + }); + + it('calls useRedirectEngine with signInRedirectRules', () => { + renderHook(() => + useSignInRedirect({ + afterSignInUrl: '/dashboard', + organizationTicket: 'test_ticket', + }), + ); + + expect(useRedirectEngine).toHaveBeenCalledWith({ + rules: [], + additionalContext: expect.objectContaining({ + afterSignInUrl: '/dashboard', + organizationTicket: 'test_ticket', + queryParams: { test: 'value' }, + hasInitializedRef: expect.objectContaining({ current: expect.any(Boolean) }), + }), + }); + }); + + it('returns isRedirecting from useRedirectEngine', () => { + (useRedirectEngine as any).mockReturnValue({ isRedirecting: true }); + + const { result } = renderHook(() => + useSignInRedirect({ + afterSignInUrl: '/dashboard', + }), + ); + + expect(result.current.isRedirecting).toBe(true); + }); + + it('sets hasInitializedRef to true after first render', () => { + const { rerender } = renderHook(() => + useSignInRedirect({ + afterSignInUrl: '/dashboard', + }), + ); + + const [[firstCall]] = (useRedirectEngine as any).mock.calls; + const ref = firstCall.additionalContext.hasInitializedRef; + + // After first render, ref should be true + rerender(); + expect(ref.current).toBe(true); + }); +}); diff --git a/packages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsx b/packages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsx new file mode 100644 index 00000000000..451ef5deaef --- /dev/null +++ b/packages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsx @@ -0,0 +1,65 @@ +import { renderHook } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { useSignUpRedirect } from '../useSignUpRedirect'; + +vi.mock('../useRedirectEngine', () => ({ + useRedirectEngine: vi.fn(() => ({ isRedirecting: false })), +})); + +vi.mock('../../router', () => ({ + useRouter: vi.fn(() => ({ queryParams: {} })), +})); + +vi.mock('../../utils/redirectRules', () => ({ + signUpRedirectRules: [], +})); + +import { useRedirectEngine } from '../useRedirectEngine'; +import { useRouter } from '../../router'; + +describe('useSignUpRedirect', () => { + beforeEach(() => { + vi.clearAllMocks(); + (useRouter as any).mockReturnValue({ queryParams: { test: 'value' } }); + }); + + it('calls useRedirectEngine with signUpRedirectRules', () => { + renderHook(() => + useSignUpRedirect({ + afterSignUpUrl: '/onboarding', + }), + ); + + expect(useRedirectEngine).toHaveBeenCalledWith({ + rules: [], + additionalContext: expect.objectContaining({ + afterSignUpUrl: '/onboarding', + queryParams: { test: 'value' }, + }), + }); + }); + + it('returns isRedirecting from useRedirectEngine', () => { + (useRedirectEngine as any).mockReturnValue({ isRedirecting: true }); + + const { result } = renderHook(() => + useSignUpRedirect({ + afterSignUpUrl: '/onboarding', + }), + ); + + expect(result.current.isRedirecting).toBe(true); + }); + + it('does not include hasInitializedRef for SignUp flow', () => { + renderHook(() => + useSignUpRedirect({ + afterSignUpUrl: '/onboarding', + }), + ); + + const [[call]] = (useRedirectEngine as any).mock.calls; + expect(call.additionalContext.hasInitializedRef).toBeUndefined(); + }); +}); diff --git a/packages/clerk-js/src/ui/hooks/index.ts b/packages/clerk-js/src/ui/hooks/index.ts index 4324869cc4c..daba77acc55 100644 --- a/packages/clerk-js/src/ui/hooks/index.ts +++ b/packages/clerk-js/src/ui/hooks/index.ts @@ -1,4 +1,3 @@ -export * from './useAuthRedirect'; export * from './useClerkModalStateParams'; export * from './useClipboard'; export * from './useDebounce'; @@ -17,4 +16,6 @@ export * from './usePrefersReducedMotion'; export * from './useSafeState'; export * from './useScrollLock'; export * from './useSearchInput'; +export * from './useSignInRedirect'; +export * from './useSignUpRedirect'; export * from './useWindowEventListener'; diff --git a/packages/clerk-js/src/ui/hooks/useAuthRedirect.ts b/packages/clerk-js/src/ui/hooks/useRedirectEngine.ts similarity index 54% rename from packages/clerk-js/src/ui/hooks/useAuthRedirect.ts rename to packages/clerk-js/src/ui/hooks/useRedirectEngine.ts index 61cd2a08360..4735ec9cb9c 100644 --- a/packages/clerk-js/src/ui/hooks/useAuthRedirect.ts +++ b/packages/clerk-js/src/ui/hooks/useRedirectEngine.ts @@ -3,28 +3,25 @@ import React from 'react'; import { useEnvironment } from '../contexts'; import { useRouter } from '../router'; -import type { RedirectRule } from '../utils/redirectRules'; +import type { RedirectContext, RedirectRule } from '../utils/redirectRules'; import { evaluateRedirectRules } from '../utils/redirectRules'; -export interface UseAuthRedirectOptions = Record> { - rules: RedirectRule[]; +interface UseRedirectEngineOptions = Record> { additionalContext?: C; + rules: RedirectRule[]; } -export interface UseAuthRedirectReturn { +interface UseRedirectEngineReturn { isRedirecting: boolean; } /** - * Hook to handle authentication redirects based on rules. - * - * **Important**: The `additionalContext` object should be memoized by the caller - * - * @template C - The type of additional context to pass to redirect rules + * Internal redirect engine - use dedicated hooks like useSignInRedirect instead + * @internal */ -export function useAuthRedirect = Record>( - options: UseAuthRedirectOptions, -): UseAuthRedirectReturn { +export function useRedirectEngine = Record>( + options: UseRedirectEngineOptions, +): UseRedirectEngineReturn { const clerk = useClerk(); const environment = useEnvironment(); const { navigate, currentPath } = useRouter(); @@ -36,15 +33,19 @@ export function useAuthRedirect = Record params.delete(param)); + const newSearch = params.toString(); + const newUrl = window.location.pathname + (newSearch ? `?${newSearch}` : ''); + window.history.replaceState({}, '', newUrl); + } - // Only navigate if not explicitly skipped if (!result.skipNavigation) { setIsRedirecting(true); void navigate(result.destination); @@ -53,12 +54,10 @@ export function useAuthRedirect = Record { + hasInitializedRef.current = true; + }, []); + + const additionalContext = useMemo( + () => ({ + afterSignInUrl: options.afterSignInUrl, + hasInitializedRef, + organizationTicket: options.organizationTicket, + queryParams, + }), + [options.afterSignInUrl, options.organizationTicket, queryParams], + ); + + return useRedirectEngine({ + rules: signInRedirectRules, + additionalContext, + }); +} diff --git a/packages/clerk-js/src/ui/hooks/useSignUpRedirect.ts b/packages/clerk-js/src/ui/hooks/useSignUpRedirect.ts new file mode 100644 index 00000000000..13737090523 --- /dev/null +++ b/packages/clerk-js/src/ui/hooks/useSignUpRedirect.ts @@ -0,0 +1,30 @@ +import { useMemo } from 'react'; + +import { useRouter } from '../router'; +import { signUpRedirectRules } from '../utils/redirectRules'; +import { useRedirectEngine } from './useRedirectEngine'; + +export interface UseSignUpRedirectOptions { + afterSignUpUrl?: string; +} + +export interface UseSignUpRedirectReturn { + isRedirecting: boolean; +} + +export function useSignUpRedirect(options: UseSignUpRedirectOptions): UseSignUpRedirectReturn { + const { queryParams } = useRouter(); + + const additionalContext = useMemo( + () => ({ + afterSignUpUrl: options.afterSignUpUrl, + queryParams, + }), + [options.afterSignUpUrl, queryParams], + ); + + return useRedirectEngine({ + rules: signUpRedirectRules, + additionalContext, + }); +} diff --git a/packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts b/packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts index dbce850d54a..d611803fe74 100644 --- a/packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts +++ b/packages/clerk-js/src/ui/utils/__tests__/redirectRules.test.ts @@ -2,7 +2,7 @@ import type { Clerk, EnvironmentResource } from '@clerk/types'; import { describe, expect, it } from 'vitest'; import type { RedirectContext } from '../redirectRules'; -import { evaluateRedirectRules, signInRedirectRules, StopRedirectEvaluation } from '../redirectRules'; +import { evaluateRedirectRules, signInRedirectRules } from '../redirectRules'; describe('evaluateRedirectRules', () => { it('returns null when no rules match', () => { @@ -20,9 +20,9 @@ describe('evaluateRedirectRules', () => { it('returns the first matching rule', () => { const rules = [ - () => null, - () => ({ destination: '/first', reason: 'First rule' }), - () => ({ destination: '/second', reason: 'Second rule' }), + () => [null, false] as const, + () => [{ destination: '/first', reason: 'First rule' }, false] as const, + () => [{ destination: '/second', reason: 'Second rule' }, false] as const, ]; const context: RedirectContext = { @@ -35,12 +35,10 @@ describe('evaluateRedirectRules', () => { expect(result).toEqual({ destination: '/first', reason: 'First rule' }); }); - it('handles StopRedirectEvaluation exception and returns null', () => { + it('handles shouldStop flag and returns null', () => { const rules = [ - () => { - throw new StopRedirectEvaluation('Guard triggered'); - }, - () => ({ destination: '/should-not-reach', reason: 'Should not execute' }), + () => [null, true] as const, + () => [{ destination: '/should-not-reach', reason: 'Should not execute' }, false] as const, ]; const context: RedirectContext = { @@ -52,22 +50,6 @@ describe('evaluateRedirectRules', () => { const result = evaluateRedirectRules(rules, context); expect(result).toBeNull(); }); - - it('re-throws unexpected errors', () => { - const rules = [ - () => { - throw new Error('Unexpected error'); - }, - ]; - - const context: RedirectContext = { - clerk: {} as Clerk, - currentPath: '/sign-in', - environment: {} as EnvironmentResource, - }; - - expect(() => evaluateRedirectRules(rules, context)).toThrow('Unexpected error'); - }); }); describe('signInRedirectRules', () => { @@ -201,7 +183,7 @@ describe('signInRedirectRules', () => { environment: { authConfig: { singleSessionMode: false }, } as EnvironmentResource, - hasInitialized: false, + hasInitializedRef: { current: false }, }; const result = evaluateRedirectRules(signInRedirectRules, context); @@ -222,7 +204,7 @@ describe('signInRedirectRules', () => { environment: { authConfig: { singleSessionMode: false }, } as EnvironmentResource, - hasInitialized: false, + hasInitializedRef: { current: false }, }; const result = evaluateRedirectRules(signInRedirectRules, context); @@ -243,7 +225,7 @@ describe('signInRedirectRules', () => { environment: { authConfig: { singleSessionMode: false }, } as EnvironmentResource, - hasInitialized: true, + hasInitializedRef: { current: true }, }; const result = evaluateRedirectRules(signInRedirectRules, context); @@ -260,7 +242,7 @@ describe('signInRedirectRules', () => { environment: { authConfig: { singleSessionMode: false }, } as EnvironmentResource, - hasInitialized: false, + hasInitializedRef: { current: false }, }; const result = evaluateRedirectRules(signInRedirectRules, context); @@ -280,7 +262,7 @@ describe('signInRedirectRules', () => { environment: { authConfig: { singleSessionMode: true }, } as EnvironmentResource, - hasInitialized: false, + hasInitializedRef: { current: false }, afterSignInUrl: '/custom', }; @@ -311,8 +293,8 @@ describe('signInRedirectRules', () => { destination: '/sign-in', reason: 'User is adding account', skipNavigation: true, + cleanupQueryParams: ['__clerk_add_account'], }); - expect(result?.onRedirect).toBeDefined(); }); it('does not skip navigation when __clerk_add_account query param is absent', () => { diff --git a/packages/clerk-js/src/ui/utils/redirectRules.ts b/packages/clerk-js/src/ui/utils/redirectRules.ts index 5dcda25667c..4af94ba45ef 100644 --- a/packages/clerk-js/src/ui/utils/redirectRules.ts +++ b/packages/clerk-js/src/ui/utils/redirectRules.ts @@ -1,71 +1,72 @@ import type { Clerk, EnvironmentResource } from '@clerk/types'; +import type React from 'react'; import { debugLogger } from '../../utils/debug'; export interface RedirectContext { readonly afterSignInUrl?: string; + readonly afterSignUpUrl?: string; readonly clerk: Clerk; readonly currentPath: string; readonly environment: EnvironmentResource; - readonly hasInitialized?: boolean; + readonly hasInitializedRef?: React.MutableRefObject; readonly organizationTicket?: string; readonly queryParams?: Record; - readonly [key: string]: any; } export interface RedirectResult { readonly destination: string; readonly reason: string; readonly skipNavigation?: boolean; - readonly onRedirect?: () => void; + readonly cleanupQueryParams?: string[]; } -export type RedirectRule = (context: RedirectContext) => RedirectResult | null; +export type RedirectRule = Record> = ( + context: RedirectContext & T, +) => readonly [RedirectResult | null, boolean]; -/** - * Thrown by rules to stop evaluation without redirecting. - * Used for guard conditions that prevent redirects. - */ -export class StopRedirectEvaluation extends Error { - constructor(public readonly reason: string) { - super(reason); - this.name = 'StopRedirectEvaluation'; +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; } } /** * Evaluates redirect rules in order, returning the first match. - * Rules can throw StopRedirectEvaluation to short-circuit without redirecting. * * @param rules - Array of redirect rules to evaluate * @param context - Context containing clerk instance, path, environment, etc. * @returns The first matching redirect result, or null if no rules match */ -export function evaluateRedirectRules(rules: RedirectRule[], context: RedirectContext): RedirectResult | null { - try { - for (const rule of rules) { - const result = rule(context); - if (result) { - debugLogger.info( - 'Redirect rule matched', - { - destination: result.destination, - reason: result.reason, - }, - 'redirect', - ); - return result; - } - } - return null; - } catch (error) { - if (error instanceof StopRedirectEvaluation) { - debugLogger.info('Redirect evaluation stopped', { reason: error.reason }, 'redirect'); +export function evaluateRedirectRules = Record>( + rules: RedirectRule[], + context: RedirectContext & T, +): RedirectResult | null { + for (const rule of rules) { + const [result, shouldStop] = rule(context); + + if (shouldStop) { + debugLogger.info('Redirect evaluation stopped', { reason: 'Guard triggered' }, 'redirect'); return null; } - // Re-throw unexpected errors - throw error; + + if (result) { + debugLogger.info( + 'Redirect rule matched', + { + destination: result.destination, + reason: result.reason, + }, + 'redirect', + ); + return result; + } } + return null; } /** @@ -75,62 +76,91 @@ export const signInRedirectRules: RedirectRule[] = [ // Guard: Organization ticket flow is handled separately in component ctx => { if (ctx.organizationTicket) { - throw new StopRedirectEvaluation('Organization ticket flow is handled separately'); + return [null, true]; } - return null; + return [null, false]; }, // Rule 1: Single session mode - user already signed in ctx => { if (ctx.clerk.isSignedIn && ctx.environment.authConfig.singleSessionMode) { - const destination = ctx.afterSignInUrl || ctx.clerk.buildAfterSignInUrl(); - return { - destination, - reason: 'User already signed in (single session mode)', - }; + let destination = ctx.afterSignInUrl || ctx.clerk.buildAfterSignInUrl(); + + if (ctx.afterSignInUrl && !isValidRedirectUrl(ctx.afterSignInUrl)) { + destination = ctx.clerk.buildAfterSignInUrl(); + } + + return [ + { + destination, + reason: 'User already signed in (single session mode)', + }, + false, + ]; } - return null; + return [null, false]; }, // Rule 2: Skip redirect if adding account (preserves add account flow) ctx => { const isAddingAccount = ctx.queryParams?.['__clerk_add_account']; if (isAddingAccount) { - return { - destination: ctx.currentPath, - reason: 'User is adding account', - skipNavigation: true, - onRedirect: () => { - // Clean up query param (platform-specific) - if (typeof window !== 'undefined' && window.history) { - const params = new URLSearchParams(window.location.search); - params.delete('__clerk_add_account'); - const newSearch = params.toString(); - const newUrl = window.location.pathname + (newSearch ? `?${newSearch}` : ''); - window.history.replaceState({}, '', newUrl); - } + return [ + { + destination: ctx.currentPath, + reason: 'User is adding account', + skipNavigation: true, + cleanupQueryParams: ['__clerk_add_account'], }, - }; + false, + ]; } - return null; + return [null, false]; }, // Rule 3: Multi-session mode - redirect to account switcher with active sessions ctx => { - // Only apply on first initialization to prevent redirect loops - if (ctx.hasInitialized) { - return null; + if (ctx.hasInitializedRef?.current) { + return [null, false]; } const isMultiSessionMode = !ctx.environment.authConfig.singleSessionMode; const hasActiveSessions = (ctx.clerk.client?.signedInSessions?.length ?? 0) > 0; if (hasActiveSessions && isMultiSessionMode) { - return { - destination: 'choose', - reason: 'Active sessions detected (multi-session mode)', - }; + return [ + { + destination: 'choose', + reason: 'Active sessions detected (multi-session mode)', + }, + false, + ]; + } + return [null, false]; + }, +]; + +/** + * Redirect rules for SignUp component + */ +export const signUpRedirectRules: RedirectRule[] = [ + // Rule 1: Single session mode - user already signed in + ctx => { + if (ctx.clerk.isSignedIn && ctx.environment.authConfig.singleSessionMode) { + let destination = ctx.afterSignUpUrl || ctx.clerk.buildAfterSignUpUrl(); + + if (ctx.afterSignUpUrl && !isValidRedirectUrl(ctx.afterSignUpUrl)) { + destination = ctx.clerk.buildAfterSignUpUrl(); + } + + return [ + { + destination, + reason: 'User already signed in (single session mode)', + }, + false, + ]; } - return null; + return [null, false]; }, ]; From 7b3316768205d1788db13d1e663c7dfeb2458627 Mon Sep 17 00:00:00 2001 From: Jacek Date: Mon, 27 Oct 2025 13:01:24 -0500 Subject: [PATCH 13/13] lint + format --- .../src/ui/hooks/__tests__/useSignInRedirect.test.tsx | 2 +- .../src/ui/hooks/__tests__/useSignUpRedirect.test.tsx | 2 +- packages/clerk-js/src/ui/utils/redirectRules.ts | 4 +++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsx b/packages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsx index b4e3db7d120..a6504f4113a 100644 --- a/packages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsx +++ b/packages/clerk-js/src/ui/hooks/__tests__/useSignInRedirect.test.tsx @@ -15,8 +15,8 @@ vi.mock('../../utils/redirectRules', () => ({ signInRedirectRules: [], })); -import { useRedirectEngine } from '../useRedirectEngine'; import { useRouter } from '../../router'; +import { useRedirectEngine } from '../useRedirectEngine'; describe('useSignInRedirect', () => { beforeEach(() => { diff --git a/packages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsx b/packages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsx index 451ef5deaef..d1ad75f4303 100644 --- a/packages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsx +++ b/packages/clerk-js/src/ui/hooks/__tests__/useSignUpRedirect.test.tsx @@ -15,8 +15,8 @@ vi.mock('../../utils/redirectRules', () => ({ signUpRedirectRules: [], })); -import { useRedirectEngine } from '../useRedirectEngine'; import { useRouter } from '../../router'; +import { useRedirectEngine } from '../useRedirectEngine'; describe('useSignUpRedirect', () => { beforeEach(() => { diff --git a/packages/clerk-js/src/ui/utils/redirectRules.ts b/packages/clerk-js/src/ui/utils/redirectRules.ts index 4af94ba45ef..a1e0b3251c6 100644 --- a/packages/clerk-js/src/ui/utils/redirectRules.ts +++ b/packages/clerk-js/src/ui/utils/redirectRules.ts @@ -27,7 +27,9 @@ export type RedirectRule = Record