feat(clerk-js): Introduce navigate for setActive
#6486
Conversation
🦋 Changeset detectedLatest commit: fa3a908 The changes in this PR will be included in the next version bump. This PR includes changesets to release 22 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
LauraBeatris
changed the title
onPendingStatusonPendingSession
LauraBeatris
force-pushed
the
laura/introduce-on-pending-session
branch
from
970cf52 to
cd73364
Compare
LauraBeatris
force-pushed
the
laura/introduce-on-pending-session
branch
from
c3ad953 to
45a338c
Compare
LauraBeatris
force-pushed
the
laura/introduce-on-pending-session
branch
from
beb0443 to
1b3771c
Compare
clerk-cookie
added
remix
tanstack
react-router
vue
and removed
remix
tanstack
react-router
vue
labels
LauraBeatris
force-pushed
the
laura/introduce-on-pending-session
branch
from
d270d37 to
d185134
Compare
LauraBeatris
force-pushed
the
laura/introduce-on-pending-session
branch
from
fe1c8b7 to
2b13e93
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (12)
integration/tests/session-tasks-sign-up.test.ts (2)
37-46: Guard against missing OAuth provider instance keys in teardowncreateClerkClient will throw if instanceKeys.get('oauth-provider') is undefined or has missing keys. Add a guard to avoid teardown failures in CI matrices without those secrets.
Apply this diff:
- const client = createClerkClient({ - secretKey: instanceKeys.get('oauth-provider').sk, - publishableKey: instanceKeys.get('oauth-provider').pk, - }); - const users = createUserService(client); - await users.deleteIfExists({ email: fakeUserForOAuth.email }); + const oauthKeys = instanceKeys.get('oauth-provider'); + if (oauthKeys?.sk && oauthKeys?.pk) { + const client = createClerkClient({ + secretKey: oauthKeys.sk, + publishableKey: oauthKeys.pk, + }); + const users = createUserService(client); + await users.deleteIfExists({ email: fakeUserForOAuth.email }); + }
95-97: Wait for session-tasks view before resolving the taskAfter OTP, the actionable UI is the session tasks screen. Waiting on SignUp can race and cause flakiness. Prefer waiting on sessionTask.
Apply this diff:
- // Resolves task - await u.po.signUp.waitForMounted(); + // Resolves task + await u.po.sessionTask.waitForMounted();If sessionTask.waitForMounted() isn’t available yet, add it to the POM (page-objects/sessionTask.ts) to wait for a stable selector (e.g.,
.cl-sessionTasks-root).packages/clerk-js/src/ui/contexts/components/SignUp.ts (1)
118-125: Preserve redirectUrl and prefer taskUrls override in navigateOnSetActive; avoid route-map couplingnavigateOnSetActive drops the redirectUrl and ignores provider-level taskUrls. It also depends on INTERNAL_SESSION_TASK_ROUTE_BY_KEY, which can drift from core URL builders. This diverges from the PR goal to preserve redirectUrl and respect taskUrls.
Apply this diff to unify with the taskUrl computation and preserve redirect/auth params:
- const navigateOnSetActive = async ({ session, redirectUrl }: { session: SessionResource; redirectUrl: string }) => { - const currentTask = session.currentTask; - if (!currentTask) { - return navigate(redirectUrl); - } - - return navigate(`/${basePath}/tasks/${INTERNAL_SESSION_TASK_ROUTE_BY_KEY[currentTask.key]}`); - }; + const navigateOnSetActive = async ({ session, redirectUrl }: { session: SessionResource; redirectUrl: string }) => { + const currentTask = session.currentTask; + if (!currentTask) { + return navigate(redirectUrl); + } + const customTaskUrl = clerk.__internal_getOption('taskUrls')?.[currentTask.key]; + const defaultTaskUrl = buildTaskRedirectUrl(currentTask, { + routing: ctx.routing, + baseUrl: signUpUrl, + authQueryString, + path: ctx.path, + }); + const baseTaskUrl = customTaskUrl ?? defaultTaskUrl; + const nextUrl = buildURL( + { + base: baseTaskUrl, + hashSearchParams: [{ redirect_url: redirectUrl }, redirectUrls.getPreservedSearchParams()], + }, + { stringify: true }, + ); + return navigate(nextUrl); + };Additionally, once the above change is applied, you can drop the INTERNAL_SESSION_TASK_ROUTE_BY_KEY import since the route-map is no longer used.
packages/clerk-js/src/core/__tests__/clerk.test.ts (1)
564-578: Add test coverage for preserving redirectUrlThe test correctly verifies navigation to a task URL. However, based on past review comments, you should also add a test that verifies
redirectUrlpassed tosetActiveis preserved when navigating to ataskUrl.#!/bin/bash # Check if there are tests that verify redirectUrl preservation with taskUrls rg -A 10 "redirectUrl.*taskUrls?" packages/clerk-js/src/core/__tests__/clerk.test.tspackages/clerk-js/src/ui/contexts/components/SignIn.ts (4)
40-43: Remove duplicate useRouter() callYou're calling
useRouter()twice - once on line 40 and again on line 42. Consolidate to a single call.- const { navigate, basePath } = useRouter(); const { displayConfig, userSettings } = useEnvironment(); - const { queryParams, queryString } = useRouter(); + const { navigate, basePath, queryParams, queryString } = useRouter();
123-130: Simplify navigation logic and respect taskUrls configurationThe current implementation uses a hard-coded relative path construction which:
- Doesn't respect
taskUrlsconfiguration from ClerkProvider- Uses potentially error-prone relative path construction with
basePathconst navigateOnSetActive = async ({ session, redirectUrl }: { session: SessionResource; redirectUrl: string }) => { const currentTask = session.currentTask; if (!currentTask) { return navigate(redirectUrl); } - return navigate(`/${basePath}/tasks/${INTERNAL_SESSION_TASK_ROUTE_BY_KEY[currentTask.key]}`); + const customTaskUrl = clerk.__internal_getOption('taskUrls')?.[currentTask.key]; + const defaultTaskUrl = buildTaskRedirectUrl(currentTask, { + routing: ctx.routing, + baseUrl: signInUrl, + authQueryString, + path: ctx.path, + }); + return navigate(customTaskUrl ?? defaultTaskUrl); };
132-140: Use newSession instead of clerk.session for taskUrl computationThe
taskUrlis computed usingclerk.sessionwhich may be outdated when switching to a new pending session. Use the newly resolved session object instead.This issue was already identified in previous reviews. The taskUrl should be computed from the newSession that's being activated, not from the potentially stale clerk.session.
32-34: Make redirectUrl optional in navigateOnSetActive signatureThe
redirectUrlparameter should be optional since it may not always be provided tosetActive.- navigateOnSetActive: (opts: { session: SessionResource; redirectUrl: string }) => Promise<unknown>; + navigateOnSetActive: (opts: { session: SessionResource; redirectUrl?: string }) => Promise<void>;packages/clerk-js/src/core/clerk.ts (4)
1217-1220: Add proper error handling for session ID lookupThe type assertion could cause runtime issues if the session is not found.
if (typeof session === 'string') { - session = (this.client.sessions.find(x => x.id === session) as SignedInSessionResource) || null; + const foundSession = this.client.sessions.find(x => x.id === session); + if (!foundSession) { + throw new Error(`Session with ID "${session}" not found.`); + } + session = foundSession as SignedInSessionResource; }
1232-1234: Pass merged options to avoid false positive warningsThe warning function only receives
paramswhich doesn't includetaskUrlsfromthis.#options, potentially causing false positive warnings.- warnMissingPendingTaskHandlers(params); + warnMissingPendingTaskHandlers({ ...this.#options, ...params });
1305-1308: Use newSession instead of this.session for taskUrlThe
taskUrlis incorrectly derived from the old session. UsenewSessionwhich is already in scope.const taskUrl = newSession?.status === 'pending' && newSession?.currentTask && - this.#options.taskUrls?.[this.session?.currentTask.key]; + this.#options.taskUrls?.[newSession.currentTask.key];
1321-1337: Add error handling for navigate callback and fix duplicate navigationTwo issues:
- The navigate callback should be wrapped in try-catch to prevent leaving Clerk in an inconsistent state
- There's duplicate navigation to redirectUrl at lines 1335 and 1336
if (taskUrl) { const taskUrlWithRedirect = redirectUrl ? buildURL({ base: taskUrl, hashSearchParams: { redirectUrl } }, { stringify: true }) : taskUrl; await this.navigate(taskUrlWithRedirect); } else if (setActiveNavigate && newSession) { - await setActiveNavigate({ session: newSession }); + try { + await setActiveNavigate({ session: newSession }); + } catch (err) { + logger.error('setActive.navigate callback failed', err); + } } else if (redirectUrl) { if (this.client.isEligibleForTouch()) { const absoluteRedirectUrl = new URL(redirectUrl, window.location.href); const redirectUrlWithAuth = this.buildUrlWithAuth( this.client.buildTouchUrl({ redirectUrl: absoluteRedirectUrl }), ); await this.navigate(redirectUrlWithAuth); + return; } + await this.navigate(redirectUrl); } -await this.navigate(redirectUrl);
🧹 Nitpick comments (8)
packages/clerk-js/src/ui/contexts/components/SignUp.ts (1)
39-42: Avoid multiple useRouter() callsYou call useRouter() twice to get different properties. Prefer a single call and destructure all needed fields to avoid redundant hook invocations.
Apply this refactor nearby:
- const { navigate, basePath } = useRouter(); - const { displayConfig, userSettings } = useEnvironment(); - const { queryParams, queryString } = useRouter(); + const { navigate, basePath, queryParams, queryString } = useRouter(); + const { displayConfig, userSettings } = useEnvironment();packages/clerk-js/src/core/__tests__/clerk.test.ts (2)
975-991: Inconsistent test name with implementationThe test is named "with pending session" but the describe block starting from line 975 includes general after-auth flow configuration, not just pending sessions.
Consider either:
- Renaming the describe block to reflect the broader scope (e.g., "with after-auth flows")
- Or moving the forceOrganizationSelection setup to a more appropriate location
1506-1507: Type inconsistency in test dataThe
mockSessionobject is defined withstatus: 'active'on line 1485, but the test data on lines 1506-1507 includes it insignedInSessionsarray. This seems intentional but could benefit from a comment explaining why an active session appears in the context of testing pending session handling.Consider adding a comment to clarify the test setup:
mockClientFetch.mockReturnValue( Promise.resolve({ sessions: [mockSession], + // Active session included to test navigation behavior signedInSessions: [mockSession],packages/clerk-js/src/core/sessionTasks.ts (3)
14-14: Consider adding explicit return typeAdd an explicit return type for better TypeScript clarity.
-const getTaskEndpoint = (task: SessionTask) => `/tasks/${INTERNAL_SESSION_TASK_ROUTE_BY_KEY[task.key]}`; +const getTaskEndpoint = (task: SessionTask): string => `/tasks/${INTERNAL_SESSION_TASK_ROUTE_BY_KEY[task.key]}`;
19-33: Add explicit return typeAdd an explicit return type for consistency with TypeScript best practices.
export function buildTaskRedirectUrl( task: SessionTask, opts: Omit<Parameters<typeof buildRedirectUrl>[0], 'endpoint'>, -) { +): string {
38-47: Add explicit return typeAdd an explicit return type for consistency.
export function buildTaskUrl( task: SessionTask, { baseUrl, }: { baseUrl: string; }, -) { +): string {packages/clerk-js/src/core/clerk.ts (2)
1554-1574: Consider preserving redirectUrl in buildTasksUrlThe
buildTasksUrlmethod doesn't preserve anyredirectUrlthat might have been passed tosetActive. Consider whether this should be included for consistency with task navigation.public buildTasksUrl(): string { const currentTask = this.session?.currentTask; if (!currentTask) { return ''; } const customTaskUrl = this.#options.taskUrls?.[currentTask.key]; if (customTaskUrl) { return customTaskUrl; } return buildURL( { base: this.buildSignInUrl(), hashPath: `/tasks/${INTERNAL_SESSION_TASK_ROUTE_BY_KEY[currentTask.key]}`, }, { stringify: true, }, ); }
1995-1997: Consider using setActive for pending session navigationWhen there's a pending session task, consider using
setActivewith the navigate callback for consistency instead of direct navigation.if (this.session?.currentTask) { - return this.navigate(params.taskUrl ?? this.buildTasksUrl()); + return this.setActive({ + session: this.session, + navigate: async ({ session }) => { + await this.navigate(params.taskUrl ?? this.buildTasksUrl()); + } + }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
packages/react-router/src/__tests__/__snapshots__/exports.test.ts.snapis excluded by!**/*.snappackages/remix/src/__tests__/__snapshots__/exports.test.ts.snapis excluded by!**/*.snappackages/tanstack-react-start/src/__tests__/__snapshots__/exports.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (48)
.changeset/rich-donuts-agree.md(1 hunks).changeset/warm-rocks-flow.md(1 hunks)integration/tests/session-tasks-sign-up.test.ts(4 hunks)packages/clerk-js/bundlewatch.config.json(1 hunks)packages/clerk-js/src/core/__tests__/clerk.test.ts(8 hunks)packages/clerk-js/src/core/clerk.ts(23 hunks)packages/clerk-js/src/core/resources/SignIn.ts(1 hunks)packages/clerk-js/src/core/resources/SignUp.ts(1 hunks)packages/clerk-js/src/core/sessionTasks.ts(1 hunks)packages/clerk-js/src/ui/common/redirects.ts(0 hunks)packages/clerk-js/src/ui/common/withRedirect.tsx(3 hunks)packages/clerk-js/src/ui/components/SessionTasks/index.tsx(5 hunks)packages/clerk-js/src/ui/components/SessionTasks/tasks/TaskChooseOrganization/ChooseOrganizationScreen.tsx(3 hunks)packages/clerk-js/src/ui/components/SessionTasks/tasks/TaskChooseOrganization/CreateOrganizationScreen.tsx(3 hunks)packages/clerk-js/src/ui/components/SignIn/SignInAccountSwitcher.tsx(2 hunks)packages/clerk-js/src/ui/components/SignIn/SignInFactorOne.tsx(2 hunks)packages/clerk-js/src/ui/components/SignIn/SignInFactorOneAlternativeChannelCodeForm.tsx(2 hunks)packages/clerk-js/src/ui/components/SignIn/SignInFactorOneCodeForm.tsx(2 hunks)packages/clerk-js/src/ui/components/SignIn/SignInFactorOnePasswordCard.tsx(2 hunks)packages/clerk-js/src/ui/components/SignIn/SignInFactorTwo.tsx(2 hunks)packages/clerk-js/src/ui/components/SignIn/SignInFactorTwoBackupCodeCard.tsx(2 hunks)packages/clerk-js/src/ui/components/SignIn/SignInFactorTwoCodeForm.tsx(2 hunks)packages/clerk-js/src/ui/components/SignIn/SignInSSOCallback.tsx(1 hunks)packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx(6 hunks)packages/clerk-js/src/ui/components/SignIn/__tests__/handleCombinedFlowTransfer.test.ts(5 hunks)packages/clerk-js/src/ui/components/SignIn/handleCombinedFlowTransfer.ts(4 hunks)packages/clerk-js/src/ui/components/SignIn/index.tsx(2 hunks)packages/clerk-js/src/ui/components/SignIn/shared.ts(2 hunks)packages/clerk-js/src/ui/components/SignUp/SignUpContinue.tsx(2 hunks)packages/clerk-js/src/ui/components/SignUp/SignUpEmailLinkCard.tsx(2 hunks)packages/clerk-js/src/ui/components/SignUp/SignUpSSOCallback.tsx(1 hunks)packages/clerk-js/src/ui/components/SignUp/SignUpStart.tsx(5 hunks)packages/clerk-js/src/ui/components/SignUp/SignUpVerificationCodeForm.tsx(2 hunks)packages/clerk-js/src/ui/components/SignUp/index.tsx(2 hunks)packages/clerk-js/src/ui/components/UserButton/useMultisessionActions.tsx(3 hunks)packages/clerk-js/src/ui/contexts/ClerkUIComponentsContext.tsx(2 hunks)packages/clerk-js/src/ui/contexts/components/SignIn.ts(4 hunks)packages/clerk-js/src/ui/contexts/components/SignUp.ts(4 hunks)packages/clerk-js/src/ui/lazyModules/components.ts(1 hunks)packages/clerk-js/src/ui/types.ts(2 hunks)packages/clerk-js/src/utils/componentGuards.ts(1 hunks)packages/nextjs/src/client-boundary/controlComponents.ts(1 hunks)packages/nextjs/src/index.ts(0 hunks)packages/react/src/components/controlComponents.tsx(2 hunks)packages/react/src/components/index.ts(1 hunks)packages/react/src/isomorphicClerk.ts(2 hunks)packages/types/src/clerk.ts(6 hunks)packages/vue/src/components/controlComponents.ts(2 hunks)
💤 Files with no reviewable changes (2)
- packages/nextjs/src/index.ts
- packages/clerk-js/src/ui/common/redirects.ts
🚧 Files skipped from review as they are similar to previous changes (36)
- packages/react/src/components/index.ts
- packages/react/src/components/controlComponents.tsx
- packages/clerk-js/src/ui/components/SignUp/SignUpSSOCallback.tsx
- packages/clerk-js/src/ui/common/withRedirect.tsx
- .changeset/rich-donuts-agree.md
- packages/clerk-js/src/ui/components/SignIn/handleCombinedFlowTransfer.ts
- packages/clerk-js/src/ui/components/SignIn/SignInFactorOne.tsx
- packages/clerk-js/src/ui/components/SignIn/SignInSSOCallback.tsx
- packages/clerk-js/src/ui/components/SignIn/SignInFactorTwo.tsx
- packages/clerk-js/src/ui/lazyModules/components.ts
- packages/clerk-js/src/ui/components/SignIn/SignInFactorTwoCodeForm.tsx
- packages/clerk-js/src/ui/types.ts
- packages/vue/src/components/controlComponents.ts
- .changeset/warm-rocks-flow.md
- packages/clerk-js/src/ui/components/SignIn/SignInFactorOneAlternativeChannelCodeForm.tsx
- packages/clerk-js/src/ui/components/SignUp/SignUpStart.tsx
- packages/clerk-js/src/ui/components/SignUp/SignUpContinue.tsx
- packages/clerk-js/src/ui/components/SignIn/shared.ts
- packages/clerk-js/src/ui/components/SignIn/index.tsx
- packages/clerk-js/src/ui/components/SignIn/SignInFactorTwoBackupCodeCard.tsx
- packages/clerk-js/src/ui/components/SignIn/SignInAccountSwitcher.tsx
- packages/clerk-js/src/ui/components/SessionTasks/index.tsx
- packages/clerk-js/src/utils/componentGuards.ts
- packages/clerk-js/src/ui/components/SignUp/SignUpVerificationCodeForm.tsx
- packages/clerk-js/src/ui/components/SignIn/SignInFactorOnePasswordCard.tsx
- packages/clerk-js/src/core/resources/SignUp.ts
- packages/clerk-js/bundlewatch.config.json
- packages/clerk-js/src/ui/components/SessionTasks/tasks/TaskChooseOrganization/CreateOrganizationScreen.tsx
- packages/clerk-js/src/ui/components/SignUp/SignUpEmailLinkCard.tsx
- packages/clerk-js/src/ui/components/SignIn/tests/handleCombinedFlowTransfer.test.ts
- packages/clerk-js/src/ui/contexts/ClerkUIComponentsContext.tsx
- packages/clerk-js/src/ui/components/SignIn/SignInFactorOneCodeForm.tsx
- packages/types/src/clerk.ts
- packages/nextjs/src/client-boundary/controlComponents.ts
- packages/clerk-js/src/ui/components/SessionTasks/tasks/TaskChooseOrganization/ChooseOrganizationScreen.tsx
- packages/react/src/isomorphicClerk.ts
🧰 Additional context used
📓 Path-based instructions (16)
packages/clerk-js/src/ui/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/clerk-js-ui.mdc)
packages/clerk-js/src/ui/**/*.{ts,tsx}: Element descriptors should always be camelCase
Use element descriptors in UI components to enable consistent theming and styling via appearance.elements
Element descriptors should generate unique, stable CSS classes for theming
Element descriptors should handle state classes (e.g., cl-loading, cl-active, cl-error, cl-open) automatically based on component state
Do not render hard-coded values; all user-facing strings must be localized using provided localization methods
Use the useLocalizations hook and localizationKeys utility for all text and error messages
Use the styled system (sx prop, theme tokens, responsive values) for custom component styling
Use useCardState for card-level state, useFormState for form-level state, and useLoadingStatus for loading states
Always use handleError utility for API errors and use translateError for localized error messages
Use useFormControl for form field state, implement proper validation, and handle loading and error states in forms
Use localization keys for all form labels and placeholders
Use element descriptors for consistent styling and follow the theme token system
Use the Card and FormContainer patterns for consistent UI structure
Files:
packages/clerk-js/src/ui/components/UserButton/useMultisessionActions.tsxpackages/clerk-js/src/ui/contexts/components/SignUp.tspackages/clerk-js/src/ui/contexts/components/SignIn.tspackages/clerk-js/src/ui/components/SignUp/index.tsxpackages/clerk-js/src/ui/components/SignIn/SignInStart.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/clerk-js/src/ui/components/UserButton/useMultisessionActions.tsxpackages/clerk-js/src/core/resources/SignIn.tspackages/clerk-js/src/ui/contexts/components/SignUp.tsintegration/tests/session-tasks-sign-up.test.tspackages/clerk-js/src/ui/contexts/components/SignIn.tspackages/clerk-js/src/core/__tests__/clerk.test.tspackages/clerk-js/src/ui/components/SignUp/index.tsxpackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/core/sessionTasks.tspackages/clerk-js/src/core/clerk.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/clerk-js/src/ui/components/UserButton/useMultisessionActions.tsxpackages/clerk-js/src/core/resources/SignIn.tspackages/clerk-js/src/ui/contexts/components/SignUp.tsintegration/tests/session-tasks-sign-up.test.tspackages/clerk-js/src/ui/contexts/components/SignIn.tspackages/clerk-js/src/core/__tests__/clerk.test.tspackages/clerk-js/src/ui/components/SignUp/index.tsxpackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/core/sessionTasks.tspackages/clerk-js/src/core/clerk.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/ui/components/UserButton/useMultisessionActions.tsxpackages/clerk-js/src/core/resources/SignIn.tspackages/clerk-js/src/ui/contexts/components/SignUp.tspackages/clerk-js/src/ui/contexts/components/SignIn.tspackages/clerk-js/src/core/__tests__/clerk.test.tspackages/clerk-js/src/ui/components/SignUp/index.tsxpackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/core/sessionTasks.tspackages/clerk-js/src/core/clerk.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/clerk-js/src/ui/components/UserButton/useMultisessionActions.tsxpackages/clerk-js/src/core/resources/SignIn.tspackages/clerk-js/src/ui/contexts/components/SignUp.tspackages/clerk-js/src/ui/contexts/components/SignIn.tspackages/clerk-js/src/core/__tests__/clerk.test.tspackages/clerk-js/src/ui/components/SignUp/index.tsxpackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/core/sessionTasks.tspackages/clerk-js/src/core/clerk.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/clerk-js/src/ui/components/UserButton/useMultisessionActions.tsxpackages/clerk-js/src/core/resources/SignIn.tspackages/clerk-js/src/ui/contexts/components/SignUp.tsintegration/tests/session-tasks-sign-up.test.tspackages/clerk-js/src/ui/contexts/components/SignIn.tspackages/clerk-js/src/core/__tests__/clerk.test.tspackages/clerk-js/src/ui/components/SignUp/index.tsxpackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/core/sessionTasks.tspackages/clerk-js/src/core/clerk.ts
**/*.{jsx,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
**/*.{jsx,tsx}: Use error boundaries in React components
Minimize re-renders in React components
**/*.{jsx,tsx}: Always use functional components with hooks instead of class components
Follow PascalCase naming for components:UserProfile,NavigationMenu
Keep components focused on a single responsibility - split large components
Limit component size to 150-200 lines; extract logic into custom hooks
Use composition over inheritance - prefer smaller, composable components
Export components as named exports for better tree-shaking
One component per file with matching filename and component name
Use useState for simple state management
Use useReducer for complex state logic
Implement proper state initialization
Use proper state updates with callbacks
Implement proper state cleanup
Use Context API for theme/authentication
Implement proper state selectors
Use proper state normalization
Implement proper state persistence
Use React.memo for expensive components
Implement proper useCallback for handlers
Use proper useMemo for expensive computations
Implement proper virtualization for lists
Use proper code splitting with React.lazy
Implement proper cleanup in useEffect
Use proper refs for DOM access
Implement proper event listener cleanup
Use proper abort controllers for fetch
Implement proper subscription cleanup
Use proper HTML elements
Implement proper ARIA attributes
Use proper heading hierarchy
Implement proper form labels
Use proper button types
Implement proper focus management
Use proper keyboard shortcuts
Implement proper tab order
Use proper skip links
Implement proper focus traps
Implement proper error boundaries
Use proper error logging
Implement proper error recovery
Use proper error messages
Implement proper error fallbacks
Use proper form validation
Implement proper error states
Use proper error messages
Implement proper form submission
Use proper form reset
Use proper component naming
Implement proper file naming
Use proper prop naming
Implement proper...
Files:
packages/clerk-js/src/ui/components/UserButton/useMultisessionActions.tsxpackages/clerk-js/src/ui/components/SignUp/index.tsxpackages/clerk-js/src/ui/components/SignIn/SignInStart.tsx
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/clerk-js/src/ui/components/UserButton/useMultisessionActions.tsxpackages/clerk-js/src/core/resources/SignIn.tspackages/clerk-js/src/ui/contexts/components/SignUp.tsintegration/tests/session-tasks-sign-up.test.tspackages/clerk-js/src/ui/contexts/components/SignIn.tspackages/clerk-js/src/core/__tests__/clerk.test.tspackages/clerk-js/src/ui/components/SignUp/index.tsxpackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/core/sessionTasks.tspackages/clerk-js/src/core/clerk.ts
**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/react.mdc)
**/*.tsx: Use proper type definitions for props and state
Leverage TypeScript's type inference where possible
Use proper event types for handlers
Implement proper generic types for reusable components
Use proper type guards for conditional rendering
Files:
packages/clerk-js/src/ui/components/UserButton/useMultisessionActions.tsxpackages/clerk-js/src/ui/components/SignUp/index.tsxpackages/clerk-js/src/ui/components/SignIn/SignInStart.tsx
**/*
⚙️ CodeRabbit Configuration File
If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.
Files:
packages/clerk-js/src/ui/components/UserButton/useMultisessionActions.tsxpackages/clerk-js/src/core/resources/SignIn.tspackages/clerk-js/src/ui/contexts/components/SignUp.tsintegration/tests/session-tasks-sign-up.test.tspackages/clerk-js/src/ui/contexts/components/SignIn.tspackages/clerk-js/src/core/__tests__/clerk.test.tspackages/clerk-js/src/ui/components/SignUp/index.tsxpackages/clerk-js/src/ui/components/SignIn/SignInStart.tsxpackages/clerk-js/src/core/sessionTasks.tspackages/clerk-js/src/core/clerk.ts
integration/**
📄 CodeRabbit Inference Engine (.cursor/rules/global.mdc)
Framework integration templates and E2E tests should be placed under the integration/ directory
Files:
integration/tests/session-tasks-sign-up.test.ts
integration/**/*
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
End-to-end tests and integration templates must be located in the 'integration/' directory.
Files:
integration/tests/session-tasks-sign-up.test.ts
integration/**/*.{test,spec}.{js,ts}
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Integration tests should use Playwright.
Files:
integration/tests/session-tasks-sign-up.test.ts
packages/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Unit tests should use Jest or Vitest as the test runner.
Files:
packages/clerk-js/src/core/__tests__/clerk.test.ts
packages/{clerk-js,elements,themes}/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Visual regression testing should be performed for UI components.
Files:
packages/clerk-js/src/core/__tests__/clerk.test.ts
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)
**/__tests__/**/*.{ts,tsx}: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces
Files:
packages/clerk-js/src/core/__tests__/clerk.test.ts
🧠 Learnings (1)
📚 Learning: 2025-08-08T19:00:08.931Z
Learnt from: LauraBeatris
PR: clerk/javascript#6486
File: packages/clerk-js/src/core/clerk.ts:1303-1344
Timestamp: 2025-08-08T19:00:08.931Z
Learning: In Clerk's navigation system for pending sessions, `taskUrls` (configured at the ClerkProvider level) should take priority over the `navigate` callback (from All-In-One components). This ensures that explicit provider-level configuration overrides component-level navigation logic, providing developers with predictable control over navigation behavior.
Applied to files:
packages/clerk-js/src/core/clerk.ts
🧬 Code Graph Analysis (8)
packages/clerk-js/src/ui/components/UserButton/useMultisessionActions.tsx (3)
packages/clerk-js/src/ui/elements/contexts/index.tsx (1)
useCardState(42-70)packages/react/src/isomorphicClerk.ts (1)
session(672-678)packages/clerk-js/src/core/sessionTasks.ts (1)
buildTaskUrl(38-47)
packages/clerk-js/src/ui/contexts/components/SignUp.ts (5)
packages/types/src/session.ts (1)
SessionResource(208-261)packages/clerk-js/src/ui/router/RouteContext.tsx (1)
useRouter(33-39)packages/clerk-js/src/core/resources/Session.ts (1)
currentTask(390-393)packages/react/src/isomorphicClerk.ts (1)
session(672-678)packages/clerk-js/src/core/sessionTasks.ts (2)
INTERNAL_SESSION_TASK_ROUTE_BY_KEY(10-12)buildTaskRedirectUrl(19-33)
integration/tests/session-tasks-sign-up.test.ts (3)
integration/presets/index.ts (1)
appConfigs(14-30)integration/presets/envs.ts (1)
instanceKeys(23-23)integration/testUtils/usersService.ts (1)
createUserService(101-218)
packages/clerk-js/src/ui/contexts/components/SignIn.ts (6)
packages/types/src/session.ts (1)
SessionResource(208-261)packages/clerk-js/src/ui/types.ts (1)
SignInCtx(64-68)packages/clerk-js/src/ui/router/RouteContext.tsx (1)
useRouter(33-39)packages/clerk-js/src/core/resources/Session.ts (1)
currentTask(390-393)packages/react/src/isomorphicClerk.ts (1)
session(672-678)packages/clerk-js/src/core/sessionTasks.ts (2)
INTERNAL_SESSION_TASK_ROUTE_BY_KEY(10-12)buildTaskRedirectUrl(19-33)
packages/clerk-js/src/core/__tests__/clerk.test.ts (2)
packages/types/src/clerk.ts (1)
Clerk(164-886)packages/types/src/session.ts (2)
PendingSessionResource(275-279)ActiveSessionResource(267-270)
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx (2)
packages/clerk-js/src/ui/common/withRedirect.tsx (2)
withRedirectToSignInTask(93-113)withRedirectToAfterSignIn(55-72)packages/clerk-js/src/ui/elements/contexts/index.tsx (1)
withCardStateProvider(72-81)
packages/clerk-js/src/core/sessionTasks.ts (4)
packages/types/src/session.ts (1)
SessionTask(332-337)packages/clerk-js/src/ui/common/redirects.ts (1)
buildRedirectUrl(58-79)packages/clerk-js/src/utils/url.ts (1)
buildURL(81-155)packages/types/src/clerk.ts (1)
ClerkOptions(967-1087)
packages/clerk-js/src/core/clerk.ts (7)
packages/clerk-js/src/utils/componentGuards.ts (1)
isSignedInAndSingleSessionModeEnabled(9-11)packages/types/src/clerk.ts (1)
SetActiveParams(1189-1235)packages/react/src/isomorphicClerk.ts (1)
session(672-678)packages/types/src/session.ts (2)
SignedInSessionResource(285-285)SessionResource(208-261)packages/clerk-js/src/core/sessionTasks.ts (3)
warnMissingPendingTaskHandlers(49-63)INTERNAL_SESSION_TASK_ROUTE_BY_KEY(10-12)buildTaskUrl(38-47)packages/clerk-js/src/core/resources/Session.ts (1)
currentTask(390-393)packages/clerk-js/src/utils/runtime.ts (1)
inBrowser(1-3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (22)
packages/clerk-js/src/ui/components/UserButton/useMultisessionActions.tsx (1)
4-5: Importing sessionTasks and environment looks correctUsing buildTaskUrl and useEnvironment aligns this hook with the new explicit after-auth navigation model.
packages/clerk-js/src/core/resources/SignIn.ts (1)
252-256: Redirect URL decoration centralized and consistent — looks goodUsing SignIn.clerk.buildUrlWithAuth(params.redirectUrl) once and passing it through to both create and prepareFirstFactor removes duplication and preserves dev-browser auth context across OAuth/SSO flows.
Also applies to: 261-263, 269-271
packages/clerk-js/src/ui/contexts/components/SignUp.ts (1)
127-135: taskUrl computation aligns with the new modelPreferring taskUrls override and falling back to buildTaskRedirectUrl is the right approach and consistent with preserving auth/query context.
packages/clerk-js/src/ui/components/SignUp/index.tsx (2)
47-57: Propagating taskUrl to SSO callback is correctPassing taskUrl from context into SignUpSSOCallback keeps SSO flows consistent with the new per-task routing model.
5-6: Alias '@/…' is correctly configured in packages/clerk-js/tsconfig.jsonThe import
import { usePreloadTasks } from '@/ui/hooks/usePreloadTasks';will resolve topackages/clerk-js/src/ui/hooks/usePreloadTasksbecause:
- packages/clerk-js/tsconfig.json (lines 23–25) defines
"paths": { "@/*": ["./src/*"] }No changes needed.
packages/clerk-js/src/core/__tests__/clerk.test.ts (6)
447-457: LGTM!The test correctly validates that
setActivecalls bothsession.touchand thenavigatecallback for active sessions. This aligns with the new navigate option for after-auth flows.
528-536: LGTM!The test correctly validates the basic behavior of
setActivewith a pending session - it should callsession.touchas expected.
580-590: LGTM!The test correctly validates that the
navigatecallback is invoked when provided tosetActivefor a pending session.
992-1038: LGTM!The test correctly validates navigation to a task URL when a pending session is returned from sign-up completion. The use of
/sign-in#/tasks/choose-organizationfor the expected navigation path correctly reflects the new per-task routing model.
1546-1547: Consistent test data structureGood job updating test data to include sessions in both
sessionsandsignedInSessionsarrays consistently across tests. This properly reflects the expected client state.Also applies to: 1567-1568
538-549: Add explicit verification that hooks are not calledThe test should explicitly verify that the hooks were not called rather than relying on the lack of assertion failure.
const onBeforeSetActive = jest.fn(); (window as any).__unstable__onBeforeSetActive = onBeforeSetActive; const sut = new Clerk(productionPublishableKey); await sut.load(); await sut.setActive({ session: mockSession as any as ActiveSessionResource }); expect(onBeforeSetActive).not.toHaveBeenCalled(); + expect(mockSession.touch).toHaveBeenCalled();Likely an incorrect or invalid review comment.
packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx (4)
239-241: LGTM! Proper use of navigate callbackGood implementation of the new
navigatecallback pattern forsetActive. This correctly passes the session context to enable task-based navigation.
392-394: Consistent implementation of navigate callbackThe navigation callback pattern is consistently applied here as well. Good job maintaining consistency across different sign-in flows.
476-477: LGTM! Navigate callback properly passed to combined flowThe
navigateOnSetActiveis correctly passed through to the combined flow handler, ensuring consistent navigation behavior.
684-686: Good HOC composition for task navigationThe addition of
withRedirectToSignInTaskas the outermost HOC is appropriate, as it should check for pending tasks before other redirects. The composition order is correct.packages/clerk-js/src/core/sessionTasks.ts (2)
1-6: LGTM! Clean imports and dependenciesThe imports are well-organized and appropriate for the session task handling functionality.
49-63: LGTM! Well-implemented warning logicThe warning function properly checks for the presence of task handlers and provides a helpful warning message when they're missing. The use of
logger.warnOnceensures the warning doesn't spam the console.packages/clerk-js/src/core/clerk.ts (5)
1201-1204: LGTM! Clean parameter destructuringThe new
SetActiveParamssignature properly supports the navigate callback and other parameters through a single params object.
1666-1671: LGTM! Clean implementation of redirectToTasksThe new
redirectToTasksmethod properly integrates with the task navigation system.
1854-1869: Good implementation of setActiveNavigate helperThe helper function properly handles both task navigation and regular redirects. The logic is clear and follows the expected navigation precedence.
1871-1877: LGTM! Proper use of navigate callback in OAuth flowThe OAuth completion flow correctly uses the new navigate callback pattern with
setActiveNavigate.
2167-2178: LGTM! Consistent implementation in Web3 flowThe Web3 authentication flow properly implements the same navigation pattern as OAuth.
| it('does not call __unstable__onAfterSetActive after session.touch', async () => { | ||
| mockSession.touch.mockReturnValueOnce(Promise.resolve()); | ||
| mockClientFetch.mockReturnValue(Promise.resolve({ signedInSessions: [mockSession] })); | ||
|
|
||
| const onAfterSetActive = jest.fn(); | ||
| (window as any).__unstable__onAfterSetActive = onAfterSetActive; | ||
|
|
||
| const sut = new Clerk(productionPublishableKey); | ||
| await sut.load(); | ||
| await sut.setActive({ session: mockSession as any as ActiveSessionResource }); | ||
| expect(onAfterSetActive).not.toHaveBeenCalled(); | ||
| }); |
There was a problem hiding this comment.
Add explicit verification that hooks are not called
Similar to the previous comment, the test should explicitly verify that the hooks were not called.
const onAfterSetActive = jest.fn();
(window as any).__unstable__onAfterSetActive = onAfterSetActive;
const sut = new Clerk(productionPublishableKey);
await sut.load();
await sut.setActive({ session: mockSession as any as ActiveSessionResource });
expect(onAfterSetActive).not.toHaveBeenCalled();
+ expect(mockSession.touch).toHaveBeenCalled();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('does not call __unstable__onAfterSetActive after session.touch', async () => { | |
| mockSession.touch.mockReturnValueOnce(Promise.resolve()); | |
| mockClientFetch.mockReturnValue(Promise.resolve({ signedInSessions: [mockSession] })); | |
| const onAfterSetActive = jest.fn(); | |
| (window as any).__unstable__onAfterSetActive = onAfterSetActive; | |
| const sut = new Clerk(productionPublishableKey); | |
| await sut.load(); | |
| await sut.setActive({ session: mockSession as any as ActiveSessionResource }); | |
| expect(onAfterSetActive).not.toHaveBeenCalled(); | |
| }); | |
| it('does not call __unstable__onAfterSetActive after session.touch', async () => { | |
| mockSession.touch.mockReturnValueOnce(Promise.resolve()); | |
| mockClientFetch.mockReturnValue(Promise.resolve({ signedInSessions: [mockSession] })); | |
| const onAfterSetActive = jest.fn(); | |
| (window as any).__unstable__onAfterSetActive = onAfterSetActive; | |
| const sut = new Clerk(productionPublishableKey); | |
| await sut.load(); | |
| await sut.setActive({ session: mockSession as any as ActiveSessionResource }); | |
| expect(onAfterSetActive).not.toHaveBeenCalled(); | |
| expect(mockSession.touch).toHaveBeenCalled(); | |
| }); |
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/__tests__/clerk.test.ts around lines 551 to 562,
the test only sets and asserts a single hook; add explicit verification for the
related hook(s) so the test clearly asserts no lifecycle hooks are invoked.
Create spies for both window.__unstable__onBeforeSetActive and
window.__unstable__onAfterSetActive (or any other related hooks used in similar
tests), assign them to window, run the same actions (load and setActive), and
add expect(...).not.toHaveBeenCalled() assertions for each spy; restore/cleanup
the window properties after the test.
| const { setActive, signOut, openUserProfile } = useClerk(); | ||
| const card = useCardState(); | ||
| const { signedInSessions, otherSessions } = useMultipleSessions({ user: opts.user }); | ||
| const { navigate } = useRouter(); | ||
| const { displayConfig } = useEnvironment(); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prefer accessing the Clerk instance to support taskUrls override resolution
You destructure methods from useClerk(), but you’ll need the Clerk instance to honor a provider-level taskUrls override inside your navigate callback.
Apply this diff to keep access to the Clerk object while preserving current usage:
- const { setActive, signOut, openUserProfile } = useClerk();
+ const clerk = useClerk();
+ const { setActive, signOut, openUserProfile } = clerk;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { setActive, signOut, openUserProfile } = useClerk(); | |
| const card = useCardState(); | |
| const { signedInSessions, otherSessions } = useMultipleSessions({ user: opts.user }); | |
| const { navigate } = useRouter(); | |
| const { displayConfig } = useEnvironment(); | |
| const clerk = useClerk(); | |
| const { setActive, signOut, openUserProfile } = clerk; | |
| const card = useCardState(); | |
| const { signedInSessions, otherSessions } = useMultipleSessions({ user: opts.user }); | |
| const { navigate } = useRouter(); | |
| const { displayConfig } = useEnvironment(); |
| return setActive({ | ||
| session, | ||
| navigate: async ({ session }) => { | ||
| if (!session.currentTask && opts.afterSwitchSessionUrl) { | ||
| await navigate(opts.afterSwitchSessionUrl); | ||
| return; | ||
| } | ||
|
|
||
| if (session.currentTask) { | ||
| await navigate( | ||
| buildTaskUrl(session.currentTask, { | ||
| baseUrl: opts.signInUrl ?? displayConfig.signInUrl, | ||
| }), | ||
| ); | ||
| } | ||
| }, | ||
| }).finally(() => { | ||
| card.setIdle(); | ||
| opts.actionCompleteCallback?.(); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Honor taskUrls overrides when navigating to session tasks during session switch
When a currentTask exists, the navigate callback always uses buildTaskUrl. If a developer provided taskUrls via ClerkProvider, we should prefer that mapping for consistency with the rest of the UI contexts.
Apply this diff to prefer the override and fallback to the default URL:
- return setActive({
+ return setActive({
session,
navigate: async ({ session }) => {
if (!session.currentTask && opts.afterSwitchSessionUrl) {
await navigate(opts.afterSwitchSessionUrl);
return;
}
- if (session.currentTask) {
- await navigate(
- buildTaskUrl(session.currentTask, {
- baseUrl: opts.signInUrl ?? displayConfig.signInUrl,
- }),
- );
- }
+ if (session.currentTask) {
+ const override = clerk.__internal_getOption('taskUrls')?.[session.currentTask.key];
+ const target =
+ override ??
+ buildTaskUrl(session.currentTask, {
+ baseUrl: opts.signInUrl ?? displayConfig.signInUrl,
+ });
+ await navigate(target);
+ }
},
}).finally(() => {
card.setIdle();
opts.actionCompleteCallback?.();
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return setActive({ | |
| session, | |
| navigate: async ({ session }) => { | |
| if (!session.currentTask && opts.afterSwitchSessionUrl) { | |
| await navigate(opts.afterSwitchSessionUrl); | |
| return; | |
| } | |
| if (session.currentTask) { | |
| await navigate( | |
| buildTaskUrl(session.currentTask, { | |
| baseUrl: opts.signInUrl ?? displayConfig.signInUrl, | |
| }), | |
| ); | |
| } | |
| }, | |
| }).finally(() => { | |
| card.setIdle(); | |
| opts.actionCompleteCallback?.(); | |
| }); | |
| return setActive({ | |
| session, | |
| navigate: async ({ session }) => { | |
| if (!session.currentTask && opts.afterSwitchSessionUrl) { | |
| await navigate(opts.afterSwitchSessionUrl); | |
| return; | |
| } | |
| if (session.currentTask) { | |
| const override = clerk.__internal_getOption('taskUrls')?.[session.currentTask.key]; | |
| const target = | |
| override ?? | |
| buildTaskUrl(session.currentTask, { | |
| baseUrl: opts.signInUrl ?? displayConfig.signInUrl, | |
| }); | |
| await navigate(target); | |
| } | |
| }, | |
| }).finally(() => { | |
| card.setIdle(); | |
| opts.actionCompleteCallback?.(); | |
| }); |
🤖 Prompt for AI Agents
In packages/clerk-js/src/ui/components/UserButton/useMultisessionActions.tsx
around lines 76 to 95 the navigate branch that handles session.currentTask
always calls buildTaskUrl; update it to first prefer any provided taskUrls
override (opts.taskUrls ?? displayConfig.taskUrls) and, if that mapping contains
an entry for session.currentTask, navigate to that URL; otherwise fall back to
the existing buildTaskUrl call (still using baseUrl: opts.signInUrl ??
displayConfig.signInUrl). Ensure the lookup supports string mappings (and falls
back to default behavior when no mapping or entry exists), then keep the same
finally block that calls card.setIdle() and opts.actionCompleteCallback?.().
9adcc3c to
d005e2d
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (13)
packages/clerk-js/src/ui/contexts/components/SignIn.ts (4)
33-34: Make redirectUrl optional and refine return typeredirectUrl can be absent on setActive. Also, the callback returns nothing meaningful; use Promise.
- navigateOnSetActive: (opts: { session: SessionResource; redirectUrl: string }) => Promise<unknown>; + navigateOnSetActive: (opts: { session: SessionResource; redirectUrl?: string }) => Promise<void>;
41-43: Consolidate duplicate useRouter() callsYou're calling useRouter() twice on lines 41 and 43, which is inefficient.
- const { navigate, basePath } = useRouter(); + const { navigate, basePath, queryParams, queryString } = useRouter(); const { displayConfig, userSettings } = useEnvironment(); - const { queryParams, queryString } = useRouter();
124-131: Honor taskUrls configuration and simplify navigation logicThe current implementation manually constructs the task URL path and doesn't respect ClerkProvider's
taskUrlsconfiguration.const navigateOnSetActive = async ({ session, redirectUrl }: { session: SessionResource; redirectUrl: string }) => { const currentTask = session.currentTask; if (!currentTask) { return navigate(redirectUrl); } - return navigate(`/${basePath}/tasks/${INTERNAL_SESSION_TASK_ROUTE_BY_KEY[currentTask.key]}`); + const customTaskUrl = clerk.__internal_getOption('taskUrls')?.[currentTask.key]; + const target = customTaskUrl ?? buildTaskUrl(currentTask, { baseUrl: signInUrl }); + return navigate(target); };
133-141: Use newSession for computing taskUrl to avoid stale state
clerk.sessionmight not be updated yet when switching to a new pending session, causingtaskUrlto be null. Use the resolved session fromsetActiveinstead.Since
taskUrlis computed during context initialization and not during thesetActiveflow, consider computing it dynamically or documenting this limitation.packages/clerk-js/src/core/sessionTasks.ts (2)
7-12: Consider usingsatisfiesoperator for better type safetyUsing
satisfiesensures the object matches the expected type while preserving the literal types.-export const INTERNAL_SESSION_TASK_ROUTE_BY_KEY: Record<SessionTask['key'], string> = { - 'choose-organization': 'choose-organization', -} as const; +export const INTERNAL_SESSION_TASK_ROUTE_BY_KEY = { + 'choose-organization': 'choose-organization', +} as const satisfies Record<SessionTask['key'], string>;
38-47: Add explicit return type annotation and consider supporting redirectUrl preservationThe function should have an explicit return type. Also, consider supporting an optional
redirectUrlparameter that can be preserved in the task URL for maintaining navigation context.export function buildTaskUrl( task: SessionTask, { baseUrl, + redirectUrl, }: { baseUrl: string; + redirectUrl?: string; }, -) { - return buildURL({ base: baseUrl, hashPath: getTaskEndpoint(task) }, { stringify: true }); +): string { + const params = new URLSearchParams(); + if (redirectUrl) { + params.set('redirect_url', redirectUrl); + } + return buildURL( + { + base: baseUrl, + hashPath: getTaskEndpoint(task), + hashSearchParams: params + }, + { stringify: true } + ); }packages/clerk-js/src/core/clerk.ts (7)
1232-1234: Fix warning function call to include merged optionsBased on the past review comments, the warning function receives incomplete data.
paramsdoesn't containtaskUrlsfrom the global options, causing false positive warnings.Apply this fix to pass the complete configuration:
-warnMissingPendingTaskHandlers({ ...this.#options, ...params }); +warnMissingPendingTaskHandlers({ ...this.#options, ...params });Wait, let me check the current code again. The current implementation already merges both objects, so this appears to be correctly implemented now.
1321-1325: Improve redirectUrl preservation for different URL typesThe current implementation always appends
redirectUrlas hash parameters, which could introduce unexpected hash fragments for non-hash URLs.if (taskUrl) { - const taskUrlWithRedirect = redirectUrl - ? buildURL({ base: taskUrl, hashSearchParams: { redirectUrl } }, { stringify: true }) - : taskUrl; + let taskUrlWithRedirect = taskUrl; + if (redirectUrl) { + const useHash = taskUrl.includes('#'); + const buildOpts: any = { base: taskUrl }; + if (useHash) { + buildOpts.hashSearchParams = { redirectUrl }; + } else { + buildOpts.searchParams = { redirectUrl }; + } + taskUrlWithRedirect = buildURL(buildOpts, { stringify: true }) as string; + } await this.navigate(taskUrlWithRedirect);
1326-1327: Add error handling for consumer navigate callbackThe consumer-supplied
navigatecallback could throw exceptions, leaving Clerk in an inconsistent state with__internal_setActiveInProgressstuck astrue.if (setActiveNavigate && newSession) { - await setActiveNavigate({ session: newSession }); + try { + await setActiveNavigate({ session: newSession }); + } catch (err) { + logger.error('setActive.navigate callback failed', err); + // Re-throw to maintain existing behavior but ensure cleanup happens + throw err; + } return; }
1329-1336: Fix duplicate redirectUrl navigationThere appears to be duplicate navigation logic where
redirectUrlis navigated to twice - once conditionally inside theifblock and once unconditionally after.} else if (redirectUrl) { if (this.client.isEligibleForTouch()) { const absoluteRedirectUrl = new URL(redirectUrl, window.location.href); const redirectUrlWithAuth = this.buildUrlWithAuth( this.client.buildTouchUrl({ redirectUrl: absoluteRedirectUrl }), ); await this.navigate(redirectUrlWithAuth); + return; } -} -await this.navigate(redirectUrl); + await this.navigate(redirectUrl); +}
1854-1869: setActiveNavigate helper needs redirectUrl propagationThe helper function doesn't receive or use the
redirectUrlparameter, which could cause the redirect URL to be lost during task navigation in OAuth callbacks.const setActiveNavigate = async ({ session, baseUrl, redirectUrl, }: { session: SessionResource; baseUrl: string; redirectUrl: string; }) => { if (!session.currentTask) { await this.navigate(redirectUrl); return; } - await navigate(params.taskUrl ?? buildTaskUrl(session.currentTask, { baseUrl })); + const taskUrl = params.taskUrl ?? buildTaskUrl(session.currentTask, { baseUrl }); + const taskUrlWithRedirect = redirectUrl + ? buildURL({ base: taskUrl, hashSearchParams: { redirectUrl } }, { stringify: true }) + : taskUrl; + await navigate(taskUrlWithRedirect); };
2167-2178: Apply consistent redirectUrl handling in Web3 flowsThe Web3
setActiveNavigatehelper also needs to handleredirectUrlpropagation to maintain consistency across all authentication flows.const setActiveNavigate = async ({ session, redirectUrl }: { session: SessionResource; redirectUrl: string }) => { if (!session.currentTask) { await this.navigate(redirectUrl); return; } - await this.navigate( - buildTaskUrl(session.currentTask, { - baseUrl: displayConfig.signInUrl, - }), - ); + const taskUrl = buildTaskUrl(session.currentTask, { + baseUrl: displayConfig.signInUrl, + }); + const taskUrlWithRedirect = redirectUrl + ? buildURL({ base: taskUrl, hashSearchParams: { redirectUrl } }, { stringify: true }) + : taskUrl; + await this.navigate(taskUrlWithRedirect); };
1217-1220: Add proper error handling for session ID lookupThe current implementation unsafely casts the result of
find()which could beundefined. This could lead to runtime errors if a session ID doesn't exist.if (typeof session === 'string') { - session = (this.client.sessions.find(x => x.id === session) as SignedInSessionResource) || null; + const foundSession = this.client.sessions.find(x => x.id === session); + if (!foundSession) { + throw new Error(`Session with ID "${session}" not found in client sessions.`); + } + session = foundSession as SignedInSessionResource; }
🧹 Nitpick comments (7)
packages/clerk-js/src/utils/redirects.ts (4)
13-34: Add explicit return type annotations for helper functionsWhile the main
buildRedirectUrlfunction has a return type annotation, the helper functions lack them.-const buildHashBasedUrl = (authQueryString: AuthQueryString, endpoint: string): string => { +const buildHashBasedUrl = (authQueryString: AuthQueryString, endpoint: string): string => { // Strip hash to get the URL where we're mounted const hash = endpoint + (authQueryString ? `?${authQueryString}` : ''); return buildURL({ hash }, { stringify: true }); }; -const buildPathBasedUrl = (path: string, authQueryString: AuthQueryString, endpoint: string): string => { +const buildPathBasedUrl = (path: string, authQueryString: AuthQueryString, endpoint: string): string => { const searchArg = authQueryString ? { search: '?' + authQueryString } : {}; return buildURL( { pathname: path + endpoint, ...searchArg, }, { stringify: true }, ); }; -const buildVirtualBasedUrl = (base: string, authQueryString: AuthQueryString, endpoint: string): string => { +const buildVirtualBasedUrl = (base: string, authQueryString: AuthQueryString, endpoint: string): string => { const hash = endpoint + (authQueryString ? `?${authQueryString}` : ''); return buildURL({ base, hash }, { stringify: true }); };
36-40: Type assertion is unnecessary with TypeScript's control flow analysisThe
buildURLfunction already returnsstringwhenstringify: trueis passed, so the type assertion on Line 39 is redundant.const buildHashBasedUrl = (authQueryString: AuthQueryString, endpoint: string): string => { // Strip hash to get the URL where we're mounted const hash = endpoint + (authQueryString ? `?${authQueryString}` : ''); - return buildURL({ hash }, { stringify: true }); + return buildURL({ hash }, { stringify: true }); };
42-51: Type assertion is unnecessarySame as above - the type assertion is not needed when
stringify: trueis passed.const buildPathBasedUrl = (path: string, authQueryString: AuthQueryString, endpoint: string): string => { const searchArg = authQueryString ? { search: '?' + authQueryString } : {}; return buildURL( { pathname: path + endpoint, ...searchArg, }, { stringify: true }, - ); + ); };
53-56: Type assertion is unnecessarySame as above - the type assertion is not needed.
const buildVirtualBasedUrl = (base: string, authQueryString: AuthQueryString, endpoint: string): string => { const hash = endpoint + (authQueryString ? `?${authQueryString}` : ''); - return buildURL({ base, hash }, { stringify: true }); + return buildURL({ base, hash }, { stringify: true }); };packages/clerk-js/src/core/sessionTasks.ts (3)
14-14: Add explicit return type annotationAccording to the coding guidelines, functions should have explicit return types.
-const getTaskEndpoint = (task: SessionTask) => `/tasks/${INTERNAL_SESSION_TASK_ROUTE_BY_KEY[task.key]}`; +const getTaskEndpoint = (task: SessionTask): string => `/tasks/${INTERNAL_SESSION_TASK_ROUTE_BY_KEY[task.key]}`;
19-33: Add explicit return type annotationAccording to the coding guidelines, functions should have explicit return types.
export function buildTaskRedirectUrl( task: SessionTask, opts: Omit<Parameters<typeof buildRedirectUrl>[0], 'endpoint'>, -) { +): string {
49-63: Add explicit return type annotationAccording to the coding guidelines, functions should have explicit return types.
-export function warnMissingPendingTaskHandlers(options: Record<string, unknown>) { +export function warnMissingPendingTaskHandlers(options: Record<string, unknown>): void {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/clerk-js/src/core/clerk.ts(23 hunks)packages/clerk-js/src/core/sessionTasks.ts(1 hunks)packages/clerk-js/src/ui/common/redirects.ts(1 hunks)packages/clerk-js/src/ui/contexts/components/SignIn.ts(4 hunks)packages/clerk-js/src/ui/contexts/components/SignUp.ts(4 hunks)packages/clerk-js/src/utils/index.ts(2 hunks)packages/clerk-js/src/utils/redirects.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/clerk-js/src/ui/contexts/components/SignUp.ts
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/clerk-js/src/utils/redirects.tspackages/clerk-js/src/utils/index.tspackages/clerk-js/src/core/clerk.tspackages/clerk-js/src/core/sessionTasks.tspackages/clerk-js/src/ui/common/redirects.tspackages/clerk-js/src/ui/contexts/components/SignIn.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/clerk-js/src/utils/redirects.tspackages/clerk-js/src/utils/index.tspackages/clerk-js/src/core/clerk.tspackages/clerk-js/src/core/sessionTasks.tspackages/clerk-js/src/ui/common/redirects.tspackages/clerk-js/src/ui/contexts/components/SignIn.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/utils/redirects.tspackages/clerk-js/src/utils/index.tspackages/clerk-js/src/core/clerk.tspackages/clerk-js/src/core/sessionTasks.tspackages/clerk-js/src/ui/common/redirects.tspackages/clerk-js/src/ui/contexts/components/SignIn.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/clerk-js/src/utils/redirects.tspackages/clerk-js/src/utils/index.tspackages/clerk-js/src/core/clerk.tspackages/clerk-js/src/core/sessionTasks.tspackages/clerk-js/src/ui/common/redirects.tspackages/clerk-js/src/ui/contexts/components/SignIn.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/clerk-js/src/utils/redirects.tspackages/clerk-js/src/utils/index.tspackages/clerk-js/src/core/clerk.tspackages/clerk-js/src/core/sessionTasks.tspackages/clerk-js/src/ui/common/redirects.tspackages/clerk-js/src/ui/contexts/components/SignIn.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/clerk-js/src/utils/redirects.tspackages/clerk-js/src/utils/index.tspackages/clerk-js/src/core/clerk.tspackages/clerk-js/src/core/sessionTasks.tspackages/clerk-js/src/ui/common/redirects.tspackages/clerk-js/src/ui/contexts/components/SignIn.ts
**/*
⚙️ CodeRabbit Configuration File
If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.
Files:
packages/clerk-js/src/utils/redirects.tspackages/clerk-js/src/utils/index.tspackages/clerk-js/src/core/clerk.tspackages/clerk-js/src/core/sessionTasks.tspackages/clerk-js/src/ui/common/redirects.tspackages/clerk-js/src/ui/contexts/components/SignIn.ts
packages/**/index.{js,ts}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use tree-shaking friendly exports
Files:
packages/clerk-js/src/utils/index.ts
**/index.ts
📄 CodeRabbit Inference Engine (.cursor/rules/react.mdc)
Use index.ts files for clean imports but avoid deep barrel exports
Avoid barrel files (index.ts re-exports) as they can cause circular dependencies
Files:
packages/clerk-js/src/utils/index.ts
packages/clerk-js/src/ui/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/clerk-js-ui.mdc)
packages/clerk-js/src/ui/**/*.{ts,tsx}: Element descriptors should always be camelCase
Use element descriptors in UI components to enable consistent theming and styling via appearance.elements
Element descriptors should generate unique, stable CSS classes for theming
Element descriptors should handle state classes (e.g., cl-loading, cl-active, cl-error, cl-open) automatically based on component state
Do not render hard-coded values; all user-facing strings must be localized using provided localization methods
Use the useLocalizations hook and localizationKeys utility for all text and error messages
Use the styled system (sx prop, theme tokens, responsive values) for custom component styling
Use useCardState for card-level state, useFormState for form-level state, and useLoadingStatus for loading states
Always use handleError utility for API errors and use translateError for localized error messages
Use useFormControl for form field state, implement proper validation, and handle loading and error states in forms
Use localization keys for all form labels and placeholders
Use element descriptors for consistent styling and follow the theme token system
Use the Card and FormContainer patterns for consistent UI structure
Files:
packages/clerk-js/src/ui/common/redirects.tspackages/clerk-js/src/ui/contexts/components/SignIn.ts
🧠 Learnings (1)
📚 Learning: 2025-08-08T19:00:08.931Z
Learnt from: LauraBeatris
PR: clerk/javascript#6486
File: packages/clerk-js/src/core/clerk.ts:1303-1344
Timestamp: 2025-08-08T19:00:08.931Z
Learning: In Clerk's navigation system for pending sessions, `taskUrls` (configured at the ClerkProvider level) should take priority over the `navigate` callback (from All-In-One components). This ensures that explicit provider-level configuration overrides component-level navigation logic, providing developers with predictable control over navigation behavior.
Applied to files:
packages/clerk-js/src/core/clerk.ts
🧬 Code Graph Analysis (4)
packages/clerk-js/src/utils/redirects.ts (1)
packages/clerk-js/src/utils/url.ts (1)
buildURL(81-155)
packages/clerk-js/src/core/clerk.ts (8)
packages/clerk-js/src/utils/componentGuards.ts (1)
isSignedInAndSingleSessionModeEnabled(9-11)packages/types/src/clerk.ts (1)
SetActiveParams(1189-1235)packages/react/src/isomorphicClerk.ts (1)
session(672-678)packages/types/src/session.ts (2)
SignedInSessionResource(285-285)SessionResource(208-261)packages/clerk-js/src/core/sessionTasks.ts (3)
warnMissingPendingTaskHandlers(49-63)INTERNAL_SESSION_TASK_ROUTE_BY_KEY(10-12)buildTaskUrl(38-47)packages/clerk-js/src/utils/url.ts (1)
buildURL(81-155)packages/clerk-js/src/core/resources/Session.ts (1)
currentTask(390-393)packages/clerk-js/src/utils/runtime.ts (1)
inBrowser(1-3)
packages/clerk-js/src/core/sessionTasks.ts (4)
packages/types/src/session.ts (1)
SessionTask(332-337)packages/clerk-js/src/utils/redirects.ts (1)
buildRedirectUrl(13-34)packages/clerk-js/src/utils/url.ts (1)
buildURL(81-155)packages/types/src/clerk.ts (2)
SetActiveParams(1189-1235)ClerkOptions(967-1087)
packages/clerk-js/src/ui/contexts/components/SignIn.ts (5)
packages/types/src/session.ts (1)
SessionResource(208-261)packages/clerk-js/src/ui/types.ts (1)
SignInCtx(64-68)packages/clerk-js/src/ui/router/RouteContext.tsx (1)
useRouter(33-39)packages/clerk-js/src/core/resources/Session.ts (1)
currentTask(390-393)packages/clerk-js/src/core/sessionTasks.ts (2)
INTERNAL_SESSION_TASK_ROUTE_BY_KEY(10-12)buildTaskRedirectUrl(19-33)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (10)
packages/clerk-js/src/utils/index.ts (1)
1-29: LGTM!The export order change and addition of the redirects module export are appropriate.
packages/clerk-js/src/ui/common/redirects.ts (1)
1-47: LGTM!The refactoring to use the centralized
buildRedirectUrlutility simplifies the code and promotes consistency across the codebase.packages/clerk-js/src/core/clerk.ts (8)
1201-1204: LGTM: Clean parameter destructuring and method signature migrationThe refactored method signature properly adopts the new
SetActiveParamsinterface with clear parameter destructuring. The renaming ofnavigatetosetActiveNavigateprovides good internal clarity.
1305-1308: Good fix: taskUrl now correctly uses newSessionThe implementation correctly derives
taskUrlfromnewSessioninstead of the potentially stalethis.session. This resolves the previous issue where task URLs wouldn't be computed correctly during session transitions.
1321-1327: Verify precedence order aligns with requirementsBased on the retrieved learning,
taskUrls(provider-level configuration) should take priority over thenavigatecallback (AIO component level). The current order correctly implements this precedence:
taskUrl(from provider-leveltaskUrls)setActiveNavigate(from AIO components)redirectUrl(fallback)This matches the expected behavior where explicit provider configuration overrides component-level logic.
1554-1574: Excellent addition: buildTasksUrl() methodThis new public method provides a clean way to build task URLs with proper fallback logic:
- Check for custom
taskUrlsconfiguration first- Fall back to default sign-in URL with task-specific hash path
The implementation correctly handles the case where no current task exists.
1666-1671: Good companion method: redirectToTasks()This method pairs well with
buildTasksUrl()to provide a complete public API for task navigation. The implementation follows the same pattern as other redirect methods.
554-554: Good refactoring: Consistent guard usageThe replacement of inline component guards with
isSignedInAndSingleSessionModeEnabledprovides better consistency and maintainability acrossopenSignInandopenSignUpmethods.Also applies to: 688-688
1354-1356: Essential: Proper cleanup in finally blockThe addition of a
finallyblock ensures that__internal_setActiveInProgressis always reset tofalse, preventing the library from getting stuck in an inconsistent state even if errors occur.
154-154: Clean imports: Task-related utilitiesThe import statement properly brings in the new session task utilities (
buildTaskUrl,INTERNAL_SESSION_TASK_ROUTE_BY_KEY,warnMissingPendingTaskHandlers) that support the enhanced task navigation functionality.
LauraBeatris
force-pushed
the
laura/introduce-on-pending-session
branch
from
d005e2d to
9c23c16
Compare
LauraBeatris
force-pushed
the
laura/introduce-on-pending-session
branch
from
9c23c16 to
f67d96e
Compare
LauraBeatris
force-pushed
the
laura/introduce-on-pending-session
branch
from
f67d96e to
c5418c9
Compare
LauraBeatris
force-pushed
the
laura/introduce-on-pending-session
branch
from
c5418c9 to
fa3a908
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (2)
packages/nextjs/src/client-boundary/controlComponents.ts (1)
3-18: Add a deprecated alias and enforce a minimum @clerk/clerk-react versionWe verified there are no lingering
RedirectToTaskimports in the monorepo. To smooth the rename fromRedirectToTask→RedirectToTasksfor Next.js consumers, please:• In
packages/nextjs/src/client-boundary/controlComponents.ts, append:// Temporary BC alias for vX.Y → vX+1; remove in next major export { RedirectToTasks as RedirectToTask } from '@clerk/clerk-react';• In
packages/nextjs/package.json, add apeerDependenciesentry requiring the release of@clerk/clerk-reactthat shipsRedirectToTasks(update^X.Y.Zaccordingly):"peerDependencies": { "@clerk/clerk-react": "^X.Y.Z" }• No further imports of
RedirectToTaskexist—internal usages are fully migrated.packages/clerk-js/src/core/clerk.ts (1)
1305-1339: Navigation logic is well-structured but needs minor improvements.The current implementation correctly prioritizes
taskUrloversetActiveNavigate(as per learnings), but has some issues:
- Redirect URL preservation: Current logic always uses
hashSearchParams, which may not be appropriate for non-hash URLs- Error handling: Missing try-catch for the navigate callback
if (taskUrl) { - const taskUrlWithRedirect = redirectUrl - ? buildURL({ base: taskUrl, hashSearchParams: { redirectUrl } }, { stringify: true }) - : taskUrl; + let taskUrlWithRedirect = taskUrl; + if (redirectUrl) { + const useHash = taskUrl.includes('#'); + const buildOpts: any = { base: taskUrl }; + if (useHash) { + buildOpts.hashSearchParams = { redirectUrl }; + } else { + buildOpts.searchParams = { redirectUrl }; + } + taskUrlWithRedirect = buildURL(buildOpts, { stringify: true }) as string; + } await this.navigate(taskUrlWithRedirect); + return; } else if (setActiveNavigate && newSession) { - await setActiveNavigate({ session: newSession }); + try { + await setActiveNavigate({ session: newSession }); + } catch (err) { + logger.error('setActive.navigate callback failed', err); + } + return;
♻️ Duplicate comments (3)
packages/clerk-js/src/ui/contexts/components/SignIn.ts (1)
32-33: Fix function signature: make redirectUrl optional and improve return type.Based on the context and learning that
redirectUrlcan be absent insetActive, the parameter should be optional. Also, the callback doesn't return meaningful data.- navigateOnSetActive: (opts: { session: SessionResource; redirectUrl: string }) => Promise<unknown>; + navigateOnSetActive: (opts: { session: SessionResource; redirectUrl?: string }) => Promise<void>;packages/clerk-js/src/core/clerk.ts (2)
1232-1234: Fix argument to warning function.The warning function receives only
paramsbut needs access totaskUrlsfrom options. Pass the merged configuration:if (newSession?.status === 'pending') { - warnMissingPendingTaskHandlers({ ...this.#options, ...params }); + warnMissingPendingTaskHandlers({ ...this.#options, ...params }); }
1201-1204: Fix parameter destructuring and type safety.The current string session handling uses unsafe type assertion. Improve type safety:
- public setActive = async (params: SetActiveParams): Promise<void> => { - const { organization, beforeEmit, redirectUrl, navigate: setActiveNavigate } = params; - let { session } = params; - this.__internal_setActiveInProgress = true; + public setActive = async (params: SetActiveParams): Promise<void> => { + const { organization, beforeEmit, redirectUrl, navigate: setActiveNavigate } = params; + let { session } = params; + this.__internal_setActiveInProgress = true;For the string session handling (lines 1217-1219):
if (typeof session === 'string') { - session = (this.client.sessions.find(x => x.id === session) as SignedInSessionResource) || null; + const foundSession = this.client.sessions.find(x => x.id === session); + if (!foundSession) { + throw new Error(`Session with ID "${session}" not found.`); + } + session = foundSession as SignedInSessionResource; }
🧹 Nitpick comments (2)
packages/nextjs/src/client-boundary/controlComponents.ts (1)
13-13: Add a minimal smoke test to assert the new export is available.No tests appear to cover this rename from the Next.js surface. Add a small test to prevent regressions.
Example:
// packages/nextjs/__tests__/client-boundary/controlComponents.test.ts import { RedirectToTasks } from '../../src/client-boundary/controlComponents'; describe('controlComponents exports', () => { it('exposes RedirectToTasks', () => { expect(RedirectToTasks).toBeDefined(); }); });I can add this test (and optionally a temporary alias test if you keep BC for one cycle). Do you want me to open a follow-up PR?
packages/clerk-js/src/ui/contexts/components/SignIn.ts (1)
40-40: Remove unused destructured variables.The
basePathvariable is destructured but not used in the current implementation. This creates unnecessary clutter.- const { navigate, basePath } = useRouter(); + const { navigate } = useRouter();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
packages/clerk-js/bundlewatch.config.json(1 hunks)packages/clerk-js/src/core/clerk.ts(22 hunks)packages/clerk-js/src/core/sessionTasks.ts(1 hunks)packages/clerk-js/src/ui/common/redirects.ts(1 hunks)packages/clerk-js/src/ui/contexts/components/SignIn.ts(4 hunks)packages/clerk-js/src/ui/contexts/components/SignUp.ts(4 hunks)packages/clerk-js/src/utils/index.ts(1 hunks)packages/clerk-js/src/utils/redirects.ts(1 hunks)packages/nextjs/src/client-boundary/controlComponents.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/clerk-js/bundlewatch.config.json
- packages/clerk-js/src/ui/contexts/components/SignUp.ts
- packages/clerk-js/src/utils/redirects.ts
- packages/clerk-js/src/core/sessionTasks.ts
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/clerk-js/src/utils/index.tspackages/nextjs/src/client-boundary/controlComponents.tspackages/clerk-js/src/ui/contexts/components/SignIn.tspackages/clerk-js/src/ui/common/redirects.tspackages/clerk-js/src/core/clerk.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/clerk-js/src/utils/index.tspackages/nextjs/src/client-boundary/controlComponents.tspackages/clerk-js/src/ui/contexts/components/SignIn.tspackages/clerk-js/src/ui/common/redirects.tspackages/clerk-js/src/core/clerk.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/utils/index.tspackages/nextjs/src/client-boundary/controlComponents.tspackages/clerk-js/src/ui/contexts/components/SignIn.tspackages/clerk-js/src/ui/common/redirects.tspackages/clerk-js/src/core/clerk.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/clerk-js/src/utils/index.tspackages/nextjs/src/client-boundary/controlComponents.tspackages/clerk-js/src/ui/contexts/components/SignIn.tspackages/clerk-js/src/ui/common/redirects.tspackages/clerk-js/src/core/clerk.ts
packages/**/index.{js,ts}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use tree-shaking friendly exports
Files:
packages/clerk-js/src/utils/index.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/clerk-js/src/utils/index.tspackages/nextjs/src/client-boundary/controlComponents.tspackages/clerk-js/src/ui/contexts/components/SignIn.tspackages/clerk-js/src/ui/common/redirects.tspackages/clerk-js/src/core/clerk.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/clerk-js/src/utils/index.tspackages/nextjs/src/client-boundary/controlComponents.tspackages/clerk-js/src/ui/contexts/components/SignIn.tspackages/clerk-js/src/ui/common/redirects.tspackages/clerk-js/src/core/clerk.ts
**/index.ts
📄 CodeRabbit Inference Engine (.cursor/rules/react.mdc)
Use index.ts files for clean imports but avoid deep barrel exports
Avoid barrel files (index.ts re-exports) as they can cause circular dependencies
Files:
packages/clerk-js/src/utils/index.ts
**/*
⚙️ CodeRabbit Configuration File
If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.
Files:
packages/clerk-js/src/utils/index.tspackages/nextjs/src/client-boundary/controlComponents.tspackages/clerk-js/src/ui/contexts/components/SignIn.tspackages/clerk-js/src/ui/common/redirects.tspackages/clerk-js/src/core/clerk.ts
packages/clerk-js/src/ui/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/clerk-js-ui.mdc)
packages/clerk-js/src/ui/**/*.{ts,tsx}: Element descriptors should always be camelCase
Use element descriptors in UI components to enable consistent theming and styling via appearance.elements
Element descriptors should generate unique, stable CSS classes for theming
Element descriptors should handle state classes (e.g., cl-loading, cl-active, cl-error, cl-open) automatically based on component state
Do not render hard-coded values; all user-facing strings must be localized using provided localization methods
Use the useLocalizations hook and localizationKeys utility for all text and error messages
Use the styled system (sx prop, theme tokens, responsive values) for custom component styling
Use useCardState for card-level state, useFormState for form-level state, and useLoadingStatus for loading states
Always use handleError utility for API errors and use translateError for localized error messages
Use useFormControl for form field state, implement proper validation, and handle loading and error states in forms
Use localization keys for all form labels and placeholders
Use element descriptors for consistent styling and follow the theme token system
Use the Card and FormContainer patterns for consistent UI structure
Files:
packages/clerk-js/src/ui/contexts/components/SignIn.tspackages/clerk-js/src/ui/common/redirects.ts
🧠 Learnings (1)
📚 Learning: 2025-08-08T19:00:08.931Z
Learnt from: LauraBeatris
PR: clerk/javascript#6486
File: packages/clerk-js/src/core/clerk.ts:1303-1344
Timestamp: 2025-08-08T19:00:08.931Z
Learning: In Clerk's navigation system for pending sessions, `taskUrls` (configured at the ClerkProvider level) should take priority over the `navigate` callback (from All-In-One components). This ensures that explicit provider-level configuration overrides component-level navigation logic, providing developers with predictable control over navigation behavior.
Applied to files:
packages/clerk-js/src/core/clerk.ts
🧬 Code Graph Analysis (2)
packages/clerk-js/src/ui/contexts/components/SignIn.ts (6)
packages/types/src/session.ts (1)
SessionResource(208-261)packages/clerk-js/src/ui/types.ts (1)
SignInCtx(64-68)packages/clerk-js/src/ui/router/RouteContext.tsx (1)
useRouter(33-39)packages/clerk-js/src/core/resources/Session.ts (1)
currentTask(390-393)packages/react/src/isomorphicClerk.ts (1)
session(672-678)packages/clerk-js/src/core/sessionTasks.ts (2)
INTERNAL_SESSION_TASK_ROUTE_BY_KEY(10-12)buildTaskRedirectUrl(19-33)
packages/clerk-js/src/core/clerk.ts (7)
packages/clerk-js/src/utils/componentGuards.ts (1)
isSignedInAndSingleSessionModeEnabled(9-11)packages/types/src/clerk.ts (1)
SetActiveParams(1189-1235)packages/react/src/isomorphicClerk.ts (1)
session(672-678)packages/types/src/session.ts (2)
SignedInSessionResource(285-285)SessionResource(208-261)packages/clerk-js/src/core/sessionTasks.ts (3)
warnMissingPendingTaskHandlers(49-63)INTERNAL_SESSION_TASK_ROUTE_BY_KEY(10-12)buildTaskUrl(38-47)packages/clerk-js/src/core/resources/Session.ts (1)
currentTask(390-393)packages/clerk-js/src/utils/runtime.ts (1)
inBrowser(1-3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (10)
packages/nextjs/src/client-boundary/controlComponents.ts (1)
13-13: Rename to RedirectToTasks is correct and consistent with the PR direction.The re-export aligns with the cross-package rename and keeps Next.js consumers in sync with @clerk/clerk-react.
packages/clerk-js/src/utils/index.ts (1)
24-24: LGTM! Clean barrel export addition.The addition of
export * from './redirects'is appropriate and follows the established pattern in the file. This properly exposes the new redirect utilities to consumers of the utils module.packages/clerk-js/src/ui/contexts/components/SignIn.ts (1)
132-140: Ignore thenewSessionsubstitution suggestion—newSessionisn’t in scope here
TheSignIn.tscomponent only ever referencesclerk.session; there are nonewSessionvariables or props passed into this context. If you need to computetaskUrlbased on the pending session rather than the current one, you’ll first have to exposenewSessionthrough your UI context or perform the URL computation upstream wherenewSessionis available.Likely an incorrect or invalid review comment.
packages/clerk-js/src/ui/common/redirects.ts (2)
1-1: LGTM! Centralized redirect logic.Good refactor to use the centralized
buildRedirectUrlfrom the utils module instead of maintaining duplicate URL-building logic.
3-3: LGTM! Updated imports align with context refactoring.The import path change reflects the broader refactoring to centralize context types, which improves maintainability.
packages/clerk-js/src/core/clerk.ts (5)
64-64: LGTM! Type import update aligns with session task changes.The import of
SessionResourceinstead of other session types aligns with the broader refactoring to use unified session types for task handling.
112-112: LGTM! Component guard consolidation.Using
isSignedInAndSingleSessionModeEnabledconsistently across bothopenSignInandopenSignUpmethods improves maintainability.
154-154: LGTM! Session task utilities import.The import of session task utilities (
buildTaskUrl,INTERNAL_SESSION_TASK_ROUTE_BY_KEY,warnMissingPendingTaskHandlers) supports the new per-task routing model.
1554-1574: LGTM! Well-implemented task URL builder.The
buildTasksUrlmethod properly:
- Checks for current task existence
- Prioritizes custom task URLs from options
- Falls back to building default URLs
- Uses appropriate URL building utilities
1666-1671: LGTM! Consistent redirect method.The
redirectToTasksmethod follows the same pattern as other redirect methods and properly delegates tobuildTasksUrl.
| const setActiveNavigate = async ({ | ||
| session, | ||
| baseUrl, | ||
| redirectUrl, | ||
| }: { | ||
| session: SessionResource; | ||
| baseUrl: string; | ||
| redirectUrl: string; | ||
| }) => { | ||
| if (!session.currentTask) { | ||
| await this.navigate(redirectUrl); | ||
| return; | ||
| } | ||
|
|
||
| await navigate(params.taskUrl ?? buildTaskUrl(session.currentTask, { baseUrl })); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve task navigation in OAuth callback.
The current implementation doesn't preserve redirectUrl when navigating to tasks. Also consider using the centralized task URL building:
const setActiveNavigate = async ({
session,
baseUrl,
redirectUrl,
}: {
session: SessionResource;
baseUrl: string;
redirectUrl: string;
}) => {
if (!session.currentTask) {
await this.navigate(redirectUrl);
return;
}
- await navigate(params.taskUrl ?? buildTaskUrl(session.currentTask, { baseUrl }));
+ const customTaskUrl = this.#options.taskUrls?.[session.currentTask.key];
+ const taskUrl = customTaskUrl ?? params.taskUrl ?? buildTaskUrl(session.currentTask, { baseUrl });
+ await navigate(taskUrl);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const setActiveNavigate = async ({ | |
| session, | |
| baseUrl, | |
| redirectUrl, | |
| }: { | |
| session: SessionResource; | |
| baseUrl: string; | |
| redirectUrl: string; | |
| }) => { | |
| if (!session.currentTask) { | |
| await this.navigate(redirectUrl); | |
| return; | |
| } | |
| await navigate(params.taskUrl ?? buildTaskUrl(session.currentTask, { baseUrl })); | |
| }; | |
| const setActiveNavigate = async ({ | |
| session, | |
| baseUrl, | |
| redirectUrl, | |
| }: { | |
| session: SessionResource; | |
| baseUrl: string; | |
| redirectUrl: string; | |
| }) => { | |
| if (!session.currentTask) { | |
| await this.navigate(redirectUrl); | |
| return; | |
| } | |
| const customTaskUrl = this.#options.taskUrls?.[session.currentTask.key]; | |
| const taskUrl = customTaskUrl ?? params.taskUrl ?? buildTaskUrl(session.currentTask, { baseUrl }); | |
| await navigate(taskUrl); | |
| }; |
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/clerk.ts around lines 1854 to 1869, the task
navigation handler drops the original redirectUrl and references an undefined
params variable; update it to preserve redirectUrl and use the centralized task
URL builder by calling the shared buildTaskUrl with both baseUrl and redirectUrl
(e.g. buildTaskUrl(session.currentTask, { baseUrl, redirectUrl })) and remove
the undefined params usage so the navigate call consistently navigates to the
constructed task URL when session.currentTask exists.
| const setActiveNavigate = async ({ session, redirectUrl }: { session: SessionResource; redirectUrl: string }) => { | ||
| if (!session.currentTask) { | ||
| await this.navigate(redirectUrl); | ||
| return; | ||
| } | ||
|
|
||
| await this.navigate( | ||
| buildTaskUrl(session.currentTask, { | ||
| baseUrl: displayConfig.signInUrl, | ||
| }), | ||
| ); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Align Web3 navigation with OAuth pattern.
The Web3 flow should also respect provider-level taskUrls configuration like the OAuth flow:
const setActiveNavigate = async ({ session, redirectUrl }: { session: SessionResource; redirectUrl: string }) => {
if (!session.currentTask) {
await this.navigate(redirectUrl);
return;
}
- await this.navigate(
- buildTaskUrl(session.currentTask, {
- baseUrl: displayConfig.signInUrl,
- }),
- );
+ const customTaskUrl = this.#options.taskUrls?.[session.currentTask.key];
+ const taskUrl = customTaskUrl ?? buildTaskUrl(session.currentTask, {
+ baseUrl: displayConfig.signInUrl,
+ });
+ await this.navigate(taskUrl);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const setActiveNavigate = async ({ session, redirectUrl }: { session: SessionResource; redirectUrl: string }) => { | |
| if (!session.currentTask) { | |
| await this.navigate(redirectUrl); | |
| return; | |
| } | |
| await this.navigate( | |
| buildTaskUrl(session.currentTask, { | |
| baseUrl: displayConfig.signInUrl, | |
| }), | |
| ); | |
| }; | |
| const setActiveNavigate = async ({ session, redirectUrl }: { session: SessionResource; redirectUrl: string }) => { | |
| if (!session.currentTask) { | |
| await this.navigate(redirectUrl); | |
| return; | |
| } | |
| const customTaskUrl = this.#options.taskUrls?.[session.currentTask.key]; | |
| const taskUrl = customTaskUrl ?? buildTaskUrl(session.currentTask, { | |
| baseUrl: displayConfig.signInUrl, | |
| }); | |
| await this.navigate(taskUrl); | |
| }; |
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/clerk.ts around lines 2167 to 2178, the Web3
navigation always builds the task URL using displayConfig.signInUrl and ignores
provider-level taskUrls like the OAuth flow; update the logic so when
session.currentTask exists you first check the session's active provider (or
provider for the session) for a matching taskUrls entry and navigate to that URL
if present, otherwise fall back to buildTaskUrl(...) with
displayConfig.signInUrl; keep the existing early-return that navigates to
redirectUrl when there is no currentTask, and ensure you await the navigate
call.
| const navigateOnSetActive = async ({ session, redirectUrl }: { session: SessionResource; redirectUrl: string }) => { | ||
| const currentTask = session.currentTask; | ||
| if (!currentTask) { | ||
| return navigate(redirectUrl); | ||
| } | ||
|
|
||
| return navigate(`/${basePath}/tasks/${INTERNAL_SESSION_TASK_ROUTE_BY_KEY[currentTask.key]}`); | ||
| }; |
There was a problem hiding this comment.
Critical issues: Remove debug code and fix navigation logic.
Multiple issues need addressing:
- Lint error: The
debuggerstatement violates ESLint rules - Incorrect task URL construction: Manual path joining with
basePathis error-prone - Missing provider-level
taskUrlssupport: Should respectClerkProvider'staskUrlsconfiguration - Poor fallback handling: No fallback when
redirectUrlis missing for active sessions
- const navigateOnSetActive = async ({ session, redirectUrl }: { session: SessionResource; redirectUrl: string }) => {
- debugger;
- const currentTask = session.currentTask;
- if (!currentTask) {
- return navigate(redirectUrl);
- }
-
- return navigate(`/${basePath}/tasks/${INTERNAL_SESSION_TASK_ROUTE_BY_KEY[currentTask.key]}`);
- };
+ const navigateOnSetActive = useCallback(
+ async ({ session, redirectUrl }: { session: SessionResource; redirectUrl?: string }) => {
+ const currentTask = session.currentTask;
+ if (!currentTask) {
+ // No pending task: use provided redirectUrl or fall back to afterSignInUrl
+ return navigate(redirectUrl || afterSignInUrl);
+ }
+ // Pending task: prefer provider override, otherwise build the default task URL
+ const customTaskUrl = clerk.__internal_getOption('taskUrls')?.[currentTask.key];
+ const target = customTaskUrl ?? buildTaskRedirectUrl(currentTask, {
+ routing: ctx.routing,
+ baseUrl: signInUrl,
+ authQueryString,
+ path: ctx.path,
+ });
+ return navigate(target);
+ },
+ [navigate, clerk, signInUrl, afterSignInUrl, ctx.routing, authQueryString, ctx.path],
+ );Add missing import:
-import { createContext, useContext, useMemo } from 'react';
+import { createContext, useContext, useMemo, useCallback } from 'react';Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/clerk-js/src/ui/contexts/components/SignIn.ts around lines 123-130,
remove any leftover debugger statements, stop manually concatenating paths with
basePath, and instead construct task URLs via the provider-level taskUrls
mapping (falling back to INTERNAL_SESSION_TASK_ROUTE_BY_KEY when provider
mapping missing); ensure you read the taskUrls from ClerkProvider context and
call the shared path-building utility (add the missing import for that utility)
to produce a correct absolute URL, and if redirectUrl is absent for an active
session fall back to a safe default (e.g., provider taskUrl fallback or '/')
before calling navigate.
Description
This PR introduces a new API to navigate on
setActive, that can be leveraged for after-auth flows to handle pending sessions, featuring a callback mechanism that enables developers to integrate their application logic, such as navigation.Previously, our flows were mistakenly navigating after
setActive, which could cause issues with session revalidation and re-render of pages.From now on, we're proposing two options to deal with after-auth:
taskUrlsornavigatenavigateA callback function that can be provided as an
setActiveoption.taskUrls(existing)A map of URLs per session task key that can be provided as a Clerk option. On
setActive, it'll be used for navigation.AIOs behavior
CleanShot.2025-08-07.at.22.30.57.mp4
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Breaking Changes
Tests
Chores