-
Notifications
You must be signed in to change notification settings - Fork 371
feat(clerk-js): Introduce navigate
for setActive
#6486
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: c0c0091 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 ↗︎
|
onPendingStatus
onPendingSession
970cf52
to
cd73364
Compare
c3ad953
to
45a338c
Compare
beb0443
to
1b3771c
Compare
d270d37
to
d185134
Compare
dcc6bfa
to
0c4e976
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/clerk-js/src/core/sessionTasks.ts (2)
9-11
: Tighten mapping with satisfies for exhaustive type-safety.Use satisfies to ensure the map stays in sync with SessionTask['key'] without needing Record<> + as const.
-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', +} satisfies Record<SessionTask['key'], string>;
16-23
: Explicit return type and preserve redirectUrl in hash for SPA routing.Add an explicit return type and append redirect_url to the hash to survive client-side routing. This also addresses the PR objective to preserve redirectUrl.
-export function buildTasksUrl(task: SessionTask, opts: Pick<Parameters<typeof buildURL>[0], 'base'>) { - return buildURL( - { - base: opts.base, - hashPath: `/tasks/${INTERNAL_SESSION_TASK_ROUTE_BY_KEY[task.key]}`, - }, - { stringify: true }, - ); -} +export function buildTasksUrl( + task: SessionTask, + opts: { base: string; redirectUrl?: string }, +): string { + const params = new URLSearchParams(); + if (opts.redirectUrl) { + params.set('redirect_url', opts.redirectUrl); + } + return buildURL( + { + base: opts.base, + hashPath: `/tasks/${INTERNAL_SESSION_TASK_ROUTE_BY_KEY[task.key]}`, + hashSearchParams: params, + }, + { stringify: true }, + ) as string; +}
🧹 Nitpick comments (7)
packages/clerk-js/src/core/sessionTasks.ts (3)
47-55
: Tighten types, remove any-casts, and add explicit return type.Make the API self-descriptive and avoid loose Record usage.
-export function warnMissingPendingTaskHandlers(options: Record<string, unknown>) { - const taskOptions = ['taskUrls', 'navigate'] as Array< - keyof (Pick<SetActiveParams, 'navigate'> & Pick<ClerkOptions, 'taskUrls'>) - >; +export function warnMissingPendingTaskHandlers( + options: Partial<Pick<SetActiveParams, 'navigate'> & Pick<ClerkOptions, 'taskUrls'>>, +): void { + const taskOptions = ['taskUrls', 'navigate'] as const; - const hasAtLeastOneOption = Object.keys(options).some(option => taskOptions.includes(option as any)); + const hasAtLeastOneOption = taskOptions.some(opt => options[opt] != null); if (hasAtLeastOneOption) { return; } // TODO - Link to after-auth docs once it gets released logger.warnOnce( `Clerk: Session has pending tasks but no handling is configured. To handle pending tasks, provide either "taskUrls" for navigation to custom URLs or "navigate" for programmatic navigation. Without these options, users may get stuck on incomplete flows.`, ); }Also applies to: 57-60
57-60
: Docs TODO: add link.Replace the TODO with a link to the after-auth docs when available.
29-45
: Add unit tests for navigation and warnings.Cover:
- navigateIfTaskExists returns void when no currentTask and calls navigate with a URL containing redirect_url when provided.
- warnMissingPendingTaskHandlers logs a single warning only when neither taskUrls nor navigate are present.
I can draft Jest tests if helpful.
packages/types/src/clerk.ts (4)
1205-1208
: Clarify precedence of redirectUrl vs navigate in docs.Make it explicit that
redirectUrl
is ignored whennavigate
is provided.- /** - * The full URL or path to redirect to just before the session and/or organization is set. - */ + /** + * The full URL or path to redirect to just before the session and/or organization is set. + * Ignored when a `navigate` callback is provided. + */
678-682
: Optional: expand JSDoc to mention routing strategy.Consider noting that the returned URL respects the app routing strategy (path/hash/virtual) to set expectations for integrators.
770-774
: Optional: mention redirectUrl preservation behavior.If
setActive({ redirectUrl })
is provided, document thatredirectToTasks
or the internal navigation flow will appendredirect_url
to the task URL to survive SPA routing.
1231-1231
: Tests suggestion for new API surface.Add tests covering:
setActive
callingnavigate
when provided, and not usingredirectUrl
in that case.redirectToTasks
navigating to the configured tasks root.I can scaffold the test cases upon request.
📜 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.snap
is excluded by!**/*.snap
packages/remix/src/__tests__/__snapshots__/exports.test.ts.snap
is excluded by!**/*.snap
packages/tanstack-react-start/src/__tests__/__snapshots__/exports.test.ts.snap
is excluded by!**/*.snap
📒 Files selected for processing (11)
.changeset/warm-rocks-flow.md
(1 hunks)packages/clerk-js/src/core/clerk.ts
(20 hunks)packages/clerk-js/src/core/sessionTasks.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/nextjs/src/client-boundary/controlComponents.ts
(1 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
(5 hunks)packages/vue/src/components/controlComponents.ts
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/react/src/components/index.ts
- .changeset/warm-rocks-flow.md
- packages/vue/src/components/controlComponents.ts
- packages/nextjs/src/client-boundary/controlComponents.ts
- packages/react/src/components/controlComponents.tsx
- packages/clerk-js/src/ui/contexts/components/SignUp.ts
- packages/react/src/isomorphicClerk.ts
- packages/clerk-js/src/ui/contexts/components/SignIn.ts
- packages/clerk-js/src/core/clerk.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{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/core/sessionTasks.ts
packages/types/src/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/core/sessionTasks.ts
packages/types/src/clerk.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/core/sessionTasks.ts
packages/types/src/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/core/sessionTasks.ts
packages/types/src/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
Avoidany
type - preferunknown
when type is uncertain, then narrow with type guards
Useinterface
for object shapes that might be extended
Usetype
for unions, primitives, and computed types
Preferreadonly
properties for immutable data structures
Useprivate
for internal implementation details
Useprotected
for inheritance hierarchies
Usepublic
explicitly for clarity in public APIs
Preferreadonly
for 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 assertions
for literal types:as const
Usesatisfies
operator 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 ...
Noany
types without justification
Proper error handling with typed errors
Consistent use ofreadonly
for 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/core/sessionTasks.ts
packages/types/src/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/core/sessionTasks.ts
packages/types/src/clerk.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/core/sessionTasks.ts
packages/types/src/clerk.ts
🧬 Code Graph Analysis (2)
packages/clerk-js/src/core/sessionTasks.ts (1)
packages/types/src/session.ts (1)
SessionResource
(208-261)
packages/types/src/clerk.ts (1)
packages/types/src/session.ts (1)
SessionResource
(208-261)
⏰ 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: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
export function navigateIfTaskExists( | ||
session: SessionResource, | ||
{ | ||
navigate, | ||
baseUrl, | ||
}: { | ||
navigate: (to: string) => Promise<unknown>; | ||
baseUrl: string; | ||
}, | ||
) { | ||
const customTaskUrl = options?.taskUrls?.[routeKey]; | ||
const internalTaskRoute = `/tasks/${INTERNAL_SESSION_TASK_ROUTE_BY_KEY[routeKey]}`; | ||
const currentTask = session.currentTask; | ||
if (!currentTask) { | ||
return; | ||
} | ||
|
||
return navigate(buildTasksUrl(currentTask, { base: baseUrl })); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Propagate redirectUrl and add explicit return type.
Let callers preserve redirectUrl during task navigation and declare the return type.
-export function navigateIfTaskExists(
- session: SessionResource,
- {
- navigate,
- baseUrl,
- }: {
- navigate: (to: string) => Promise<unknown>;
- baseUrl: string;
- },
-) {
+export function navigateIfTaskExists(
+ session: SessionResource,
+ {
+ navigate,
+ baseUrl,
+ redirectUrl,
+ }: {
+ navigate: (to: string) => Promise<unknown>;
+ baseUrl: string;
+ redirectUrl?: string;
+ },
+): Promise<unknown> | void {
const currentTask = session.currentTask;
if (!currentTask) {
return;
}
- return navigate(buildTasksUrl(currentTask, { base: baseUrl }));
+ return navigate(buildTasksUrl(currentTask, { base: baseUrl, redirectUrl }));
}
Follow-up: if custom taskUrls overrides are applied elsewhere, ensure redirect_url is appended to the right place (query vs hash) consistently.
📝 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.
export function navigateIfTaskExists( | |
session: SessionResource, | |
{ | |
navigate, | |
baseUrl, | |
}: { | |
navigate: (to: string) => Promise<unknown>; | |
baseUrl: string; | |
}, | |
) { | |
const customTaskUrl = options?.taskUrls?.[routeKey]; | |
const internalTaskRoute = `/tasks/${INTERNAL_SESSION_TASK_ROUTE_BY_KEY[routeKey]}`; | |
const currentTask = session.currentTask; | |
if (!currentTask) { | |
return; | |
} | |
return navigate(buildTasksUrl(currentTask, { base: baseUrl })); | |
} | |
export function navigateIfTaskExists( | |
session: SessionResource, | |
{ | |
navigate, | |
baseUrl, | |
redirectUrl, | |
}: { | |
navigate: (to: string) => Promise<unknown>; | |
baseUrl: string; | |
redirectUrl?: string; | |
}, | |
): Promise<unknown> | void { | |
const currentTask = session.currentTask; | |
if (!currentTask) { | |
return; | |
} | |
return navigate(buildTasksUrl(currentTask, { base: baseUrl, redirectUrl })); | |
} |
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/sessionTasks.ts around lines 29 to 45, the
function navigateIfTaskExists lacks an explicit return type and does not
propagate the redirectUrl parameter. Update the function signature to include
redirectUrl as an optional parameter, ensure it is passed through to
buildTasksUrl so the redirectUrl is preserved during navigation, and explicitly
declare the function's return type as Promise<unknown> | undefined. Also verify
that redirectUrl is appended correctly in buildTasksUrl, consistent with any
custom taskUrl overrides.
0c4e976
to
437169e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (4)
packages/clerk-js/src/ui/components/SignIn/shared.ts (2)
29-64
: Add missing useCallback dependencies (react-hooks/exhaustive-deps).This callback closes over multiple values but has an empty dependency array, causing stale captures and violating ESLint. Include all referenced deps.
Apply this diff:
- return useCallback(async (...args: Parameters<typeof authenticateWithPasskey>) => { + return useCallback(async (...args: Parameters<typeof authenticateWithPasskey>) => { try { const res = await authenticateWithPasskey(...args); switch (res.status) { case 'complete': return setActive({ session: res.createdSessionId, navigate: async ({ session }) => { await navigateOnSetActive({ session, redirectUrl: afterSignInUrl }); }, }); case 'needs_second_factor': return onSecondFactor(); default: return console.error(clerkInvalidFAPIResponse(res.status, supportEmail)); } } catch (err) { const { flow } = args[0] || {}; if (isClerkRuntimeError(err)) { if (err.code === 'passkey_operation_aborted') { return; } if (flow === 'autofill' && err.code === 'passkey_retrieval_cancelled') { return; } } if (isUserLockedError(err)) { return __internal_navigateWithError('..', err.errors[0]); } handleError(err, [], card.setError); } - }, []); + }, [ + authenticateWithPasskey, + onSecondFactor, + setActive, + navigateOnSetActive, + afterSignInUrl, + supportEmail, + card.setError, + __internal_navigateWithError, + ]);
34-39
: Preserve redirectUrl through task navigation (do not drop afterSignInUrl during tasks).You pass redirectUrl to navigateOnSetActive, but the current implementation (see packages/clerk-js/src/ui/contexts/components/SignIn.ts) ignores it when a task exists and navigates to /tasks/... without appending redirect_url. This risks losing the final post-auth destination.
Recommended follow-ups:
- Update buildTasksUrl to accept and append redirectUrl (hash search param).
- Update navigateIfTaskExists to forward redirectUrl.
Run the following to locate definitions/usages:
#!/bin/bash # Find buildTasksUrl definition and all call sites rg -n "function buildTasksUrl|buildTasksUrl\(" -A 2 -g "packages/**/*.ts*" # Find navigateIfTaskExists definition and all call sites rg -n "function navigateIfTaskExists|navigateIfTaskExists\(" -A 2 -g "packages/**/*.ts*" # Check for any usage of 'redirect_url' in task routes rg -n "redirect_url" -g "packages/**/*.ts*"packages/clerk-js/src/core/sessionTasks.ts (2)
16-23
: Add explicit return type and propagate redirectUrl in buildTasksUrl.To preserve the developer’s redirectUrl through tasks and improve type clarity, accept an optional redirectUrl and append it to the hash search params.
-export function buildTasksUrl(task: SessionTask, opts: Pick<Parameters<typeof buildURL>[0], 'base'>) { - return buildURL( - { - base: opts.base, - hashPath: `/tasks/${INTERNAL_SESSION_TASK_ROUTE_BY_KEY[task.key]}`, - }, - { stringify: true }, - ); -} +export function buildTasksUrl( + task: SessionTask, + opts: { base: string; redirectUrl?: string }, +): string { + const hashSearchParams = + opts.redirectUrl ? new URLSearchParams([['redirect_url', opts.redirectUrl]]) : undefined; + return buildURL( + { + base: opts.base, + hashPath: `/tasks/${INTERNAL_SESSION_TASK_ROUTE_BY_KEY[task.key]}`, + hashSearchParams, + }, + { stringify: true }, + ) as string; +}
29-45
: Propagate redirectUrl and add explicit return type in navigateIfTaskExists.Forward redirectUrl to the task URL builder so the final destination survives task flows. Also declare the return type.
-export function navigateIfTaskExists( - session: SessionResource, - { - navigate, - baseUrl, - }: { - navigate: (to: string) => Promise<unknown>; - baseUrl: string; - }, -) { +export function navigateIfTaskExists( + session: SessionResource, + { + navigate, + baseUrl, + redirectUrl, + }: { + navigate: (to: string) => Promise<unknown>; + baseUrl: string; + redirectUrl?: string; + }, +): Promise<unknown> | void { const currentTask = session.currentTask; if (!currentTask) { return; } - return navigate(buildTasksUrl(currentTask, { base: baseUrl })); + return navigate(buildTasksUrl(currentTask, { base: baseUrl, redirectUrl })); }
🧹 Nitpick comments (2)
packages/clerk-js/src/core/sessionTasks.ts (2)
9-11
: Use satisfies to tighten the task route map typing.Strengthen typing for future task additions and avoid broad Record usage.
-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', +} satisfies Record<SessionTask['key'], string>;
47-60
: Tighten typing and simplify logic in warnMissingPendingTaskHandlers.Prefer a precise options type and a straightforward presence check over Object.keys scans.
-export function warnMissingPendingTaskHandlers(options: Record<string, unknown>) { - const taskOptions = ['taskUrls', 'navigate'] as Array< - keyof (Pick<SetActiveParams, 'navigate'> & Pick<ClerkOptions, 'taskUrls'>) - >; - - const hasAtLeastOneOption = Object.keys(options).some(option => taskOptions.includes(option as any)); - if (hasAtLeastOneOption) { - return; - } +export function warnMissingPendingTaskHandlers( + options: Partial<Pick<SetActiveParams, 'navigate'> & Pick<ClerkOptions, 'taskUrls'>>, +) { + if (options.taskUrls || options.navigate) { + return; + } // TODO - Link to after-auth docs once it gets released logger.warnOnce( `Clerk: Session has pending tasks but no handling is configured. To handle pending tasks, provide either "taskUrls" for navigation to custom URLs or "navigate" for programmatic navigation. Without these options, users may get stuck on incomplete flows.`, ); }
📜 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.snap
is excluded by!**/*.snap
packages/remix/src/__tests__/__snapshots__/exports.test.ts.snap
is excluded by!**/*.snap
packages/tanstack-react-start/src/__tests__/__snapshots__/exports.test.ts.snap
is excluded by!**/*.snap
📒 Files selected for processing (12)
.changeset/warm-rocks-flow.md
(1 hunks)packages/clerk-js/src/core/clerk.ts
(20 hunks)packages/clerk-js/src/core/sessionTasks.ts
(1 hunks)packages/clerk-js/src/ui/components/SignIn/shared.ts
(3 hunks)packages/clerk-js/src/ui/contexts/components/SignIn.ts
(4 hunks)packages/clerk-js/src/ui/contexts/components/SignUp.ts
(4 hunks)packages/nextjs/src/client-boundary/controlComponents.ts
(1 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
(5 hunks)packages/vue/src/components/controlComponents.ts
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/react/src/components/index.ts
- .changeset/warm-rocks-flow.md
- packages/clerk-js/src/ui/contexts/components/SignUp.ts
- packages/react/src/components/controlComponents.tsx
- packages/nextjs/src/client-boundary/controlComponents.ts
- packages/vue/src/components/controlComponents.ts
- packages/clerk-js/src/ui/contexts/components/SignIn.ts
- packages/types/src/clerk.ts
- packages/react/src/isomorphicClerk.ts
- packages/clerk-js/src/core/clerk.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{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/core/sessionTasks.ts
packages/clerk-js/src/ui/components/SignIn/shared.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/core/sessionTasks.ts
packages/clerk-js/src/ui/components/SignIn/shared.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/core/sessionTasks.ts
packages/clerk-js/src/ui/components/SignIn/shared.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/core/sessionTasks.ts
packages/clerk-js/src/ui/components/SignIn/shared.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
Avoidany
type - preferunknown
when type is uncertain, then narrow with type guards
Useinterface
for object shapes that might be extended
Usetype
for unions, primitives, and computed types
Preferreadonly
properties for immutable data structures
Useprivate
for internal implementation details
Useprotected
for inheritance hierarchies
Usepublic
explicitly for clarity in public APIs
Preferreadonly
for 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 assertions
for literal types:as const
Usesatisfies
operator 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 ...
Noany
types without justification
Proper error handling with typed errors
Consistent use ofreadonly
for 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/core/sessionTasks.ts
packages/clerk-js/src/ui/components/SignIn/shared.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/core/sessionTasks.ts
packages/clerk-js/src/ui/components/SignIn/shared.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/core/sessionTasks.ts
packages/clerk-js/src/ui/components/SignIn/shared.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/components/SignIn/shared.ts
🧬 Code Graph Analysis (1)
packages/clerk-js/src/ui/components/SignIn/shared.ts (1)
packages/clerk-js/src/ui/contexts/components/SignIn.ts (1)
useSignInContext
(38-158)
🪛 ESLint
packages/clerk-js/src/ui/components/SignIn/shared.ts
[error] 19-19: 'signInUrl' is assigned a value but never used. Allowed unused vars must match /^_/u.
(@typescript-eslint/no-unused-vars)
[error] 21-21: 'navigate' is assigned a value but never used. Allowed unused vars must match /^_/u.
(@typescript-eslint/no-unused-vars)
⏰ 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 (1)
packages/clerk-js/src/core/sessionTasks.ts (1)
1-2
: LGTM: imports and internal docs.Logger import and internal docs look good.
Also applies to: 6-8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
integration/tests/session-tasks-sign-up.test.ts
(1 hunks)packages/clerk-js/src/ui/components/SignIn/shared.ts
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/clerk-js/src/ui/components/SignIn/shared.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}
: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
integration/tests/session-tasks-sign-up.test.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
integration/tests/session-tasks-sign-up.test.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}
: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidany
type - preferunknown
when type is uncertain, then narrow with type guards
Useinterface
for object shapes that might be extended
Usetype
for unions, primitives, and computed types
Preferreadonly
properties for immutable data structures
Useprivate
for internal implementation details
Useprotected
for inheritance hierarchies
Usepublic
explicitly for clarity in public APIs
Preferreadonly
for 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 assertions
for literal types:as const
Usesatisfies
operator 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 ...
Noany
types without justification
Proper error handling with typed errors
Consistent use ofreadonly
for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
integration/tests/session-tasks-sign-up.test.ts
integration/**
📄 CodeRabbit Inference Engine (.cursor/rules/global.mdc)
Framework integration templates and E2E tests should be placed under the integration/ directory
Files:
integration/tests/session-tasks-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
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
integration/tests/session-tasks-sign-up.test.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:
integration/tests/session-tasks-sign-up.test.ts
⏰ 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). (24)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (tanstack-react-router, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (quickstart, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Unit Tests (18, --filter=@clerk/astro --filter=@clerk/backend --filter=@clerk/express --filter=@c...
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Static analysis
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
a210986
to
86c21ce
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (3)
packages/clerk-js/src/ui/contexts/components/SignIn.ts (3)
135-138
: Minor: compute currentTask once (clearer, less repetition)Slight refactor for readability; keeps existing behavior and precedence.
- const taskUrl = clerk.session?.currentTask - ? (clerk.__internal_getOption('taskUrls')?.[clerk.session?.currentTask.key] ?? - buildTasksUrl(clerk.session?.currentTask, { base: signInUrl })) - : null; + const currentTask = clerk.session?.currentTask; + const taskUrl = currentTask + ? (clerk.__internal_getOption('taskUrls')?.[currentTask.key] ?? + buildTasksUrl(currentTask, { base: signInUrl })) + : null;
158-160
: Stabilize context values to reduce re-rendersYou’ve now memoized navigateOnSetActive with useCallback (good). Consider also memoizing taskUrl (e.g., with useMemo) if consumers depend on referential stability; otherwise they may re-render on every parent render.
If you prefer, wrap the entire returned object in useMemo with appropriate deps.
123-134
: Add tests for navigation callback behaviorPlease add tests to cover:
- Active session: uses redirectUrl when provided, otherwise afterSignInUrl.
- Pending task: honors provider-level taskUrls override; otherwise uses buildTasksUrl default.
- SSO callback flow: ensure navigateOnSetActive correctly routes to the task screen and does not lose state.
I can scaffold tests for these cases if helpful.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
integration/tests/session-tasks-sign-up.test.ts
(1 hunks)packages/clerk-js/src/ui/components/SignIn/shared.ts
(2 hunks)packages/clerk-js/src/ui/contexts/components/SignIn.ts
(4 hunks)packages/clerk-js/src/ui/contexts/components/SignUp.ts
(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- integration/tests/session-tasks-sign-up.test.ts
- packages/clerk-js/src/ui/components/SignIn/shared.ts
- packages/clerk-js/src/ui/contexts/components/SignUp.ts
🧰 Additional context used
📓 Path-based instructions (8)
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.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}
: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/clerk-js/src/ui/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/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/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/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
Avoidany
type - preferunknown
when type is uncertain, then narrow with type guards
Useinterface
for object shapes that might be extended
Usetype
for unions, primitives, and computed types
Preferreadonly
properties for immutable data structures
Useprivate
for internal implementation details
Useprotected
for inheritance hierarchies
Usepublic
explicitly for clarity in public APIs
Preferreadonly
for 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 assertions
for literal types:as const
Usesatisfies
operator 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 ...
Noany
types without justification
Proper error handling with typed errors
Consistent use ofreadonly
for 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/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/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/ui/contexts/components/SignIn.ts
🧠 Learnings (1)
📚 Learning: 2025-08-08T21:39:41.509Z
Learnt from: LauraBeatris
PR: clerk/javascript#6486
File: packages/clerk-js/src/ui/contexts/components/SignIn.ts:123-130
Timestamp: 2025-08-08T21:39:41.509Z
Learning: In the Clerk authentication flow, `redirectUrl` should only be used for navigation after sign-in/sign-up when the session is fully active (no pending tasks). When a session has pending tasks (session.currentTask exists), navigation should go directly to the task URL without preserving or using the `redirectUrl`. The `redirectUrl` represents the final destination after all authentication steps are complete, not an intermediate destination during task completion.
Applied to files:
packages/clerk-js/src/ui/contexts/components/SignIn.ts
🪛 Biome (2.1.2)
packages/clerk-js/src/ui/contexts/components/SignIn.ts
[error] 125-125: This is an unexpected use of the debugger statement.
Unsafe fix: Remove debugger statement
(lint/suspicious/noDebugger)
🪛 ESLint
packages/clerk-js/src/ui/contexts/components/SignIn.ts
[error] 125-125: Unexpected 'debugger' statement.
(no-debugger)
⏰ 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: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (3)
packages/clerk-js/src/ui/contexts/components/SignIn.ts (3)
3-3
: Type-only import is correctUsing a type-only import for SessionResource is appropriate and tree-shake friendly.
11-11
: Import looks fineThe redirects utilities are used correctly for email link and SSO callback URLs.
141-141
: No actionSpreading ctx into the returned context remains appropriate here.
86c21ce
to
39a3019
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
packages/clerk-js/src/ui/common/withRedirect.tsx (2)
31-41
: Redirect effect never fires when the condition flips after mount (breaks SSO callback flows)
useEffect
runs only once due to[]
, so ifshouldRedirect
becomes true later (e.g., after session/task state updates on SSO callbacks), navigation never occurs and the component rendersnull
without redirecting. Add proper deps and guard; also drop the floating-promise disable by usingvoid
.- const shouldRedirect = condition(clerk, environment, options); - React.useEffect(() => { - if (shouldRedirect) { - if (warning && isDevelopmentFromPublishableKey(clerk.publishableKey)) { - console.info(warning); - } - // TODO: Fix this properly - // eslint-disable-next-line @typescript-eslint/no-floating-promises - navigate(redirectUrl({ clerk, environment, options })); - } - }, []); + const shouldRedirect = condition(clerk, environment, options); + React.useEffect(() => { + if (!shouldRedirect) { + return; + } + if (warning && isDevelopmentFromPublishableKey(clerk.publishableKey)) { + console.info(warning); + } + void navigate(redirectUrl({ clerk, environment, options })); + }, [shouldRedirect, navigate, clerk, environment, options, redirectUrl, warning]);
55-55
: Add explicit return types to exported HOCs (public API) and JSDocPer guidelines, public APIs should have explicit return types and JSDoc. These HOCs return a React component.
-export const withRedirectToAfterSignIn = <P extends AvailableComponentProps>(Component: ComponentType<P>) => { +export const withRedirectToAfterSignIn = <P extends AvailableComponentProps>( + Component: ComponentType<P>, +): ComponentType<P> => {-export const withRedirectToAfterSignUp = <P extends AvailableComponentProps>(Component: ComponentType<P>) => { +export const withRedirectToAfterSignUp = <P extends AvailableComponentProps>( + Component: ComponentType<P>, +): ComponentType<P> => {-export const withRedirectToSignInTask = <P extends AvailableComponentProps>(Component: ComponentType<P>) => { +export const withRedirectToSignInTask = <P extends AvailableComponentProps>( + Component: ComponentType<P>, +): ComponentType<P> => {-export const withRedirectToSignUpTask = <P extends AvailableComponentProps>(Component: ComponentType<P>) => { +export const withRedirectToSignUpTask = <P extends AvailableComponentProps>( + Component: ComponentType<P>, +): ComponentType<P> => {Add short JSDoc atop each export:
- Purpose of the HOC
- Conditions under which redirect occurs
- Notes on single-session mode and task URL requirements
Also applies to: 74-74, 93-93, 115-115
🧹 Nitpick comments (3)
packages/clerk-js/src/ui/common/withRedirect.tsx (3)
100-107
: Solid guard to avoid navigate(''); remove non-null assertion with an invariantCondition correctly depends on
currentTask
andtaskUrl
. Drop theno-non-null-assertion
disable by asserting the invariant before returning the URL.- (clerk, environment) => - !!environment?.authConfig.singleSessionMode && !!(clerk.session?.currentTask && signInCtx?.taskUrl), - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - () => signInCtx.taskUrl!, + (clerk, environment) => + !!environment?.authConfig.singleSessionMode && !!(clerk.session?.currentTask && signInCtx?.taskUrl), + () => { + if (!signInCtx?.taskUrl) { + throw new Error('Invariant violated: taskUrl must be defined when redirect condition passes.'); + } + return signInCtx.taskUrl; + },
122-129
: Mirror the invariant removal for SignUp task redirectSame reasoning as the SignIn variant; prefer an explicit invariant over a non-null assertion and disable.
- (clerk, environment) => - !!environment?.authConfig.singleSessionMode && !!(clerk.session?.currentTask && signUpCtx?.taskUrl), - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - () => signUpCtx.taskUrl!, + (clerk, environment) => + !!environment?.authConfig.singleSessionMode && !!(clerk.session?.currentTask && signUpCtx?.taskUrl), + () => { + if (!signUpCtx?.taskUrl) { + throw new Error('Invariant violated: taskUrl must be defined when redirect condition passes.'); + } + return signUpCtx.taskUrl; + },
22-24
: Avoid mutating the wrapped component’s displayNameSetting
Component.displayName = displayName;
is redundant and mutates the wrapped component. Prefer only settingHOC.displayName
.- const displayName = Component.displayName || Component.name || 'Component'; - Component.displayName = displayName; + const displayName = Component.displayName || Component.name || 'Component';Apply similarly to all HOCs.
Also applies to: 56-57, 75-76, 94-95, 116-117
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/clerk-js/src/ui/common/withRedirect.tsx
(3 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
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/withRedirect.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/common/withRedirect.tsx
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/clerk-js/src/ui/common/withRedirect.tsx
packages/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/ui/common/withRedirect.tsx
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/clerk-js/src/ui/common/withRedirect.tsx
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}
: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidany
type - preferunknown
when type is uncertain, then narrow with type guards
Useinterface
for object shapes that might be extended
Usetype
for unions, primitives, and computed types
Preferreadonly
properties for immutable data structures
Useprivate
for internal implementation details
Useprotected
for inheritance hierarchies
Usepublic
explicitly for clarity in public APIs
Preferreadonly
for 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 assertions
for literal types:as const
Usesatisfies
operator 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 ...
Noany
types without justification
Proper error handling with typed errors
Consistent use ofreadonly
for 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/common/withRedirect.tsx
**/*.{jsx,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
**/*.{jsx,tsx}
: Use error boundaries in React components
Minimize re-renders in React components
**/*.{jsx,tsx}
: Always use functional components with hooks instead of class components
Follow PascalCase naming for components:UserProfile
,NavigationMenu
Keep components focused on a single responsibility - split large components
Limit component size to 150-200 lines; extract logic into custom hooks
Use composition over inheritance - prefer smaller, composable components
Export components as named exports for better tree-shaking
One component per file with matching filename and component name
Use useState for simple state management
Use useReducer for complex state logic
Implement proper state initialization
Use proper state updates with callbacks
Implement proper state cleanup
Use Context API for theme/authentication
Implement proper state selectors
Use proper state normalization
Implement proper state persistence
Use React.memo for expensive components
Implement proper useCallback for handlers
Use proper useMemo for expensive computations
Implement proper virtualization for lists
Use proper code splitting with React.lazy
Implement proper cleanup in useEffect
Use proper refs for DOM access
Implement proper event listener cleanup
Use proper abort controllers for fetch
Implement proper subscription cleanup
Use proper HTML elements
Implement proper ARIA attributes
Use proper heading hierarchy
Implement proper form labels
Use proper button types
Implement proper focus management
Use proper keyboard shortcuts
Implement proper tab order
Use proper skip links
Implement proper focus traps
Implement proper error boundaries
Use proper error logging
Implement proper error recovery
Use proper error messages
Implement proper error fallbacks
Use proper form validation
Implement proper error states
Use proper error messages
Implement proper form submission
Use proper form reset
Use proper component naming
Implement proper file naming
Use proper prop naming
Implement proper...
Files:
packages/clerk-js/src/ui/common/withRedirect.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/common/withRedirect.tsx
**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/react.mdc)
**/*.tsx
: Use proper type definitions for props and state
Leverage TypeScript's type inference where possible
Use proper event types for handlers
Implement proper generic types for reusable components
Use proper type guards for conditional rendering
Files:
packages/clerk-js/src/ui/common/withRedirect.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/common/withRedirect.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (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 (2)
packages/clerk-js/src/ui/common/withRedirect.tsx (2)
63-66
: Good switch to single-session signed-in guard and correct URL fallbackUsing
isSignedInAndSingleSessionModeEnabled
with a proper fallback toclerk.buildAfterSignInUrl()
is correct; warning alignment makes sense.
82-85
: AfterSignUp redirect logic looks correctGuard and fallback mirror the sign-in variant appropriately; warning is consistent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
packages/clerk-js/src/core/clerk.ts (1)
1201-1365
: Add tests for pending-session navigation and updatenavigateIfTaskExists
signatureWe’ve verified that
navigateIfTaskExists
is used in multiple places and should be updated to accept and appendredirectUrl
. Please:
- Add unit tests for
setActive
covering:
- Pending sessions: ensure
taskUrls
override vs.navigate
precedence.- Redirect URL preservation for both hash and non-hash URLs.
- Error resilience when
setActive.navigate
throws: Clerk exits transitive state and logs the error.- Update the
navigateIfTaskExists
signature to includeredirectUrl
, then update all call sites:
• packages/clerk-js/src/core/sessionTasks.ts (export at line 32)
• packages/clerk-js/src/core/clerk.ts at lines 1866, 1997, and 2178
• packages/clerk-js/src/ui/components/UserButton/useMultisessionActions.tsx around line 84
♻️ Duplicate comments (2)
packages/clerk-js/src/core/sessionTasks.ts (1)
32-48
: Propagate redirectUrl and declare explicit return type in navigateIfTaskExistsLet callers preserve redirectUrl when navigating to tasks and make the return type explicit.
-export function navigateIfTaskExists( - session: SessionResource, - { - navigate, - baseUrl, - }: { - navigate: (to: string) => Promise<unknown>; - baseUrl: string; - }, -) { +export function navigateIfTaskExists( + session: SessionResource, + { + navigate, + baseUrl, + redirectUrl, + }: { + navigate: (to: string) => Promise<unknown>; + baseUrl: string; + redirectUrl?: string; + }, +): Promise<unknown> | void { const currentTask = session.currentTask; if (!currentTask) { return; } - return navigate(buildTasksUrl(currentTask, { base: baseUrl })); + return navigate(buildTasksUrl(currentTask, { base: baseUrl, redirectUrl })); }packages/clerk-js/src/core/clerk.ts (1)
1329-1332
: Guard against exceptions from consumer-supplied navigate callbackWrap setActiveNavigate to prevent leaving Clerk in an inconsistent state if the app’s callback throws.
- if (setActiveNavigate && newSession) { - await setActiveNavigate({ session: newSession }); - return; - } + if (setActiveNavigate && newSession) { + try { + await setActiveNavigate({ session: newSession }); + } catch (err) { + logger.error('Clerk: setActive.navigate callback failed', err); + } + return; + }
🧹 Nitpick comments (4)
packages/clerk-js/src/core/sessionTasks.ts (2)
9-11
: Tighten the type of INTERNAL_SESSION_TASK_ROUTE_BY_KEY with satisfiesUse satisfies to keep literal keys/values and get compiler help when new tasks are added.
-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', +} satisfies Record<SessionTask['key'], string>;
50-63
: Narrow types for warnMissingPendingTaskHandlers to avoid anyAvoid casting to any; lock the option keys to a readonly tuple.
-export function warnMissingPendingTaskHandlers(options: Record<string, unknown>) { - const taskOptions = ['taskUrls', 'navigate'] as Array< - keyof (Pick<SetActiveParams, 'navigate'> & Pick<ClerkOptions, 'taskUrls'>) - >; +export function warnMissingPendingTaskHandlers(options: Record<string, unknown>) { + const taskOptions = ['taskUrls', 'navigate'] as const satisfies ReadonlyArray< + keyof (Pick<SetActiveParams, 'navigate'> & Pick<ClerkOptions, 'taskUrls'>) + >; - const hasAtLeastOneOption = Object.keys(options).some(option => taskOptions.includes(option as any)); + const hasAtLeastOneOption = Object.keys(options).some( + (option): option is typeof taskOptions[number] => taskOptions.includes(option as any), + ); if (hasAtLeastOneOption) { return; }packages/clerk-js/src/core/clerk.ts (2)
1563-1583
: Add JSDoc for the new public API buildTasksUrlPublic APIs must be documented. Add a concise description and usage info.
/** * Build the URL to the current session task. * If a custom task URL is configured via ClerkProvider.taskUrls, it is returned as-is. * Otherwise, it falls back to the default task route under the sign-in URL. */ public buildTasksUrl(): string {
1675-1681
: Add JSDoc for the new public API redirectToTasksDocument this public redirect helper.
/** * Navigate to the current session task URL if one exists. * No-op when not in a browser environment. */ public redirectToTasks = async (): Promise<unknown> => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
integration/tests/session-tasks-sign-up.test.ts
(2 hunks)packages/clerk-js/src/core/clerk.ts
(21 hunks)packages/clerk-js/src/core/sessionTasks.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- integration/tests/session-tasks-sign-up.test.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{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/core/sessionTasks.ts
packages/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/core/sessionTasks.ts
packages/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/core/sessionTasks.ts
packages/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/core/sessionTasks.ts
packages/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
Avoidany
type - preferunknown
when type is uncertain, then narrow with type guards
Useinterface
for object shapes that might be extended
Usetype
for unions, primitives, and computed types
Preferreadonly
properties for immutable data structures
Useprivate
for internal implementation details
Useprotected
for inheritance hierarchies
Usepublic
explicitly for clarity in public APIs
Preferreadonly
for 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 assertions
for literal types:as const
Usesatisfies
operator 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 ...
Noany
types without justification
Proper error handling with typed errors
Consistent use ofreadonly
for 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/core/sessionTasks.ts
packages/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/core/sessionTasks.ts
packages/clerk-js/src/core/clerk.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/core/sessionTasks.ts
packages/clerk-js/src/core/clerk.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 (1)
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
(1186-1232)packages/types/src/session.ts (2)
SignedInSessionResource
(285-285)SessionResource
(208-261)packages/clerk-js/src/core/sessionTasks.ts (3)
warnMissingPendingTaskHandlers
(50-64)INTERNAL_SESSION_TASK_ROUTE_BY_KEY
(9-11)navigateIfTaskExists
(32-48)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)
⏰ 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
@@ -2028,8 +1993,12 @@ export class Clerk implements ClerkInterface { | |||
return navigateToNextStepSignUp({ missingFields: signUp.missingFields }); | |||
} | |||
|
|||
if (this.__internal_hasAfterAuthFlows) { | |||
return this.__internal_navigateToTaskIfAvailable({ redirectUrlComplete: redirectUrls.getAfterSignInUrl() }); | |||
if (this.session?.currentTask) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't be necessary anymore when we merge #6498 - what's happening atm is that when the sign-up/sign-in are complete, with a pending session, then FAPI is redirecting to sso-callback
, therefore setActive
doesn't run.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we could merge the above first and then rebase against this PR, removing this if statement here.
|
||
await navigateIfTaskExists(session, { | ||
baseUrl: displayConfig.signInUrl, | ||
navigate: this.navigate, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tried sending the customNavigate
here to avoid unmounting the component route when switching from hash routing to path routing, but it handles an absolute path as relative path and breaks the navigation.
I think our current approach is to send a relative path via prop to SignInSsoCallback/SignUpSsoCallback
to pass to the React context navigate
reference.
We could pass taskUrl
as ../tasks
, but it'd lead to a double navigation (although with path routing) -> /sign-in/sso-callback
-> /sign-in/tasks
-> /sign-in/choose-organization
. It's fast tho, so could be better than unmounting the component.
@panteliselef @nikosdouvlis thoughts?
|
||
await __internal_navigateToTaskIfAvailable({ redirectUrlComplete }); | ||
// TODO - Add `onComplete` or `onNextTask` callbacks for `TaskChooseOrganization` standalone usage | ||
if (navigateOnSetActive) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll deprecate redirectUrlComplete
on the next release and introduce the following API:
<TaskChooseOrganization onComplete={(organization) => ...} onNextTask={() ...} />
It's ideal to support more than one instance task and to allow for full control over the callbacks to custom navigation. I won't introduce it now cause I'm mainly focusing on reducing the scope for the after-auth release so we can do so on Monday.
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:
taskUrls
ornavigate
navigate
A callback function that can be provided as an
setActive
option.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 test
runs as expected.pnpm build
runs as expected.Type of change
Summary by CodeRabbit
Summary by CodeRabbit
New Features
navigate
callback to the session activation method to enable flexible navigation control.Bug Fixes
Refactor
RedirectToTask
component renamed toRedirectToTasks
.navigate
callback.Chores