refactor(clerk-js): Remove unused debugLogger functionality #6615
Conversation
🦋 Changeset detectedLatest commit: 3c13778 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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 GitHub.
|
jacekradko
changed the title
📝 WalkthroughWalkthroughRemoved log filtering and the 'trace' level across the clerk-js debug module. Deleted DebugLogFilter and several mutable/debug-config types; DebugLogLevel, VALID_LOG_LEVELS, and related interfaces were narrowed to error|warn|info|debug. DebugLoggerInterface renamed to DebugLogger and its Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 🪧 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/Issue comments)Type 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: 2
🧹 Nitpick comments (20)
packages/clerk-js/src/core/modules/debug/transports/telemetry.ts (2)
41-58: Simplify send by removing unnecessary Promise.resolve wrapper.
await this.collector.recordLog(...)is sufficient and clearer. The current pattern evaluates the argument first anyway and doesn’t change error semantics.Apply this diff:
async send(entry: DebugLogEntry): Promise<void> { if (!this.collector) { return; } - await Promise.resolve( - this.collector.recordLog({ - context: entry.context, - level: entry.level, - message: entry.message, - organizationId: entry.organizationId, - sessionId: entry.sessionId, - source: entry.source, - timestamp: entry.timestamp, - userId: entry.userId, - }), - ); + await this.collector.recordLog({ + context: entry.context, + level: entry.level, + message: entry.message, + organizationId: entry.organizationId, + sessionId: entry.sessionId, + source: entry.source, + timestamp: entry.timestamp, + userId: entry.userId, + }); }
10-13: Consolidate duplicate TelemetryLoggerOptions definitionsThe
TelemetryLoggerOptionsinterface is currently declared in two places and only consumed in one:
packages/clerk-js/src/core/modules/debug/transports/telemetry.ts:10– declares the interface but never uses it.packages/clerk-js/src/core/modules/debug/index.ts:44– redeclares the same interface, and at line 205 it’s actually used as the type ofoptions.To avoid duplication and the risk of API drift, pick a single source of truth for this interface:
• Remove the declaration in telemetry.ts, and import/re-export the one from index.ts
• -or-
• Remove the declaration in index.ts, and have it re-export the one from telemetry.tsThis will keep the public surface tight and ensure future changes to the interface stay in sync.
packages/clerk-js/src/core/modules/debug/transports/composite.ts (3)
9-15: Avoid duplicating the log level union; use DebugLogLevel and make options readonly-friendly.This prevents divergence from the source of truth and tightens types.
Apply this diff:
-import type { DebugLogEntry, DebugTransport } from '../types'; +import type { DebugLogEntry, DebugLogLevel, DebugTransport } from '../types'; @@ -export interface CompositeLoggerOptions { - logLevel?: 'error' | 'warn' | 'info' | 'debug'; - transports: Array<{ - options?: Record<string, unknown>; - transport: DebugTransport; - }>; -} +export interface CompositeLoggerOptions { + logLevel?: DebugLogLevel; + transports: ReadonlyArray<{ + readonly options?: Record<string, unknown>; + readonly transport: DebugTransport; + }>; +}
23-33: Constructor and field are good; consider typing the field as readonly array.Minor: the property is already
readonly; usingreadonly DebugTransport[]conveys intent better at use sites and prevents accidental mutation at compile time.Apply this diff:
-export class CompositeTransport implements DebugTransport { - private readonly transports: DebugTransport[]; +export class CompositeTransport implements DebugTransport { + private readonly transports: readonly DebugTransport[];
41-48: Improve error context when child transport fails.Including the transport identity makes failures actionable without stepping through.
Apply this diff:
- const promises = this.transports.map(transport => - transport.send(entry).catch(err => { - console.error('Failed to send to transport:', err); - }), - ); + const promises = this.transports.map(t => + t.send(entry).catch((err: unknown) => { + const name = (t as any)?.constructor?.name ?? 'UnknownTransport'; + console.error(`Failed to send to transport ${name}:`, err); + }), + );packages/clerk-js/src/core/modules/debug/types.ts (3)
87-89: Slightly harden the log level guard.Optional: also ensure the value exists in the canonical constant to avoid subtle string widening bugs.
Apply this diff:
-export function isValidLogLevel(level: unknown): level is DebugLogLevel { - return typeof level === 'string' && VALID_LOG_LEVELS.includes(level as DebugLogLevel); -} +export function isValidLogLevel(level: unknown): level is DebugLogLevel { + return typeof level === 'string' && (VALID_LOG_LEVELS as readonly string[]).includes(level); +}
101-113: Type guard could validate numeric timestamp more strictly and avoid accepting NaN.Not critical, but prevents malformed entries early.
Apply this diff:
return ( typeof obj === 'object' && obj !== null && 'timestamp' in obj && 'level' in obj && 'message' in obj && - typeof (obj as DebugLogEntry).timestamp === 'number' && + typeof (obj as DebugLogEntry).timestamp === 'number' && + Number.isFinite((obj as DebugLogEntry).timestamp) && typeof (obj as DebugLogEntry).level === 'string' && isValidLogLevel((obj as DebugLogEntry).level) && typeof (obj as DebugLogEntry).message === 'string' );
9-10: Consider freezing exported arrays at runtime.
readonlyprotects at compile time but not at runtime. Freezing prevents accidental mutation when the object escapes TS.Apply this diff:
-export const VALID_LOG_LEVELS: readonly DebugLogLevel[] = ['error', 'warn', 'info', 'debug'] as const; +export const VALID_LOG_LEVELS: readonly DebugLogLevel[] = Object.freeze(['error', 'warn', 'info', 'debug'] as const);packages/clerk-js/src/core/modules/debug/__tests__/logger.spec.ts (3)
56-66: Nice threshold test; add a complementary “error-only” case for completeness.A quick extra test ensures the top-most threshold behaves as expected.
Apply this diff:
it('should respect log level filtering', () => { const infoLogger = new DebugLogger(mockTransport, 'info'); infoLogger.debug('debug message'); infoLogger.info('info message'); infoLogger.warn('warn message'); infoLogger.error('error message'); expect(mockTransport.sentEntries).toHaveLength(3); expect(mockTransport.sentEntries.map(e => e.level)).toEqual(['info', 'warn', 'error']); }); + + it('should emit only errors when log level is error', () => { + const errorOnly = new DebugLogger(mockTransport, 'error'); + errorOnly.debug('debug message'); + errorOnly.warn('warn message'); + errorOnly.info('info message'); + errorOnly.error('error message'); + expect(mockTransport.sentEntries).toHaveLength(1); + expect(mockTransport.sentEntries[0].level).toBe('error'); + });
100-117: Use a microtask tick instead of setTimeout(0) to flush async sends.
await Promise.resolve()is enough to allow the promise callbacks to run and is faster/less flaky.Apply this diff:
- await new Promise(resolve => setTimeout(resolve, 0)); + await Promise.resolve();
119-127: Timestamp assertion is solid; consider tolerating clock skew in CI.Not required, but if flakiness appears, capture
const now = Date.now()once per test and only assert lower-bound.packages/clerk-js/src/core/modules/debug/logger.ts (3)
74-86: Avoid emitting undefined fields to keep payloads lean.Conditionally include
contextandsource. This reduces noise for telemetry and wire size without changing semantics.Apply this diff:
- const entry: DebugLogEntry = { - timestamp: Date.now(), - level, - message, - context, - source, - }; + const base = { + timestamp: Date.now(), + level, + message, + } as const; + const entry: DebugLogEntry = { + ...base, + ...(context !== undefined ? { context } : {}), + ...(source !== undefined ? { source } : {}), + };
87-90: Type the caught error as unknown and narrow for safer logging.Aligns with the TypeScript error-handling guideline.
Apply this diff:
- this.transport.send(entry).catch(err => { - console.error('Failed to send log entry:', err); - }); + this.transport.send(entry).catch((err: unknown) => { + if (err instanceof Error) { + console.error('Failed to send log entry:', err); + } else { + console.error('Failed to send log entry:', { err }); + } + });
92-97: Use the central VALID_LOG_LEVELS to avoid drift and reallocation.This keeps ordering consistent with the types module and avoids recreating the array per call.
Apply this diff:
-import type { DebugLogEntry, DebugLogLevel, DebugTransport } from './types'; +import { VALID_LOG_LEVELS } from './types'; +import type { DebugLogEntry, DebugLogLevel, DebugTransport } from './types'; @@ - private shouldLogLevel(level: DebugLogLevel): boolean { - const levels: DebugLogLevel[] = ['error', 'warn', 'info', 'debug']; - const currentLevelIndex = levels.indexOf(this.logLevel); - const messageLevelIndex = levels.indexOf(level); - return messageLevelIndex <= currentLevelIndex; - } + private shouldLogLevel(level: DebugLogLevel): boolean { + const currentLevelIndex = VALID_LOG_LEVELS.indexOf(this.logLevel); + const messageLevelIndex = VALID_LOG_LEVELS.indexOf(level); + return messageLevelIndex <= currentLevelIndex; + }packages/clerk-js/src/core/modules/debug/index.ts (5)
24-31: Remove or wire up the unusedendpointoption
endpointis accepted in multiple option types and increateLogger(...), but it isn’t used anywhere in this module (the transport only receives aTelemetryCollector). This is misleading in a public API.Two options:
- Remove
endpointfrom the public options here, and keep it solely on theTelemetryCollector’s own configuration.- Or, if needed, plumb
endpointinto a suitable place (e.g., constructing the collector internally).If removing, apply:
export interface LoggerOptions { - /** Optional URL to which telemetry logs will be sent. */ - endpoint?: string; /** Minimum log level to capture. */ logLevel?: DebugLogLevel; /** Optional collector instance for custom telemetry handling. */ telemetryCollector?: TelemetryCollector; } export interface TelemetryLoggerOptions { - /** Optional URL to which telemetry logs will be sent. */ - endpoint?: string; /** Minimum log level to capture. */ logLevel?: DebugLogLevel; /** Optional collector instance for custom telemetry handling. */ telemetryCollector?: TelemetryCollector; } export function createLogger(options: { - endpoint?: string; logLevel?: DebugLogLevel; telemetryCollector?: TelemetryCollector; }): { logger: DebugLogger; transport: CompositeTransport } | null {Also applies to: 44-51, 149-153
10-10: Export and centralize the default log level
DEFAULT_LOG_LEVELhere is'info', whileutils/debug.tsdefaults to'debug'andlogger.tsJSDoc still says “Defaults to 'debug'”. Export this constant so other modules can reference one source of truth.-const DEFAULT_LOG_LEVEL: DebugLogLevel = 'info'; +export const DEFAULT_LOG_LEVEL: DebugLogLevel = 'info';Follow-up:
- Update
utils/debug.tsto import and use this constant (or pass no level and let the core decide).- Update
logger.tsJSDoc to reflect the actual default.
144-149: Update JSDoc: telemetry is not always includedThe doc says “Creates a composite logger with both console and telemetry transports,” but in non-production
createLoggeruses console-only. Adjust the description to match behavior.-/** - * Creates a composite logger with both console and telemetry transports - * +/** + * Creates a composite logger. + * In production: console + optional telemetry (when a collector is provided). + * In non-production: console-only. + *
56-59: Prefer interface-based transport typing to avoid unnecessary coupling
CompositeLoggerOptions.transportsis typed asConsoleTransport | TelemetryTransport. Consider typing this against the transport interface (e.g.,DebugTransport) so additional transports can be introduced without touching consumer types.
15-19: Harden runtime validation forlogLevelIf you keep runtime checks, validate against the allowed levels rather than only the string type. This avoids silent acceptance of typos coming from dynamic inputs.
function validateLoggerOptions<T extends { logLevel?: unknown }>(options: T): void { - if (options.logLevel && typeof options.logLevel !== 'string') { - throw new Error('logLevel must be a string'); - } + if (options.logLevel !== undefined) { + const valid = ['error', 'warn', 'info', 'debug'] as const; + if (typeof options.logLevel !== 'string' || !valid.includes(options.logLevel as any)) { + throw new Error(`Invalid logLevel: ${String(options.logLevel)}. Expected one of: ${valid.join(', ')}`); + } + } }packages/clerk-js/src/utils/debug.ts (1)
16-21: Consider documentingInitOptionsmore explicitly and clarifying single-initialization semanticsGiven the one-shot model, callers should know that subsequent
initDebugLoggercalls are ignored unless the first attempt failed. Add brief JSDoc toInitOptionsand reference the retry behavior suggested above.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
packages/clerk-js/src/core/clerk.ts(0 hunks)packages/clerk-js/src/core/modules/debug/__tests__/logger.spec.ts(4 hunks)packages/clerk-js/src/core/modules/debug/index.ts(6 hunks)packages/clerk-js/src/core/modules/debug/logger.ts(4 hunks)packages/clerk-js/src/core/modules/debug/transports/__tests__/telemetry.spec.ts(0 hunks)packages/clerk-js/src/core/modules/debug/transports/composite.ts(2 hunks)packages/clerk-js/src/core/modules/debug/transports/console.ts(0 hunks)packages/clerk-js/src/core/modules/debug/transports/telemetry.ts(1 hunks)packages/clerk-js/src/core/modules/debug/types.ts(1 hunks)packages/clerk-js/src/utils/debug.ts(5 hunks)
💤 Files with no reviewable changes (3)
- packages/clerk-js/src/core/modules/debug/transports/tests/telemetry.spec.ts
- packages/clerk-js/src/core/modules/debug/transports/console.ts
- packages/clerk-js/src/core/clerk.ts
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{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/debug/transports/telemetry.tspackages/clerk-js/src/core/modules/debug/types.tspackages/clerk-js/src/core/modules/debug/transports/composite.tspackages/clerk-js/src/core/modules/debug/logger.tspackages/clerk-js/src/core/modules/debug/index.tspackages/clerk-js/src/core/modules/debug/__tests__/logger.spec.tspackages/clerk-js/src/utils/debug.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/debug/transports/telemetry.tspackages/clerk-js/src/core/modules/debug/types.tspackages/clerk-js/src/core/modules/debug/transports/composite.tspackages/clerk-js/src/core/modules/debug/logger.tspackages/clerk-js/src/core/modules/debug/index.tspackages/clerk-js/src/core/modules/debug/__tests__/logger.spec.tspackages/clerk-js/src/utils/debug.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/core/modules/debug/transports/telemetry.tspackages/clerk-js/src/core/modules/debug/types.tspackages/clerk-js/src/core/modules/debug/transports/composite.tspackages/clerk-js/src/core/modules/debug/logger.tspackages/clerk-js/src/core/modules/debug/index.tspackages/clerk-js/src/core/modules/debug/__tests__/logger.spec.tspackages/clerk-js/src/utils/debug.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/debug/transports/telemetry.tspackages/clerk-js/src/core/modules/debug/types.tspackages/clerk-js/src/core/modules/debug/transports/composite.tspackages/clerk-js/src/core/modules/debug/logger.tspackages/clerk-js/src/core/modules/debug/index.tspackages/clerk-js/src/core/modules/debug/__tests__/logger.spec.tspackages/clerk-js/src/utils/debug.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/debug/transports/telemetry.tspackages/clerk-js/src/core/modules/debug/types.tspackages/clerk-js/src/core/modules/debug/transports/composite.tspackages/clerk-js/src/core/modules/debug/logger.tspackages/clerk-js/src/core/modules/debug/index.tspackages/clerk-js/src/core/modules/debug/__tests__/logger.spec.tspackages/clerk-js/src/utils/debug.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/debug/transports/telemetry.tspackages/clerk-js/src/core/modules/debug/types.tspackages/clerk-js/src/core/modules/debug/transports/composite.tspackages/clerk-js/src/core/modules/debug/logger.tspackages/clerk-js/src/core/modules/debug/index.tspackages/clerk-js/src/core/modules/debug/__tests__/logger.spec.tspackages/clerk-js/src/utils/debug.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/modules/debug/transports/telemetry.tspackages/clerk-js/src/core/modules/debug/types.tspackages/clerk-js/src/core/modules/debug/transports/composite.tspackages/clerk-js/src/core/modules/debug/logger.tspackages/clerk-js/src/core/modules/debug/index.tspackages/clerk-js/src/core/modules/debug/__tests__/logger.spec.tspackages/clerk-js/src/utils/debug.ts
packages/**/index.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use tree-shaking friendly exports
Files:
packages/clerk-js/src/core/modules/debug/index.ts
**/index.ts
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
Use index.ts files for clean imports but avoid deep barrel exports
Avoid barrel files (index.ts re-exports) as they can cause circular dependencies
Files:
packages/clerk-js/src/core/modules/debug/index.ts
packages/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Unit tests should use Jest or Vitest as the test runner.
Files:
packages/clerk-js/src/core/modules/debug/__tests__/logger.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/core/modules/debug/__tests__/logger.spec.ts
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/__tests__/**/*.{ts,tsx}: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces
Files:
packages/clerk-js/src/core/modules/debug/__tests__/logger.spec.ts
🧬 Code graph analysis (4)
packages/clerk-js/src/core/modules/debug/logger.ts (1)
packages/clerk-js/src/core/modules/debug/types.ts (2)
DebugTransport(52-57)DebugLogLevel(4-4)
packages/clerk-js/src/core/modules/debug/index.ts (7)
packages/clerk-js/src/core/modules/debug/transports/console.ts (1)
ConsoleTransport(36-79)packages/clerk-js/src/core/modules/debug/transports/telemetry.ts (1)
TelemetryTransport(23-59)packages/clerk-js/src/core/modules/debug/transports/composite.ts (1)
CompositeTransport(23-49)packages/clerk-js/src/core/modules/debug/logger.ts (1)
DebugLogger(13-98)packages/clerk-js/src/core/modules/debug/types.ts (1)
DebugLogLevel(4-4)packages/shared/src/telemetry/collector.ts (1)
TelemetryCollector(102-443)packages/types/src/telemetry.ts (1)
TelemetryCollector(60-65)
packages/clerk-js/src/core/modules/debug/__tests__/logger.spec.ts (3)
packages/clerk-js/src/core/modules/debug/logger.ts (1)
DebugLogger(13-98)packages/clerk-js/src/utils/debug.ts (1)
DebugLogger(9-14)packages/clerk-js/src/core/modules/debug/types.ts (1)
DebugLogEntry(24-33)
packages/clerk-js/src/utils/debug.ts (4)
packages/clerk-js/src/core/modules/debug/logger.ts (2)
DebugLogger(13-98)error(46-48)packages/clerk-js/src/core/modules/debug/types.ts (1)
DebugLogLevel(4-4)packages/types/src/telemetry.ts (1)
TelemetryCollector(60-65)packages/clerk-js/src/core/modules/debug/index.ts (1)
getDebugLogger(138-141)
🔇 Additional comments (9)
packages/clerk-js/src/core/modules/debug/transports/telemetry.ts (1)
1-3: Import changes look correct and type-only usage is appropriate.Moving
TelemetryCollectorto@clerk/typesand usingimport typepreserves tree-shaking. No concerns.packages/clerk-js/src/core/modules/debug/transports/composite.ts (1)
9-15: I’ve pulled the surrounding lines inpackages/clerk-js/src/core/modules/debug/index.tsto see howCompositeLoggerOptionsis referenced. Once you provide the snippet output, we can confirm whether it’s truly used by any public API or just re-exported internally.packages/clerk-js/src/core/modules/debug/types.ts (1)
4-10: Trace removal and VALID_LOG_LEVELS update look consistent.The narrowed
DebugLogLevelandVALID_LOG_LEVELSalign with the PR goal. Good use ofas const.packages/clerk-js/src/core/modules/debug/__tests__/logger.spec.ts (1)
24-39: Test adjustments for trace removal look good and validate ordering across levels.Good coverage of the four supported levels and the default level behavior.
packages/clerk-js/src/core/modules/debug/logger.ts (2)
23-26: Constructor simplification looks good.Removing filters, keeping an optional
logLevelwith a sensible default, and explicit return types align with the refactor goals.
61-61: Trace removal acknowledged.Public API now focuses on error/warn/info/debug only. Matches types and tests.
packages/clerk-js/src/core/modules/debug/index.ts (1)
95-106: The script above will extract the full bodies of bothcreateLoggerandperformInitializationfrompackages/clerk-js/src/core/modules/debug/index.ts. Once we have those implementations, we can confirm whethertelemetryCollectoris consistently gated by environment in both initialization paths.packages/clerk-js/src/utils/debug.ts (2)
37-41: Intentional no-op buffering when disabled — LGTMPre-init buffering is skipped unless explicitly enabled. This matches the “no-op until initialized” contract and avoids unexpected memory growth before opt-in.
150-179: Defensive pass-through with pre-init buffering — LGTMThe singleton safely buffers by level and flushes with additional pre-init context once the real logger is ready. Method signatures align with the trimmed public API (no
trace).
| try { | ||
| console.debug?.('Debug logger initialization failed', error); | ||
| } catch { | ||
| void 0; | ||
| } | ||
| // Silently return to avoid unhandled rejections and preserve behavior | ||
|
|
||
| return; | ||
| } |
There was a problem hiding this comment.
Allow retry after initialization failure
If the dynamic import or initialization fails once, initializationAttempted remains true, preventing any future retry. Reset the flag on failure so a subsequent call to initDebugLogger can reattempt.
Apply:
} catch (error) {
try {
console.debug?.('Debug logger initialization failed', error);
} catch {
void 0;
}
-
- return;
+ // Permit retry on subsequent initDebugLogger() calls
+ initializationAttempted = false;
+ return;
}Alternatively, introduce an initializationInFlight promise (like the core manager) to dedupe concurrent attempts while still allowing retries on failure.
Also applies to: 121-134
🤖 Prompt for AI Agents
In packages/clerk-js/src/utils/debug.ts around lines 94-101 (and similarly
121-134), initializationAttempted is set to true on the first failure which
blocks future retries; change the flow so that on any failure you reset
initializationAttempted to false (or better: replace the boolean with an
initializationInFlight Promise to dedupe concurrent initializations while
allowing retries on rejection). Concretely, on catch paths reset
initializationAttempted = false before returning, or implement an
initializationInFlight promise that is assigned at start, awaited by callers,
cleared on success or rejection so subsequent calls can retry.
@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: |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/clerk-js/src/core/modules/debug/types.ts (2)
4-10: Avoid drift: derive DebugLogLevel from VALID_LOG_LEVELSDefine the union from the canonical array so the type and data cannot get out of sync.
Apply:
- export type DebugLogLevel = 'error' | 'warn' | 'info' | 'debug'; +// Derived from VALID_LOG_LEVELS to ensure the union stays in sync with the values. - export const VALID_LOG_LEVELS: readonly DebugLogLevel[] = ['error', 'warn', 'info', 'debug'] as const; +export const VALID_LOG_LEVELS = ['error', 'warn', 'info', 'debug'] as const; +export type DebugLogLevel = typeof VALID_LOG_LEVELS[number];
14-20: Consistency: derive DebugEventType from VALID_EVENT_TYPES as wellMirror the same “single source of truth” pattern for event types to prevent accidental divergence.
Suggested update:
- export const VALID_EVENT_TYPES: readonly DebugEventType[] = ['navigation', 'custom_event'] as const; +export const VALID_EVENT_TYPES = ['navigation', 'custom_event'] as const; - export type DebugEventType = 'navigation' | 'custom_event'; +export type DebugEventType = typeof VALID_EVENT_TYPES[number];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/clerk-js/src/core/modules/debug/types.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/modules/debug/types.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/debug/types.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/core/modules/debug/types.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/debug/types.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/debug/types.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/debug/types.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/modules/debug/types.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: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
packages/clerk-js/src/core/modules/debug/types.ts (1)
4-4: Trace level removed — no residual references foundI’ve performed a repo-wide sweep across
.ts,.tsx, and documentation files for:
- Any
DebugLogFilterAPI orfiltersconfig- Non-
console.tracecallsites- Config entries using
'trace'as a log level- Markdown/MDX references to
trace,DebugLogFilter, orfiltersin the context of loggingNo matches were found. It looks like the
tracelevel and its related filter API have been fully removed. Feel free to clean up any associated types or docs and proceed.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/clerk-js/src/utils/debug.ts (2)
83-88: Good: wrapper no longer overrides core logLevel defaultPassing
options?.logLevelstraight through lets the core decide the default and removes the previous drift. This addresses the earlier feedback.
89-101: Permit retry when initialization returns null or throwsRight now a failed init (dynamic import error or
getDebugLoggerresolving tonull) leavesinitializationAttempted = true, permanently blocking future retries while logs continue buffering. Reset the flag on failure or null to allow subsequentinitDebugLogger()calls to re-attempt.Apply:
@@ - if (logger) { - realLogger = logger; - flushBuffered(); - } + if (logger) { + realLogger = logger; + flushBuffered(); + } else { + // Core declined to create a logger (e.g., unsupported env) — allow future retries. + initializationAttempted = false; + } @@ } catch (error) { try { console.debug?.('Debug logger initialization failed', error); } catch { void 0; } - - return; + // Allow retry on subsequent initDebugLogger() calls + initializationAttempted = false; + return; }
🧹 Nitpick comments (7)
packages/clerk-js/src/utils/debug.ts (7)
108-112: Clarify JSDoc: document the effective default for logLevelTo avoid ambiguity, note that when omitted, the core logger’s default applies. This prevents future drift without hardcoding a literal here.
- * @param options.logLevel - Minimal level to log; lower-priority logs are ignored. Valid levels: 'error' | 'warn' | 'info' | 'debug'. + * @param options.logLevel - Minimal level to log; lower-priority logs are ignored. Valid levels: 'error' | 'warn' | 'info' | 'debug'. If omitted, the core logger's default level is used.
121-134: Optional: use an in-flight promise instead of a boolean sentinelA small in-flight promise (cleared on settle) dedupes concurrent init calls while still allowing retries after rejection, and eliminates edge cases around toggling
initializationAttempted.If you want, I can follow up with a minimal refactor introducing
let initializationInFlight: Promise<void> | null = null;and routinginitDebugLoggerthrough it.
136-149: MarkdebugLoggeras public in JSDocThe surface is public API; add
@publicfor docs consistency./** * Singleton debug logger surface. * * - No-op until `initDebugLogger` initializes the real logger * - Safe to import anywhere; all methods are guarded * + * @public * @example
150-179: Runtime immutability of the singletonTypes prevent accidental mutation at compile-time, but freezing the object guards against accidental runtime mutation from JS consumers.
-export const debugLogger: Readonly<DebugLogger> = { +export const debugLogger: Readonly<DebugLogger> = Object.freeze({ debug(message: string, context?: Record<string, unknown>, source?: string): void { @@ }, -}; +});
37-45: Buffer gating is correct; keep an eye on growth under repeated failuresEarly-returning when
!isEnabledprevents unbounded growth before init. With the retry fix above, buffered logs will eventually flush; without it, the buffer just cycles at 200 entries. The retry fix is important to avoid silent log loss.
9-14: Naming nuance: avoid confusion with core’sDebugLoggerclassThis exported interface is structurally compatible with the core class of the same name. If teams import both in one file, the shared name can be confusing. Consider aliasing locally to
DebugLoggerSurfaceor type-aliasing the core class where it’s referenced.
1-20: Tests recommended for init, buffering, and retry pathsNo tests were added here. Please add focused tests to cover:
- buffer behavior before init and after successful init (flush +
__preInitmarkers),- failure case (dynamic import rejection) resets
initializationAttemptedand allows retry,- null logger case (core returns
null) also resets and retries,- disabled path (
enabled: false) remains a no-op.I can scaffold these in
packages/clerk-js/src/utils/__tests__/debug.test.tswith mocked dynamic import.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/clerk-js/src/utils/debug.ts(5 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/utils/debug.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/debug.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/utils/debug.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/debug.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/debug.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/debug.ts
**/*
⚙️ CodeRabbit configuration file
If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.
Files:
packages/clerk-js/src/utils/debug.ts
🧬 Code graph analysis (1)
packages/clerk-js/src/utils/debug.ts (4)
packages/clerk-js/src/core/modules/debug/logger.ts (2)
DebugLogger(13-98)error(46-48)packages/clerk-js/src/core/modules/debug/types.ts (1)
DebugLogLevel(4-4)packages/types/src/telemetry.ts (1)
TelemetryCollector(60-65)packages/clerk-js/src/core/modules/debug/index.ts (1)
getDebugLogger(138-141)
⏰ 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: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
Description
Removing extraneous debugLogger functionality, primarily log filtering and trace level. Also simplifying to initialization logic.
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
Breaking Changes
Behavior Changes
Tests
Chores