feat(clerk-js,clerk-react,types): Signal errors #6495
Conversation
🦋 Changeset detectedLatest commit: 4d9b742 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 ↗︎
|
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
📝 WalkthroughWalkthroughAdded a changeset for minor releases and an experimental “Signal Errors” note. Tightened SignInFuture catch typings to unknown. Reworked sign-in error handling: signInErrorSignal payload renamed from { errors } to { error }, added errorsToParsedErrors(error) to produce structured Errors (field, raw, global), and signInComputedSignal now reads parsed errors. State now emits { error } to signInErrorSignal. Removed the explicit return type from useClerkSignal. Introduced FieldError/FieldErrors/Errors interfaces and changed signInSignal to return Errors (setter removed). Bumped bundlewatch size thresholds. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (6)
.changeset/happy-dodos-sneeze.md (1)
7-7: Clarify experimental scope in the changeset bodyConsider adding a short one-liner clarifying that Signal Errors are internal-only and subject to change, plus a brief summary of what “error parsing” covers (API errors into field/global categories).
packages/react/src/hooks/useClerkSignal.ts (1)
49-51: Add minimal JSDoc for experimental surfaceMark
useSignInSignalas experimental/internal to set expectations for consumers./** * @experimental Internal API subject to change. * Returns the sign-in signal snapshot (or null on init). */packages/clerk-js/src/core/state.ts (1)
22-25: Correctly forward typed API errors to the signalThis aligns the state with the event payload. Consider adding tests to ensure:
- Non-API errors clear the error signal
- API-shaped errors are parsed into field/global buckets by the computed signal
Would you like me to draft unit tests for these two cases?
packages/clerk-js/src/core/signals.ts (2)
40-41: Shadowed variable name hinders readabilityThe arrow-function parameter
errorshadows the outererrorparameter. Rename one of them (err/apiErr, etc.) to make the control flow clearer and prevent accidental misuse.
22-38: Consider data-driven mapping instead of long switchThe repetitive
switchcan be replaced by a simple lookup table to reduce maintenance overhead and avoid missing future fields:const map: Record<string, keyof Errors['fields']> = { first_name: 'firstName', last_name: 'lastName', email_address: 'emailAddress', identifier: 'identifier', phone_number: 'phoneNumber', password: 'password', username: 'username', code: 'code', captcha: 'captcha', legal_accepted: 'legalAccepted', }; error?.errors.forEach(e => { const key = map[e.meta?.paramName as string]; key ? (parsedErrors.fields[key] = e) : parsedErrors.global.push(e); });Cleaner, faster, and harder to get out of sync with types.
packages/types/src/state.ts (1)
22-26: Prefer precise types overany[]forrawandglobalBoth arrays hold API error objects, so typing them as
any[]discards useful information. Suggest tightening to the actual Clerk error shape:- raw: any[]; - global: any[]; + raw: ClerkAPIResponseError['errors']; + global: ClerkAPIResponseError['errors'];(or a reusable
type APIError = ClerkAPIResponseError['errors'][number]).Improves IntelliSense and guards against accidental misuse.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.changeset/happy-dodos-sneeze.md(1 hunks)packages/clerk-js/src/core/events.ts(2 hunks)packages/clerk-js/src/core/resources/SignIn.ts(4 hunks)packages/clerk-js/src/core/signals.ts(1 hunks)packages/clerk-js/src/core/state.ts(2 hunks)packages/react/src/hooks/useClerkSignal.ts(1 hunks)packages/types/src/state.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
.changeset/**
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Automated releases must use Changesets.
Files:
.changeset/happy-dodos-sneeze.md
**/*.{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/resources/SignIn.tspackages/clerk-js/src/core/events.tspackages/react/src/hooks/useClerkSignal.tspackages/clerk-js/src/core/state.tspackages/clerk-js/src/core/signals.tspackages/types/src/state.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/resources/SignIn.tspackages/clerk-js/src/core/events.tspackages/react/src/hooks/useClerkSignal.tspackages/clerk-js/src/core/state.tspackages/clerk-js/src/core/signals.tspackages/types/src/state.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/core/resources/SignIn.tspackages/clerk-js/src/core/events.tspackages/react/src/hooks/useClerkSignal.tspackages/clerk-js/src/core/state.tspackages/clerk-js/src/core/signals.tspackages/types/src/state.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/resources/SignIn.tspackages/clerk-js/src/core/events.tspackages/react/src/hooks/useClerkSignal.tspackages/clerk-js/src/core/state.tspackages/clerk-js/src/core/signals.tspackages/types/src/state.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/core/resources/SignIn.tspackages/clerk-js/src/core/events.tspackages/react/src/hooks/useClerkSignal.tspackages/clerk-js/src/core/state.tspackages/clerk-js/src/core/signals.tspackages/types/src/state.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/resources/SignIn.tspackages/clerk-js/src/core/events.tspackages/react/src/hooks/useClerkSignal.tspackages/clerk-js/src/core/state.tspackages/clerk-js/src/core/signals.tspackages/types/src/state.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/resources/SignIn.tspackages/clerk-js/src/core/events.tspackages/react/src/hooks/useClerkSignal.tspackages/clerk-js/src/core/state.tspackages/clerk-js/src/core/signals.tspackages/types/src/state.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). (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 (4)
.changeset/happy-dodos-sneeze.md (1)
1-5: Appropriate changeset scope and bump levelsMinor bumps across JS, React, and Types make sense given additive typing and internal signal changes.
packages/clerk-js/src/core/events.ts (1)
17-17: Type the error payload as ClerkAPIResponseError | null — LGTMThe typed payload will help downstream consumers and aligns with the parsing work.
packages/clerk-js/src/core/resources/SignIn.ts (1)
15-15: Importing ClerkAPIResponseError for error typing — OKThis aligns with the event payload typing across the codebase.
packages/clerk-js/src/core/state.ts (1)
1-1: Type-only import for ClerkAPIResponseError — LGTMKeeps runtime clean and matches the new payload typing.
packages/clerk-js/src/core/events.ts
Outdated
| @@ -1,5 +1,5 @@ | |||
| import { createEventBus } from '@clerk/shared/eventBus'; | |||
| import type { TokenResource } from '@clerk/types'; | |||
| import type { ClerkAPIResponseError, TokenResource } from '@clerk/types'; | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify all emitters conform to the new payload
Now that ResourceErrorPayload.error is ClerkAPIResponseError | null, ensure all eventBus.emit('resource:error', …) sites either pass a properly typed API error or null (for non-API errors).
Run this to find non-conforming emitters:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "All resource:error emit sites:"
rg -n "eventBus\\.emit\\('resource:error'" -A 3 -B 1
echo
echo "Emitters that don't cast/guard to ClerkAPIResponseError or null:"
rg -n "eventBus\\.emit\\('resource:error', \\{[^}]*error: (?!null)(?!\\(.*ClerkAPIResponseError\\)).*\\}" -U || trueLength of output: 5359
Ensure all resource:error emits use ClerkAPIResponseError | null
A couple of eventBus.emit('resource:error', …) calls are still passing the raw err (type unknown) instead of casting it to ClerkAPIResponseError. Please update them to match the new payload type:
• packages/clerk-js/src/core/resources/SignIn.ts
- Line 522
- Line 535
Change each instance from:
eventBus.emit('resource:error', { resource: this.resource, error: err });to:
eventBus.emit('resource:error', { resource: this.resource, error: err as ClerkAPIResponseError });This ensures ResourceErrorPayload.error is always ClerkAPIResponseError or null.
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/resources/SignIn.ts at lines 522 and 535, the
eventBus.emit calls for 'resource:error' are passing the error as type unknown
instead of ClerkAPIResponseError. Update these calls by casting the error to
ClerkAPIResponseError using 'err as ClerkAPIResponseError' to ensure the emitted
payload matches the expected type ClerkAPIResponseError | null.
| } catch (err: unknown) { | ||
| eventBus.emit('resource:error', { resource: this.resource, error: err }); | ||
| eventBus.emit('resource:error', { resource: this.resource, error: err as ClerkAPIResponseError }); | ||
| return { error: err }; | ||
| } |
There was a problem hiding this comment.
Avoid unsound cast; guard before emitting API error
err here can be a locally thrown Error (e.g., “Email code factor not found”). Blindly casting to ClerkAPIResponseError risks runtime shape errors down the line. Guard, and emit null for non-API errors.
Apply:
- eventBus.emit('resource:error', { resource: this.resource, error: err as ClerkAPIResponseError });
+ const apiErr = isClerkAPIResponseError(err) ? err : null;
+ eventBus.emit('resource:error', { resource: this.resource, error: apiErr });Add this helper (once per module or in a shared util):
// Type guard for API errors
function isClerkAPIResponseError(err: unknown): err is ClerkAPIResponseError {
return !!err && typeof err === 'object' && Array.isArray((err as any).errors);
}🤖 Prompt for AI Agents
In packages/clerk-js/src/core/resources/SignIn.ts around lines 560 to 563, the
error object `err` is unsafely cast to `ClerkAPIResponseError` without checking
its type, which can cause runtime errors. To fix this, implement a type guard
function `isClerkAPIResponseError` that checks if `err` has the expected shape
(e.g., an `errors` array). Use this guard before emitting the error event: emit
the error only if it passes the guard, otherwise emit null or handle
accordingly. This prevents unsafe casts and ensures only valid API errors are
emitted.
| } catch (err: unknown) { | ||
| eventBus.emit('resource:error', { resource: this.resource, error: err }); | ||
| eventBus.emit('resource:error', { resource: this.resource, error: err as ClerkAPIResponseError }); | ||
| return { error: err }; | ||
| } |
There was a problem hiding this comment.
Same guard needed here
Don’t cast arbitrary errors to ClerkAPIResponseError; guard and emit null when not API-shaped.
- eventBus.emit('resource:error', { resource: this.resource, error: err as ClerkAPIResponseError });
+ const apiErr = isClerkAPIResponseError(err) ? err : null;
+ eventBus.emit('resource:error', { resource: this.resource, error: apiErr });📝 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.
| } catch (err: unknown) { | |
| eventBus.emit('resource:error', { resource: this.resource, error: err }); | |
| eventBus.emit('resource:error', { resource: this.resource, error: err as ClerkAPIResponseError }); | |
| return { error: err }; | |
| } | |
| } catch (err: unknown) { | |
| const apiErr = isClerkAPIResponseError(err) ? err : null; | |
| eventBus.emit('resource:error', { resource: this.resource, error: apiErr }); | |
| return { error: err }; | |
| } |
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/resources/SignIn.ts around lines 575 to 578, the
catch block casts any caught error to ClerkAPIResponseError without verifying
its shape. To fix this, add a type guard to check if the error matches the
ClerkAPIResponseError structure before emitting it; if it does not, emit null
instead. Also, update the returned error accordingly to avoid unsafe casting.
| } catch (err: unknown) { | ||
| eventBus.emit('resource:error', { resource: this.resource, error: err }); | ||
| eventBus.emit('resource:error', { resource: this.resource, error: err as ClerkAPIResponseError }); | ||
| return { error: err }; |
There was a problem hiding this comment.
Guard before emitting in SSO flow
This path throws local errors (e.g., “modal flow is not supported yet”). Use the same guard to avoid mis-typing the signal.
- eventBus.emit('resource:error', { resource: this.resource, error: err as ClerkAPIResponseError });
+ const apiErr = isClerkAPIResponseError(err) ? err : null;
+ eventBus.emit('resource:error', { resource: this.resource, error: apiErr });📝 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.
| } catch (err: unknown) { | |
| eventBus.emit('resource:error', { resource: this.resource, error: err }); | |
| eventBus.emit('resource:error', { resource: this.resource, error: err as ClerkAPIResponseError }); | |
| return { error: err }; | |
| } catch (err: unknown) { | |
| const apiErr = isClerkAPIResponseError(err) ? err : null; | |
| eventBus.emit('resource:error', { resource: this.resource, error: apiErr }); | |
| return { error: err }; |
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/resources/SignIn.ts around lines 615 to 617, the
catch block emits an error event without checking if the current flow supports
it, which can cause issues with local errors like "modal flow is not supported
yet." Add a guard condition before emitting the 'resource:error' event to ensure
it only emits for supported flows, preventing mis-typing or inappropriate signal
emission.
| import { useCallback, useSyncExternalStore } from 'react'; | ||
|
|
||
| import { useIsomorphicClerkContext } from '../contexts/IsomorphicClerkContext'; | ||
| import { useAssertWrappedByClerkProvider } from './useAssertWrappedByClerkProvider'; | ||
|
|
||
| function useClerkSignal(signal: 'signIn'): { errors: unknown; signIn: SignInFutureResource | null } | null { | ||
| function useClerkSignal(signal: 'signIn') { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Restore explicit return types for public APIs
Per guidelines, exported hooks should have explicit return types. You can capture the core’s type without duplicating shapes via ReturnType of the State['signInSignal'].
+import type { State as ClerkState } from '@clerk/types';
-function useClerkSignal(signal: 'signIn') {
+function useClerkSignal(signal: 'signIn'): ReturnType<ClerkState['signInSignal']> | null {
…
}
-export function useSignInSignal() {
+export function useSignInSignal(): ReturnType<ClerkState['signInSignal']> | null {
return useClerkSignal('signIn');
}📝 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.
| function useClerkSignal(signal: 'signIn') { | |
| // Add the type import | |
| import type { State as ClerkState } from '@clerk/types'; | |
| function useClerkSignal( | |
| signal: 'signIn' | |
| ): ReturnType<ClerkState['signInSignal']> | null { | |
| … | |
| } | |
| export function useSignInSignal(): ReturnType<ClerkState['signInSignal']> | null { | |
| return useClerkSignal('signIn'); | |
| } |
🤖 Prompt for AI Agents
In packages/react/src/hooks/useClerkSignal.ts at line 6, the exported hook
useClerkSignal lacks an explicit return type. Fix this by adding an explicit
return type using ReturnType<typeof State['signInSignal']> to capture the core
type without duplicating shapes, ensuring the public API follows the guidelines.
dstaley
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/clerk-js/src/core/signals.ts (1)
49-80: Add default case for unknown field namesWhile the else block handles errors without
paramName, the switch statement should have a default case to handle unknownparamNamevalues explicitly, preventing silent error drops.switch (error.meta.paramName) { case 'first_name': parsedErrors.fields.firstName = error; break; case 'last_name': parsedErrors.fields.lastName = error; break; case 'email_address': parsedErrors.fields.emailAddress = error; break; case 'identifier': parsedErrors.fields.identifier = error; break; case 'phone_number': parsedErrors.fields.phoneNumber = error; break; case 'password': parsedErrors.fields.password = error; break; case 'username': parsedErrors.fields.username = error; break; case 'code': parsedErrors.fields.code = error; break; case 'captcha': parsedErrors.fields.captcha = error; break; case 'legal_accepted': parsedErrors.fields.legalAccepted = error; break; + default: + // Unknown field names go to global errors + parsedErrors.global.push(error); + break; }
🧹 Nitpick comments (2)
packages/types/src/state.ts (1)
25-25: Minor grammatical improvement for clarityThe comment could be slightly clearer.
- global: unknown[]; // does not include any errors that could be parsed as a field error + global: unknown[]; // excludes errors that could be parsed as field errorspackages/clerk-js/src/core/signals.ts (1)
47-47: Variable shadowing could cause confusionThe
errorparameter in the forEach callback shadows the outererrorparameter, which could lead to confusion.- error.errors.forEach(error => { + error.errors.forEach(err => { - if ('meta' in error && error.meta && 'paramName' in error.meta) { + if ('meta' in err && err.meta && 'paramName' in err.meta) { - switch (error.meta.paramName) { + switch (err.meta.paramName) { case 'first_name': - parsedErrors.fields.firstName = error; + parsedErrors.fields.firstName = err; break; case 'last_name': - parsedErrors.fields.lastName = error; + parsedErrors.fields.lastName = err; break; case 'email_address': - parsedErrors.fields.emailAddress = error; + parsedErrors.fields.emailAddress = err; break; case 'identifier': - parsedErrors.fields.identifier = error; + parsedErrors.fields.identifier = err; break; case 'phone_number': - parsedErrors.fields.phoneNumber = error; + parsedErrors.fields.phoneNumber = err; break; case 'password': - parsedErrors.fields.password = error; + parsedErrors.fields.password = err; break; case 'username': - parsedErrors.fields.username = error; + parsedErrors.fields.username = err; break; case 'code': - parsedErrors.fields.code = error; + parsedErrors.fields.code = err; break; case 'captcha': - parsedErrors.fields.captcha = error; + parsedErrors.fields.captcha = err; break; case 'legal_accepted': - parsedErrors.fields.legalAccepted = error; + parsedErrors.fields.legalAccepted = err; break; } } else { - parsedErrors.global.push(error); + parsedErrors.global.push(err); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/clerk-js/bundlewatch.config.json(1 hunks)packages/clerk-js/src/core/resources/SignIn.ts(2 hunks)packages/clerk-js/src/core/signals.ts(1 hunks)packages/clerk-js/src/core/state.ts(1 hunks)packages/types/src/state.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/clerk-js/src/core/resources/SignIn.ts
- packages/clerk-js/src/core/state.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{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/bundlewatch.config.jsonpackages/types/src/state.tspackages/clerk-js/src/core/signals.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/bundlewatch.config.jsonpackages/types/src/state.tspackages/clerk-js/src/core/signals.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/types/src/state.tspackages/clerk-js/src/core/signals.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/types/src/state.tspackages/clerk-js/src/core/signals.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/types/src/state.tspackages/clerk-js/src/core/signals.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/types/src/state.tspackages/clerk-js/src/core/signals.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/types/src/state.tspackages/clerk-js/src/core/signals.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). (4)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
🔇 Additional comments (4)
packages/clerk-js/bundlewatch.config.json (1)
3-3: Bundle size increases are reasonable for the added error parsing functionalityThe ~1KB increase in bundle sizes is justified by the new error parsing and structured error handling capabilities being added. These small increases are acceptable for the value provided by improved error handling.
Also applies to: 6-6
packages/types/src/state.ts (1)
3-26: Well-structured error type definitions with proper typingThe new error interfaces provide excellent type safety for error handling. The use of
unknown[]instead ofany[]forrawandglobalarrays follows TypeScript best practices.packages/clerk-js/src/core/signals.ts (2)
1-8: Proper imports and signal structure updatesThe new imports and signal payload change from
errorstoerrorproperly align with the single error handling pattern.
10-21: Clean integration of error parsing in computed signalThe computed signal properly integrates the new error parsing function and maintains backwards compatibility by returning the same structure.
| } | ||
|
|
||
| return { errors, signIn: signIn.__internal_future }; | ||
| }); | ||
|
|
||
| export function errorsToParsedErrors(error: unknown): Errors { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add JSDoc documentation for exported function
Per the coding guidelines, all public APIs must be documented with JSDoc.
+/**
+ * Parses an error into a structured Errors object with field-specific and global errors.
+ * @param error - The error to parse, typically from a Clerk API response
+ * @returns A structured Errors object with parsed field errors and global errors
+ */
export function errorsToParsedErrors(error: unknown): Errors {📝 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 errorsToParsedErrors(error: unknown): Errors { | |
| /** | |
| * Parses an error into a structured Errors object with field-specific and global errors. | |
| * @param error - The error to parse, typically from a Clerk API response | |
| * @returns A structured Errors object with parsed field errors and global errors | |
| */ | |
| export function errorsToParsedErrors(error: unknown): Errors { |
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/signals.ts around line 23, the exported function
errorsToParsedErrors is missing JSDoc documentation; add a JSDoc block
immediately above the function that describes the function purpose, its
parameters (type and meaning for the unknown error input), return type (Errors)
and any thrown errors or notes about accepted input shapes, include examples or
behavior for common inputs if relevant, and follow the repo JSDoc style (summary
line, @param, @returns, and any @throws or @example tags as appropriate).
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/clerk-js/src/core/signals.ts (1)
27-27: Consider exportingerrorsToParsedErrorsfor reusabilityThe
errorsToParsedErrorsfunction provides valuable error parsing functionality that could be useful in other parts of the codebase. Since it follows the pattern of converting raw errors into a structured format, consider exporting it to enable reuse in other error handling scenarios.-function errorsToParsedErrors(error: unknown): Errors { +export function errorsToParsedErrors(error: unknown): Errors {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/clerk-js/src/core/signals.ts(1 hunks)
🧰 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/signals.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/signals.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/core/signals.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/signals.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/core/signals.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/signals.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/signals.ts
🧬 Code Graph Analysis (1)
packages/clerk-js/src/core/signals.ts (2)
packages/clerk-js/src/core/resources/SignIn.ts (1)
SignIn(74-492)packages/types/src/state.ts (1)
Errors(22-26)
⏰ 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 (vue, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (tanstack-react-router, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (quickstart, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Unit Tests (18, --filter=@clerk/astro --filter=@clerk/backend --filter=@clerk/express --filter=@c...
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Static analysis
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
packages/clerk-js/src/core/signals.ts (1)
45-48: Clarify the duplication of non-API errors in bothrawandglobalarraysWhen the error is not a Clerk API response error, it's being pushed to both
rawandglobalarrays. This duplication might be intentional, but it's worth confirming whether both arrays should contain the same error object.Consider if this duplication is necessary or if non-API errors should only go to one of these arrays to avoid redundancy.
| function errorsToParsedErrors(error: unknown): Errors { | ||
| const parsedErrors: Errors = { | ||
| fields: { | ||
| firstName: null, | ||
| lastName: null, | ||
| emailAddress: null, | ||
| identifier: null, | ||
| phoneNumber: null, | ||
| password: null, | ||
| username: null, | ||
| code: null, | ||
| captcha: null, | ||
| legalAccepted: null, | ||
| }, | ||
| raw: [], | ||
| global: [], | ||
| }; | ||
|
|
||
| if (!isClerkAPIResponseError(error)) { | ||
| parsedErrors.raw.push(error); | ||
| parsedErrors.global.push(error); | ||
| return parsedErrors; | ||
| } | ||
|
|
||
| parsedErrors.raw.push(...error.errors); | ||
|
|
||
| error.errors.forEach(error => { | ||
| if ('meta' in error && error.meta && 'paramName' in error.meta) { | ||
| switch (error.meta.paramName) { | ||
| case 'first_name': | ||
| parsedErrors.fields.firstName = error; | ||
| break; | ||
| case 'last_name': | ||
| parsedErrors.fields.lastName = error; | ||
| break; | ||
| case 'email_address': | ||
| parsedErrors.fields.emailAddress = error; | ||
| break; | ||
| case 'identifier': | ||
| parsedErrors.fields.identifier = error; | ||
| break; | ||
| case 'phone_number': | ||
| parsedErrors.fields.phoneNumber = error; | ||
| break; | ||
| case 'password': | ||
| parsedErrors.fields.password = error; | ||
| break; | ||
| case 'username': | ||
| parsedErrors.fields.username = error; | ||
| break; | ||
| case 'code': | ||
| parsedErrors.fields.code = error; | ||
| break; | ||
| case 'captcha': | ||
| parsedErrors.fields.captcha = error; | ||
| break; | ||
| case 'legal_accepted': | ||
| parsedErrors.fields.legalAccepted = error; | ||
| break; | ||
| default: | ||
| parsedErrors.global.push(error); | ||
| break; | ||
| } | ||
| } else { | ||
| parsedErrors.global.push(error); | ||
| } | ||
| }); | ||
|
|
||
| return parsedErrors; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add unit tests for the new error parsing functionality
The errorsToParsedErrors function introduces complex error parsing logic with multiple branches and field mappings. Please add comprehensive unit tests to cover:
- Non-Clerk API errors
- Clerk API errors with various
paramNamevalues - Errors without
metaorparamNameproperties - Multiple errors for the same field
- Unknown
paramNamevalues hitting the default case
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/signals.ts around lines 27 to 96, add
comprehensive unit tests for errorsToParsedErrors to validate its branching and
field mapping logic: create test cases for (1) non-Clerk errors (ensure raw and
global contain the original error), (2) Clerk API errors with each supported
paramName (first_name, last_name, email_address, identifier, phone_number,
password, username, code, captcha, legal_accepted) mapping to the correct
fields, (3) errors missing meta or paramName (expect them in global), (4)
multiple errors for the same field (ensure the last/expected assignment or
accumulation behavior asserted), and (5) unknown paramName values hitting the
default case (assigned to global); implement assertions on parsedErrors.fields,
parsedErrors.raw, and parsedErrors.global for each case.
| error.errors.forEach(error => { | ||
| if ('meta' in error && error.meta && 'paramName' in error.meta) { | ||
| switch (error.meta.paramName) { | ||
| case 'first_name': | ||
| parsedErrors.fields.firstName = error; | ||
| break; | ||
| case 'last_name': | ||
| parsedErrors.fields.lastName = error; | ||
| break; | ||
| case 'email_address': | ||
| parsedErrors.fields.emailAddress = error; | ||
| break; | ||
| case 'identifier': | ||
| parsedErrors.fields.identifier = error; | ||
| break; | ||
| case 'phone_number': | ||
| parsedErrors.fields.phoneNumber = error; | ||
| break; | ||
| case 'password': | ||
| parsedErrors.fields.password = error; | ||
| break; | ||
| case 'username': | ||
| parsedErrors.fields.username = error; | ||
| break; | ||
| case 'code': | ||
| parsedErrors.fields.code = error; | ||
| break; | ||
| case 'captcha': | ||
| parsedErrors.fields.captcha = error; | ||
| break; | ||
| case 'legal_accepted': | ||
| parsedErrors.fields.legalAccepted = error; | ||
| break; | ||
| default: | ||
| parsedErrors.global.push(error); | ||
| break; | ||
| } |
There was a problem hiding this comment.
Field errors may be overwritten when multiple errors exist for the same field
The current implementation assigns errors directly to field properties (e.g., parsedErrors.fields.firstName = error). If the API returns multiple errors for the same field, only the last one will be retained, potentially losing important validation information.
Consider either:
- Changing field properties to arrays to store multiple errors per field
- Documenting this limitation if it's intentional
- Logging a warning when overwriting occurs
Example of potential data loss:
// If API returns two errors for 'first_name':
// Error 1: "First name is required"
// Error 2: "First name contains invalid characters"
// Only Error 2 would be retained in parsedErrors.fields.firstName🤖 Prompt for AI Agents
In packages/clerk-js/src/core/signals.ts around lines 53 to 89, the code assigns
a single error object to each field which overwrites earlier errors for the same
field; change each field property to hold an array of errors (or, if the
surrounding types must remain, convert to an array when a second error appears)
and push errors instead of assigning so all errors are preserved; update the
parsedErrors initialization and related types/interfaces to use arrays for
fields (or handle conversion on conflict), and ensure downstream callers
expect/handle arrays (or convert them back if necessary).
Description
This PR adds error parsing to our Signals implementation, which is still internal, and not intended for public consumption (yet!).
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Chores