refactor(clerk-js): Replace internal error module with shared/errors #6841
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 5010746 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types 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 |
WalkthroughRefactors error imports/exports to source error types and guards from Changes
Sequence Diagram(s)No sequence diagram generated — control flow and runtime behavior are unchanged (only import/export refactors). Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (11)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (8)
🧰 Additional context used📓 Path-based instructions (9)**/*.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
packages/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
packages/**/*.{ts,tsx,d.ts}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
packages/**/*.{test,spec}.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Files:
packages/{clerk-js,elements,themes}/**/*.{test,spec}.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Files:
**/*.{js,ts,tsx,jsx}📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Files:
**/__tests__/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
Files:
⏰ 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). (23)
🔇 Additional comments (2)
Comment |
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.changeset/mighty-frogs-brake.md (1)
1-3: Empty/invalid changeset — add package bumps and summaryFile .changeset/mighty-frogs-brake.md contains only front‑matter ("---") with no package entries or human‑readable summary; release automation will ignore or fail this changeset. Replace it with a valid changeset that lists affected package(s) and bump types and includes a short summary. Confirm package names and bump levels (patch for internal fixes, minor for additions, major for removals/BC breaks) before committing.
🧹 Nitpick comments (6)
packages/clerk-js/src/utils/errors.ts (1)
4-6: Ensure boolean return regardless oferrorspresenceSafer and simpler boolean by using optional chaining +
some.-export function isError(err: ClerkAPIResponseError, code = ''): boolean { - return err.errors && !!err.errors.find((e: ClerkAPIError) => e.code === code); -} +export function isError(err: ClerkAPIResponseError, code = ''): boolean { + return !!err.errors?.some((e: ClerkAPIError) => e.code === code); +}packages/clerk-js/src/core/resources/BillingCheckout.ts (2)
74-86: Preferunknownfor error parameter and narrow via type guardAvoid
any; the guard already narrows.- shouldRetry(error: any, iterations: number) { + shouldRetry(error: unknown, iterations: number) { if (!isClerkAPIResponseError(error) || iterations >= 4) { return false; }
58-61: Doc/config mismatch: retry delaysComment says retries at 2s, 4s, 6s, 8s, but
maxDelayBetweenRetries: 2_000caps delay at 2s. Align the comment (preferred in this refactor-only PR) or adjust the cap in a follow-up.Also applies to: 70-73
packages/clerk-js/src/core/resources/SignUp.ts (1)
156-163: Provide a meaningful error message for captcha_unavailableEmpty error messages reduce debuggability. Add a concise developer-facing message.
- if (!captchaParams) { - throw new ClerkRuntimeError('', { code: 'captcha_unavailable' }); - } + if (!captchaParams) { + throw new ClerkRuntimeError('Captcha challenge unavailable', { code: 'captcha_unavailable' }); + }packages/clerk-js/src/index.ts (1)
6-19: Centralized error re‑exports look good; consider a type‑only export for the MetamaskError type.This aligns the public API with the shared error module and improves tree‑shaking. Optional nit: export the type with a separate type‑only statement.
Apply this diff if you prefer a type‑only export:
export { ClerkAPIResponseError, ClerkRuntimeError, EmailLinkError, EmailLinkErrorCode, EmailLinkErrorCodeStatus, isClerkAPIResponseError, isClerkRuntimeError, isEmailLinkError, isKnownError, isMetamaskError, isUserLockedError, - type MetamaskError, } from '@clerk/shared/error'; +export type { MetamaskError } from '@clerk/shared/error';packages/clerk-js/src/index.headless.ts (1)
5-18: Headless index re‑exports match main index; consider type‑only export for MetamaskError.Parity with the main entry is good. Optional nit identical to index.ts:
Apply:
export { ClerkAPIResponseError, ClerkRuntimeError, EmailLinkError, EmailLinkErrorCode, EmailLinkErrorCodeStatus, isClerkAPIResponseError, isClerkRuntimeError, isEmailLinkError, isKnownError, isMetamaskError, isUserLockedError, - type MetamaskError, } from '@clerk/shared/error'; +export type { MetamaskError } from '@clerk/shared/error';
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (14)
.changeset/mighty-frogs-brake.md(1 hunks)packages/clerk-js/src/core/clerk.ts(2 hunks)packages/clerk-js/src/core/fraudProtection.ts(1 hunks)packages/clerk-js/src/core/modules/apiKeys/index.ts(2 hunks)packages/clerk-js/src/core/resources/Base.ts(1 hunks)packages/clerk-js/src/core/resources/BillingCheckout.ts(2 hunks)packages/clerk-js/src/core/resources/Error.ts(0 hunks)packages/clerk-js/src/core/resources/SignUp.ts(2 hunks)packages/clerk-js/src/core/resources/index.ts(1 hunks)packages/clerk-js/src/core/resources/internal.ts(1 hunks)packages/clerk-js/src/index.headless.ts(1 hunks)packages/clerk-js/src/index.ts(1 hunks)packages/clerk-js/src/ui/utils/errorHandler.ts(1 hunks)packages/clerk-js/src/utils/errors.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/clerk-js/src/core/resources/Error.ts
🧰 Additional context used
📓 Path-based instructions (10)
.changeset/**
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Automated releases must use Changesets.
Files:
.changeset/mighty-frogs-brake.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/modules/apiKeys/index.tspackages/clerk-js/src/core/fraudProtection.tspackages/clerk-js/src/ui/utils/errorHandler.tspackages/clerk-js/src/core/resources/SignUp.tspackages/clerk-js/src/index.headless.tspackages/clerk-js/src/index.tspackages/clerk-js/src/core/clerk.tspackages/clerk-js/src/core/resources/Base.tspackages/clerk-js/src/core/resources/index.tspackages/clerk-js/src/utils/errors.tspackages/clerk-js/src/core/resources/BillingCheckout.tspackages/clerk-js/src/core/resources/internal.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/modules/apiKeys/index.tspackages/clerk-js/src/core/fraudProtection.tspackages/clerk-js/src/ui/utils/errorHandler.tspackages/clerk-js/src/core/resources/SignUp.tspackages/clerk-js/src/index.headless.tspackages/clerk-js/src/index.tspackages/clerk-js/src/core/clerk.tspackages/clerk-js/src/core/resources/Base.tspackages/clerk-js/src/core/resources/index.tspackages/clerk-js/src/utils/errors.tspackages/clerk-js/src/core/resources/BillingCheckout.tspackages/clerk-js/src/core/resources/internal.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/core/modules/apiKeys/index.tspackages/clerk-js/src/core/fraudProtection.tspackages/clerk-js/src/ui/utils/errorHandler.tspackages/clerk-js/src/core/resources/SignUp.tspackages/clerk-js/src/index.headless.tspackages/clerk-js/src/index.tspackages/clerk-js/src/core/clerk.tspackages/clerk-js/src/core/resources/Base.tspackages/clerk-js/src/core/resources/index.tspackages/clerk-js/src/utils/errors.tspackages/clerk-js/src/core/resources/BillingCheckout.tspackages/clerk-js/src/core/resources/internal.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/modules/apiKeys/index.tspackages/clerk-js/src/core/fraudProtection.tspackages/clerk-js/src/ui/utils/errorHandler.tspackages/clerk-js/src/core/resources/SignUp.tspackages/clerk-js/src/index.headless.tspackages/clerk-js/src/index.tspackages/clerk-js/src/core/clerk.tspackages/clerk-js/src/core/resources/Base.tspackages/clerk-js/src/core/resources/index.tspackages/clerk-js/src/utils/errors.tspackages/clerk-js/src/core/resources/BillingCheckout.tspackages/clerk-js/src/core/resources/internal.ts
packages/**/index.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use tree-shaking friendly exports
Files:
packages/clerk-js/src/core/modules/apiKeys/index.tspackages/clerk-js/src/index.tspackages/clerk-js/src/core/resources/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/clerk-js/src/core/modules/apiKeys/index.tspackages/clerk-js/src/core/fraudProtection.tspackages/clerk-js/src/ui/utils/errorHandler.tspackages/clerk-js/src/core/resources/SignUp.tspackages/clerk-js/src/index.headless.tspackages/clerk-js/src/index.tspackages/clerk-js/src/core/clerk.tspackages/clerk-js/src/core/resources/Base.tspackages/clerk-js/src/core/resources/index.tspackages/clerk-js/src/utils/errors.tspackages/clerk-js/src/core/resources/BillingCheckout.tspackages/clerk-js/src/core/resources/internal.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/modules/apiKeys/index.tspackages/clerk-js/src/core/fraudProtection.tspackages/clerk-js/src/ui/utils/errorHandler.tspackages/clerk-js/src/core/resources/SignUp.tspackages/clerk-js/src/index.headless.tspackages/clerk-js/src/index.tspackages/clerk-js/src/core/clerk.tspackages/clerk-js/src/core/resources/Base.tspackages/clerk-js/src/core/resources/index.tspackages/clerk-js/src/utils/errors.tspackages/clerk-js/src/core/resources/BillingCheckout.tspackages/clerk-js/src/core/resources/internal.ts
**/index.ts
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
Use index.ts files for clean imports but avoid deep barrel exports
Avoid barrel files (index.ts re-exports) as they can cause circular dependencies
Files:
packages/clerk-js/src/core/modules/apiKeys/index.tspackages/clerk-js/src/index.tspackages/clerk-js/src/core/resources/index.ts
packages/clerk-js/src/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/clerk-js-ui.mdc)
packages/clerk-js/src/ui/**/*.{ts,tsx}: Element descriptors should always be camelCase
Use element descriptors in UI components to enable consistent theming and styling via appearance.elements
Element descriptors should generate unique, stable CSS classes for theming
Element descriptors should handle state classes (e.g., cl-loading, cl-active, cl-error, cl-open) automatically based on component state
Do not render hard-coded values; all user-facing strings must be localized using provided localization methods
Use the useLocalizations hook and localizationKeys utility for all text and error messages
Use the styled system (sx prop, theme tokens, responsive values) for custom component styling
Use useCardState for card-level state, useFormState for form-level state, and useLoadingStatus for loading states
Always use handleError utility for API errors and use translateError for localized error messages
Use useFormControl for form field state, implement proper validation, and handle loading and error states in forms
Use localization keys for all form labels and placeholders
Use element descriptors for consistent styling and follow the theme token system
Use the Card and FormContainer patterns for consistent UI structure
Files:
packages/clerk-js/src/ui/utils/errorHandler.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: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (10)
packages/clerk-js/src/utils/errors.ts (1)
1-1: Import path update looks goodCentralizing the error type to @clerk/shared/error aligns with the PR goal.
packages/clerk-js/src/ui/utils/errorHandler.ts (1)
1-1: Import path update looks goodSwitching guards to @clerk/shared/error is correct and matches the refactor.
packages/clerk-js/src/core/modules/apiKeys/index.ts (1)
1-1: Import path update looks goodUsing ClerkRuntimeError from @clerk/shared/error is consistent with the new source of truth.
packages/clerk-js/src/core/resources/BillingCheckout.ts (1)
1-1: Import path update looks goodGuard sourced from @clerk/shared/error is correct.
packages/clerk-js/src/core/resources/SignUp.ts (1)
1-1: Import path update looks goodClerkRuntimeError/isCaptchaError/isClerkAPIResponseError now sourced from @clerk/shared/error as intended.
packages/clerk-js/src/core/resources/Base.ts (1)
2-2: Import path update looks goodCentralizing ClerkAPIResponseError/ClerkRuntimeError to @clerk/shared/error is consistent across the PR.
packages/clerk-js/src/core/fraudProtection.ts (1)
1-1: Import path updates look goodUsing ClerkRuntimeError/isClerkAPIResponseError from @clerk/shared/error and limiting internal import to Client is correct.
Also applies to: 5-5
packages/clerk-js/src/index.ts (1)
20-20: Named export for Clerk is appropriate and tree‑shaking friendly.No issues.
packages/clerk-js/src/core/clerk.ts (1)
159-159: Barrel import narrowed to resources only (no Error).Good separation; reduces the chance of cycles via error exports.
packages/clerk-js/src/core/resources/internal.ts (1)
2-45: Barrel adds multiple resource exports; Error is correctly excluded.
Confirmed: no core/resources/Error.* file or re-export found; omission is correct. Optional: prefer explicit named exports over export * to limit accidental surface growth and reduce circular-dependency risk.
| import { | ||
| ClerkRuntimeError, | ||
| EmailLinkError, | ||
| EmailLinkErrorCodeStatus, | ||
| is4xxError, | ||
| isClerkAPIResponseError, | ||
| isClerkRuntimeError, | ||
| } from '@clerk/shared/error'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
Import migration to @clerk/shared/error looks correct; verify all symbols exist (esp. is4xxError).
This decouples core from internal Error resources. Please double‑check no lingering imports remain from core/resources/Error and that @clerk/shared/error exports all referenced symbols.
Run:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Any lingering deep imports to core/resources/Error?"
rg -nP -C2 "['\"][^'\"]*core/resources/Error['\"]" --type=ts --type=tsx || true
echo
echo "2) Any error symbols still imported from resources/internal?"
rg -nP --type=ts --type=tsx -C2 "from\s+['\"][^'\"]*resources/internal['\"]" | rg -nP "(ClerkRuntimeError|ClerkAPIResponseError|EmailLinkError|EmailLinkErrorCode|EmailLinkErrorCodeStatus|isClerkAPIResponseError|isClerkRuntimeError|isEmailLinkError|isKnownError|isMetamaskError|isUserLockedError|MetamaskError)" || true
echo
echo "3) Confirm shared/error actually exports the migrated symbols"
rg -nP -C2 "export\s+(?:type\s+)?(?:class|function|const|let|var|interface|type)?\s*(ClerkRuntimeError|ClerkAPIResponseError|EmailLinkError|EmailLinkErrorCode|EmailLinkErrorCodeStatus|isClerkAPIResponseError|isClerkRuntimeError|isEmailLinkError|isKnownError|isMetamaskError|isUserLockedError|MetamaskError)" packages -g '!**/dist/**' -g '!**/build/**' || trueLength of output: 235
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Any lingering deep imports to core/resources/Error?"
rg -n -C2 "core/resources/Error" || echo "no matches"
echo
echo "2) Any imports from resources/internal that reference error symbols?"
rg -n -C2 "from\s+['\"][^'\"]*resources/internal['\"]" || echo "no imports from resources/internal"
echo
echo "Filter those imports for known error symbols (if any):"
rg -n -C2 "from\s+['\"][^'\"]*resources/internal['\"]" | rg -nP "(ClerkRuntimeError|ClerkAPIResponseError|EmailLinkError|EmailLinkErrorCode|EmailLinkErrorCodeStatus|isClerkAPIResponseError|isClerkRuntimeError|isEmailLinkError|isKnownError|isMetamaskError|isUserLockedError|MetamaskError|is4xxError)" || echo "no filtered matches"
echo
echo "3) Search repo for definitions/exports/usages of the migrated symbols (including is4xxError):"
symbols=(ClerkRuntimeError ClerkAPIResponseError EmailLinkError EmailLinkErrorCode EmailLinkErrorCodeStatus isClerkAPIResponseError isClerkRuntimeError isEmailLinkError isKnownError isMetamaskError isUserLockedError MetamaskError is4xxError)
for s in "${symbols[@]}"; do
echo "---- ${s} ----"
rg -n -S -g '!**/dist/**' -g '!**/build/**' "\b${s}\b" || echo "no matches for ${s}"
done
echo
echo "4) Any direct imports from '@clerk/shared/error' (sanity check)?"
rg -n -C2 "from\s+['\"]@clerk/shared/error['\"]" || echo "no direct imports from @clerk/shared/error"Length of output: 35678
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "A) package.json entries with @clerk/shared in the name"
rg -nP '"name"\s*:\s*"(?:@clerk/shared(?:/error)?)"' packages || echo "no package.json matched"
echo
echo "B) find directories named 'shared' or 'error' under packages (depth 3)"
find packages -maxdepth 3 -type d \( -name shared -o -name error \) -print || true
echo
echo "C) show packages/shared/error if present"
if [ -d packages/shared/error ]; then
echo "== packages/shared/error exists =="
ls -la packages/shared/error || true
if [ -f packages/shared/error/src/index.ts ]; then
echo "=== packages/shared/error/src/index.ts ==="
sed -n '1,240p' packages/shared/error/src/index.ts || true
fi
fi
echo
echo "D) locate potential definitions of is4xxError"
rg -nP "^(export\s+)?(const|function)\s+is4xxError\b|is4xxError\s*=" packages || echo "no direct definition matches for is4xxError"
echo
echo "E) search for explicit exports of main error symbols (may be re-exports)"
symbols=(is4xxError ClerkAPIResponseError ClerkRuntimeError EmailLinkError EmailLinkErrorCodeStatus parseError parseErrors buildErrorThrower isNetworkError)
for s in "${symbols[@]}"; do
echo "---- ${s} ----"
rg -nP "export\s+(?:type\s+)?(?:class|function|const|let|var|interface|type)\s+${s}\b|export\s*\{[^}]*\b${s}\b[^}]*\}" packages || echo "no export match for ${s}"
doneLength of output: 2625
Import migration verified — shared exports present; fix leftover internal/deep imports
packages/shared exports all referenced symbols (is4xxError in packages/shared/src/errors/helpers.ts; re-exports in packages/shared/src/error.ts). Update remaining files that still import from core/resources/Error or resources/internal to import from '@clerk/shared/error':
- packages/clerk-js/src/utils/tests/errors.spec.ts — imports ClerkAPIResponseError from '../../core/resources/Error'
- packages/clerk-js/src/core/fraudProtection.spec.ts — imports ClerkAPIResponseError from './resources/internal'
- packages/clerk-js/src/core/tests/clerk.test.ts — imports EmailLinkErrorCodeStatus from '../resources/internal'
🤖 Prompt for AI Agents
packages/clerk-js/src/core/clerk.ts (lines 4-11) and related files still use
deep/internal imports; update them to use the public shared export. Replace the
deep import paths in the following files so they import the listed symbols from
'@clerk/shared/error' instead of internal paths:
packages/clerk-js/src/utils/__tests__/errors.spec.ts — import
ClerkAPIResponseError from '@clerk/shared/error' instead of
'../../core/resources/Error'; packages/clerk-js/src/core/fraudProtection.spec.ts
— import ClerkAPIResponseError from '@clerk/shared/error' instead of
'./resources/internal'; packages/clerk-js/src/core/__tests__/clerk.test.ts —
import EmailLinkErrorCodeStatus (and any other error symbols) from
'@clerk/shared/error' instead of '../resources/internal'; run tests and adjust
named imports if any export names differ.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/clerk-js/src/utils/__tests__/errors.spec.ts (2)
7-7: Fix minor typo in test description.Change “the the” and tighten the wording.
Apply this diff:
-it('check if the the API response contains at least one error of the provided code', () => { +it('checks whether the API response contains at least one error with the provided code', () => {
8-15: Prefer narrower typing over a broad type assertion.Avoid asserting the whole object as ClerkAPIResponseError; narrow to the used shape and cast at the call site.
Apply this diff:
- const err = { - errors: [ - { - code: 'foo', - }, - ], - } as ClerkAPIResponseError; + const errLike = { + errors: [{ code: 'foo' }], + } as const satisfies Pick<ClerkAPIResponseError, 'errors'>; + const err = errLike as unknown as ClerkAPIResponseError;packages/clerk-js/src/core/resources/internal.ts (1)
6-6: Optional: prefer explicit named re-exports to aid tree‑shaking and avoid accidental surface growthBarrels with
export *can unintentionally widen the public surface and may hinder optimal tree‑shaking. If practical, consider switching to named re‑exports over time.Example:
-export * from './APIKey'; +export { APIKey } from './APIKey';
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/clerk-js/src/core/resources/index.ts(1 hunks)packages/clerk-js/src/core/resources/internal.ts(1 hunks)packages/clerk-js/src/utils/__tests__/errors.spec.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/clerk-js/src/core/resources/index.ts
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/clerk-js/src/utils/__tests__/errors.spec.tspackages/clerk-js/src/core/resources/internal.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/clerk-js/src/utils/__tests__/errors.spec.tspackages/clerk-js/src/core/resources/internal.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/utils/__tests__/errors.spec.tspackages/clerk-js/src/core/resources/internal.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/clerk-js/src/utils/__tests__/errors.spec.tspackages/clerk-js/src/core/resources/internal.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/clerk-js/src/utils/__tests__/errors.spec.tspackages/clerk-js/src/core/resources/internal.ts
packages/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Unit tests should use Jest or Vitest as the test runner.
Files:
packages/clerk-js/src/utils/__tests__/errors.spec.ts
packages/{clerk-js,elements,themes}/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Visual regression testing should be performed for UI components.
Files:
packages/clerk-js/src/utils/__tests__/errors.spec.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/clerk-js/src/utils/__tests__/errors.spec.tspackages/clerk-js/src/core/resources/internal.ts
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/__tests__/**/*.{ts,tsx}: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces
Files:
packages/clerk-js/src/utils/__tests__/errors.spec.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). (6)
- GitHub Check: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (3)
packages/clerk-js/src/utils/__tests__/errors.spec.ts (1)
1-1: Import migration LGTM — type-only import is correct; canonical module is '@clerk/shared/error'Verified: no occurrences of '@clerk/shared/errors' were found; imports consistently use '@clerk/shared/error'.
packages/clerk-js/src/core/resources/internal.ts (2)
6-6: Incorrect: claimed re-exports not present in resources/index.tspackages/clerk-js/src/core/resources/index.ts currently exports: AuthConfig, Client, DeletedObject, DisplayConfig, EmailAddress, Environment, ExternalAccount, IdentificationLink, Image, PhoneNumber, SamlAccount, Session, SessionWithActivities, SignIn, SignUp, Token, User, Verification, Waitlist, Web3Wallet — the review's claimed new re-exports (APIKey, BillingStatement, CommerceSettings, EnterpriseAccount, Feature, UserData, UserSettings) are not present.
Likely an incorrect or invalid review comment.
6-48: Verify all internal Error usages migrated to @clerk/shared/errors and path is consistentSandbox couldn't scan the repo — run locally and fix any matches referencing the old/local Error or the singular path '@clerk/shared/error':
- git grep -n "import { Error" || true
- git grep -n "export { Error" || true
- git grep -n "@clerk/shared/error" || true
- git grep -n "@clerk/shared/errors" || true
Replace any found imports/re-exports with the shared/errors path and remove exports of the local Error.
| /** | ||
| * Formatting for this file has been disabled in .prettierignore | ||
| * as we need to ensure that the import/export order always remains the same | ||
| */ |
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
Order-sensitive barrel: verify Prettier ignore and guard against auto-sorts
Good rationale to lock formatting for stable export order. Please confirm this file (or its parent dir) is indeed listed in .prettierignore. If the repo uses any import/export sorting ESLint plugins, consider disabling them for this file to prevent accidental reordering.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Checking .prettierignore for this file or its directory..."
rg -n -F "packages/clerk-js/src/core/resources/internal.ts" .prettierignore || true
rg -n -F "packages/clerk-js/src/core/resources" .prettierignore || true
echo
echo "Searching ESLint configs for rules that might reorder imports/exports..."
fd -HI '.*eslint.*' -t f -a | xargs -I{} sh -c 'echo "--- {} ---"; rg -n "simple-import-sort|import/order|sort-exports" "{}" || true'Length of output: 446
Lock export order: add to .prettierignore and disable import-sorting for this file
.no .prettierignore entry found for packages/clerk-js/src/core/resources/internal.ts (or its directory), and eslint.config.mjs enables eslint-plugin-simple-import-sort with "simple-import-sort/imports": "error" — that will auto-reorder imports/exports.
Action: add packages/clerk-js/src/core/resources/internal.ts (or packages/clerk-js/src/core/resources/) to .prettierignore and disable the simple-import-sort rule for this path (either via an override in eslint.config.mjs or a file-level disable comment, e.g. /* eslint-disable simple-import-sort/imports */).
.prettierignore (repo root)
eslint.config.mjs (repo root) — rule: simple-import-sort/imports
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/resources/internal.ts lines 1-4, the file intends
to lock import/export order but there is no .prettierignore entry and eslint's
simple-import-sort will auto-reorder imports; add
packages/clerk-js/src/core/resources/internal.ts (or the
packages/clerk-js/src/core/resources/ directory) to the repo .prettierignore to
prevent formatting, and disable the simple-import-sort/imports rule for this
path either by adding an override in eslint.config.mjs targeting that
file/directory or by adding a file-level disable comment at the top of the file
(e.g. /* eslint-disable simple-import-sort/imports */) so imports/exports remain
in the exact order required.
| export * from './Web3Wallet'; | ||
| export * from './Waitlist'; | ||
| export * from './APIKey'; | ||
| export * from './Web3Wallet'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
Order change: verify no subtle ordering dependency on Web3Wallet
Given the explicit note to keep order stable, double‑check that moving Web3Wallet in the list doesn’t affect any initialization or side‑effect expectations.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Show context around Web3Wallet export in this file:"
nl -ba packages/clerk-js/src/core/resources/internal.ts | sed -n '40,90p'
echo
echo "Search for imports that might rely on export order (rare, but precautionary):"
rg -nP --type=ts --type=tsx -C2 'from\s+["\'](?:\.\./)*core/resources/internal["\']'Length of output: 92
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Show internal.ts (full file):"
sed -n '1,240p' packages/clerk-js/src/core/resources/internal.ts || true
echo
echo "Search for any imports referencing 'resources/internal' (any relative form):"
rg -n --hidden -S "resources/internal" || true
echo
echo "Search repo for occurrences of 'Web3Wallet':"
rg -n --hidden -S "Web3Wallet" || true
echo
echo "Show candidate Web3Wallet file(s):"
rg -n --hidden -S "export .*Web3Wallet|class Web3Wallet|function Web3Wallet" || true
echo
echo "If Web3Wallet file found above, print its top 200 lines (attempt common path):"
for f in $(rg -n --hidden -S "Web3Wallet" --files); do
echo "---- $f ----"
sed -n '1,200p' "$f" || true
doneLength of output: 26074
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "----- Web3Wallet.ts -----"
sed -n '1,240p' packages/clerk-js/src/core/resources/Web3Wallet.ts || true
echo
echo "----- Base.ts -----"
sed -n '1,240p' packages/clerk-js/src/core/resources/Base.ts || true
echo
echo "----- User.ts -----"
sed -n '1,240p' packages/clerk-js/src/core/resources/User.ts || true
echo
echo "----- Quick grep for top-level calls/side-effects in these files -----"
rg -n --hidden -S --no-heading "(console\.|process\.|module\.|register|initialize|initiate|setup|start|create\()" packages/clerk-js/src/core/resources || trueLength of output: 28444
Keep Web3Wallet export after its dependencies in internal.ts (or import dependencies directly)
Web3Wallet imports BaseResource and Verification from './internal'; internal.ts currently exports Base and Verification before Web3Wallet — moving Web3Wallet earlier than those can break module initialization because of circular re-exports. Web3Wallet has no top-level side-effects.
Action: keep Web3Wallet after Base and Verification in packages/clerk-js/src/core/resources/internal.ts, or refactor packages/clerk-js/src/core/resources/Web3Wallet.ts to import Base and Verification directly.
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/resources/internal.ts around line 48, the export
for Web3Wallet appears before its dependencies (BaseResource and Verification)
which can cause circular re-export initialization issues; either move the
"export * from './Web3Wallet';" line to after the exports for Base and
Verification so Web3Wallet is exported only after its dependencies, or refactor
packages/clerk-js/src/core/resources/Web3Wallet.ts to import BaseResource and
Verification directly from their files (avoiding re-exports) and then keep the
current export order.
nikosdouvlis
force-pushed
the
nikos/userland-errors-2
branch
2 times, most recently
from
fba86d4 to
20486ff
Compare
nikosdouvlis
force-pushed
the
nikos/userland-errors-2
branch
from
20486ff to
5010746
Compare
4 participants
Description
Removes the internal Error module in favor of the shared one
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
Refactor
Chores