-
Notifications
You must be signed in to change notification settings - Fork 381
feat(testing): Sign in via email in tests #6545
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 8049e0e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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.
|
@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: |
📝 WalkthroughWalkthroughAdds a Playwright testing API to sign in users by email via @clerk/testing/playwright: clerk.signIn({ emailAddress, page }). Implements a ticket-based sign-in flow that looks up a user by email, creates a short-lived ticket using CLERK_SECRET_KEY, and signs the page in by ticket. Introduces ticket strategy support across testing helpers and types, refactors sign-in dispatch to a switch with stricter phone_code/email_code validation, adds a sandbox signInWithEmail helper and a Playwright integration test, and overloads the Playwright clerk.signIn typing to accept email-based params. Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Assessment against linked issues
Out-of-scope changes
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/testing/src/common/helpers-utils.ts (1)
1-124
: Add unit tests for the new sign-in strategiesWe need coverage in packages/testing/src/common/helpers-utils.ts to ensure each branch behaves as expected:
• Ticket strategy
– Success:signIn.create({ strategy: 'ticket', ticket })
leads toClerk.setActive
– Failure: non-“complete” status throws the correct error• Phone code strategy
– Invalid phone number throws the validation error
– Missingphone_code
factor throws “not enabled” error
– Success path callsprepareFirstFactor
,attemptFirstFactor
andClerk.setActive
• Email code strategy
– Missing+clerk_test
in identifier throws the validation error
– Missingemail_code
factor throws “not enabled” error
– Success path callsprepareFirstFactor
,attemptFirstFactor
andClerk.setActive
Please add Jest (or your chosen framework) tests that mock
windowObject.Clerk.client
to cover all success and failure flows.
🧹 Nitpick comments (7)
packages/testing/src/common/types.ts (1)
53-66
: Consider adding JSDoc documentation for the new ticket strategyThe new
ticket
strategy variant should have documentation explaining its purpose and usage, consistent with the existing documentation style in this file.Add JSDoc documentation above the union type:
+/** + * Parameters for signing in a user during tests. + * - `password`: Sign in with username/email and password + * - `phone_code`/`email_code`: Sign in with test phone/email using verification code + * - `ticket`: Sign in with a pre-generated sign-in token (used internally for email-based sign-in) + */ export type ClerkSignInParams =packages/clerk-js/sandbox/integration/sign-in.spec.ts (1)
22-43
: Test looks good but could benefit from better isolationThe test implementation is correct and properly validates the email sign-in functionality. However, it could be improved by cleaning up after itself to ensure test isolation.
Consider adding a cleanup step after the test to sign out the user:
test('sign in with email', async ({ page }) => { await page.goto('/sign-in'); await clerk.signIn({ emailAddress: '[email protected]', page, }); await page.waitForFunction(() => window.Clerk?.user !== null); const userInfo = await page.evaluate(() => ({ isSignedIn: window.Clerk?.user !== null && window.Clerk?.user !== undefined, email: window.Clerk?.user?.primaryEmailAddress?.emailAddress, userId: window.Clerk?.user?.id, isLoaded: window.Clerk?.loaded, })); expect(userInfo.isSignedIn).toBe(true); expect(userInfo.email).toBe('[email protected]'); expect(userInfo.userId).toBeTruthy(); expect(userInfo.isLoaded).toBe(true); + + // Clean up by signing out + await clerk.signOut({ page }); });packages/testing/src/playwright/helpers.ts (4)
126-129
: Consider using a more descriptive error messageWhile the error correctly identifies the missing environment variable, it could be more helpful by suggesting how to set it up.
- throw new Error('CLERK_SECRET_KEY environment variable is required for email-based sign-in'); + throw new Error('CLERK_SECRET_KEY environment variable is required for email-based sign-in. Please set it in your test environment or .env file.');
152-154
: Consider more specific error handlingThe generic catch block might hide useful error details. Consider preserving the original error type when possible.
- } catch (err: any) { - throw new Error(`Failed to sign in with email ${emailAddress}: ${err?.message}`); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + throw new Error(`Failed to sign in with email ${emailAddress}: ${message}`);
144-144
: Consider making the token expiration configurableThe hard-coded 5-minute expiration might be insufficient for some test scenarios. Consider making this configurable through the options.
You could add an optional
tokenExpiresInSeconds
field toPlaywrightClerkSignInParamsWithEmail
:type PlaywrightClerkSignInParamsWithEmail = { page: Page; emailAddress: string; setupClerkTestingTokenOptions?: SetupClerkTestingTokenOptions; + tokenExpiresInSeconds?: number; };
Then use it in the token creation:
const signInToken = await clerkClient.signInTokens.createSignInToken({ userId: user.id, - expiresInSeconds: 300, // 5 minutes + expiresInSeconds: opts.tokenExpiresInSeconds ?? 300, // Default to 5 minutes });
110-160
: Consider adding tests for the new email-based sign-in functionalityWhile the integration test in
sign-in.spec.ts
covers the happy path, it would be beneficial to add unit tests that specifically test error scenarios like:
- Missing CLERK_SECRET_KEY
- User not found by email
- Token creation failure
Would you like me to help create comprehensive unit tests for the new email-based sign-in functionality to ensure all error paths are covered?
packages/testing/src/common/helpers-utils.ts (1)
118-120
: Consider improving type safety for the default case.The use of
as any
in the error message could be avoided by using a more type-safe approach.Apply this diff to improve type safety:
- default: - throw new Error(`Unsupported strategy: ${(signInParams as any).strategy}`); + default: { + const exhaustiveCheck: never = signInParams; + throw new Error(`Unsupported strategy: ${(exhaustiveCheck as any).strategy}`); + }This pattern ensures TypeScript will error if a new strategy is added to the union type but not handled in the switch statement.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.changeset/easy-parrots-slide.md
(1 hunks)packages/clerk-js/sandbox/integration/helpers.ts
(1 hunks)packages/clerk-js/sandbox/integration/sign-in.spec.ts
(1 hunks)packages/testing/src/common/helpers-utils.ts
(1 hunks)packages/testing/src/common/types.ts
(1 hunks)packages/testing/src/playwright/helpers.ts
(4 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}
: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/clerk-js/sandbox/integration/helpers.ts
packages/clerk-js/sandbox/integration/sign-in.spec.ts
packages/testing/src/common/types.ts
packages/testing/src/common/helpers-utils.ts
packages/testing/src/playwright/helpers.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/sandbox/integration/helpers.ts
packages/clerk-js/sandbox/integration/sign-in.spec.ts
packages/testing/src/common/types.ts
packages/testing/src/common/helpers-utils.ts
packages/testing/src/playwright/helpers.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/sandbox/integration/helpers.ts
packages/clerk-js/sandbox/integration/sign-in.spec.ts
packages/testing/src/common/types.ts
packages/testing/src/common/helpers-utils.ts
packages/testing/src/playwright/helpers.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/sandbox/integration/helpers.ts
packages/clerk-js/sandbox/integration/sign-in.spec.ts
packages/testing/src/common/types.ts
packages/testing/src/common/helpers-utils.ts
packages/testing/src/playwright/helpers.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}
: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidany
type - preferunknown
when type is uncertain, then narrow with type guards
Useinterface
for object shapes that might be extended
Usetype
for unions, primitives, and computed types
Preferreadonly
properties for immutable data structures
Useprivate
for internal implementation details
Useprotected
for inheritance hierarchies
Usepublic
explicitly for clarity in public APIs
Preferreadonly
for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertions
for literal types:as const
Usesatisfies
operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noany
types without justification
Proper error handling with typed errors
Consistent use ofreadonly
for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/clerk-js/sandbox/integration/helpers.ts
packages/clerk-js/sandbox/integration/sign-in.spec.ts
packages/testing/src/common/types.ts
packages/testing/src/common/helpers-utils.ts
packages/testing/src/playwright/helpers.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/sandbox/integration/helpers.ts
packages/clerk-js/sandbox/integration/sign-in.spec.ts
packages/testing/src/common/types.ts
packages/testing/src/common/helpers-utils.ts
packages/testing/src/playwright/helpers.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/sandbox/integration/helpers.ts
packages/clerk-js/sandbox/integration/sign-in.spec.ts
packages/testing/src/common/types.ts
packages/testing/src/common/helpers-utils.ts
packages/testing/src/playwright/helpers.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/sandbox/integration/sign-in.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/sandbox/integration/sign-in.spec.ts
.changeset/**
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Automated releases must use Changesets.
Files:
.changeset/easy-parrots-slide.md
🧬 Code Graph Analysis (3)
packages/clerk-js/sandbox/integration/helpers.ts (2)
packages/clerk-js/bundle-check.mjs (1)
page
(219-219)packages/testing/src/playwright/helpers.ts (1)
clerk
(175-179)
packages/clerk-js/sandbox/integration/sign-in.spec.ts (1)
packages/testing/src/playwright/helpers.ts (1)
clerk
(175-179)
packages/testing/src/common/helpers-utils.ts (2)
packages/types/src/factors.ts (2)
PhoneCodeFactor
(34-41)EmailCodeFactor
(20-25)packages/clerk-js/src/core/resources/Client.ts (1)
signInAttempt
(53-55)
🪛 LanguageTool
.changeset/easy-parrots-slide.md
[grammar] ~4-~4: There might be a mistake here.
Context: --- '@clerk/testing': minor --- Introduce new helper to allow signing a user in v...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (9)
packages/clerk-js/sandbox/integration/helpers.ts (1)
23-37
: Helper function implementation looks goodThe new
signInWithEmail
helper is well-documented with JSDoc comments and follows the established pattern fromsignInWithEmailCode
. The implementation correctly uses the new email-based sign-in API.packages/testing/src/playwright/helpers.ts (3)
1-7
: Good addition of the Clerk backend clientThe import of
createClerkClient
from@clerk/backend
is necessary for the new email-based sign-in functionality that creates sign-in tokens server-side.
19-24
: Well-structured type definition for email-based sign-inThe
PlaywrightClerkSignInParamsWithEmail
type clearly defines the parameters needed for email-based authentication and properly includes optional testing token options.
133-154
: Robust implementation of email-based sign-in with proper error handlingThe implementation correctly:
- Looks up the user by email address
- Creates a short-lived sign-in token (5 minutes)
- Uses the ticket strategy to sign in
- Waits for the user object to be populated
- Provides clear error messages for common failure scenarios
packages/testing/src/common/helpers-utils.ts (5)
15-22
: Password strategy implementation looks good.The password flow is correctly implemented with proper session activation after successful sign-in.
24-38
: Ticket strategy implementation is well-structured.The ticket flow properly handles the sign-in response status and provides clear error messaging when the status is not 'complete'.
40-77
: Phone code validation and implementation are robust.Good implementation with proper test phone number validation using regex pattern. The error message is helpful and includes documentation link. The factor preparation and attempt flow is correctly implemented.
79-116
: Email code validation and implementation are well done.The test email validation checking for '+clerk_test' subaddress is appropriate, with helpful error message and documentation link. The factor preparation and attempt flow mirrors the phone_code implementation correctly.
121-124
: Error wrapping provides good context.The catch block properly wraps errors with a "Clerk: Failed to sign in: " prefix, making debugging easier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.changeset/easy-parrots-slide.md (3)
5-6
: Tighten wording in the summary.Clearer and slightly more idiomatic phrasing.
Apply this diff:
-Introduce new helper to allow signing a user in via email address: +Introduce a new helper to sign in a user via email address:
7-13
: Add the Playwright test import for completeness.Many readers copy/paste examples; including the
test
import prevents confusion.Apply this diff:
```ts -import { clerk } from '@clerk/testing/playwright' +import { test } from '@playwright/test' +import { clerk } from '@clerk/testing/playwright' test('sign in', async ({ page }) => { await clerk.signIn({ emailAddress: '[email protected]', page }) })--- `13-13`: **Document the environment requirement (CLERK_SECRET_KEY).** This helper relies on creating a short-lived ticket server-side; calling this out helps users avoid setup pitfalls. Apply this diff: ```diff })
+Note: This helper requires CLERK_SECRET_KEY to be configured in your test environment, as it creates a short‑lived sign‑in ticket for the specified email.
</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **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 settings in your CodeRabbit configuration. <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 2bb968d945cc04c9b5656f9133e89c2b6d3434ec and 58f2430f07cf80343739ba01941fde56458fba5a. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `.changeset/easy-parrots-slide.md` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>📓 Path-based instructions (1)</summary> <details> <summary>.changeset/**</summary> **📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)** > Automated releases must use Changesets. Files: - `.changeset/easy-parrots-slide.md` </details> </details><details> <summary>🪛 LanguageTool</summary> <details> <summary>.changeset/easy-parrots-slide.md</summary> [grammar] ~4-~4: There might be a mistake here. Context: --- '@clerk/testing': minor --- Introduce new helper to allow signing a user in v... (QB_NEW_EN) </details> </details> </details> <details> <summary>⏰ 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)</summary> * GitHub Check: Formatting | Dedupe | Changeset * GitHub Check: Build Packages * GitHub Check: semgrep/ci * GitHub Check: Analyze (javascript-typescript) * GitHub Check: semgrep-cloud-platform/scan </details> <details> <summary>🔇 Additional comments (2)</summary><blockquote> <details> <summary>.changeset/easy-parrots-slide.md (2)</summary> `1-3`: **Changeset frontmatter looks correct (minor bump for @clerk/testing).** Package name and bump type are valid for releasing this feature via Changesets. --- `7-13`: **Example now correctly awaits the async call.** Thanks for addressing the earlier feedback; the example is now accurate and copy-pasteable. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
…/javascript into brk.feat/testing-sign-in-via-email
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (6)
packages/testing/src/common/setup.ts (6)
41-53
: Do not fetch testing tokens with production secret keys without an explicit opt-inThis path now allows issuing testing tokens whenever a secret key is present. If a production secret key is used, this can inadvertently target production. Recommend gating this behind an explicit opt-in and clear error.
Apply this diff to reintroduce a safety guard and an explicit override:
-import { parsePublishableKey } from '@clerk/shared/keys'; +import { parsePublishableKey, isProductionFromSecretKey } from '@clerk/shared/keys'; @@ if (secretKey && !testingToken) { log('Fetching testing token from Clerk Backend API...'); + // Safety guard: avoid production secret keys unless explicitly allowed + if ( + isProductionFromSecretKey(secretKey) && + !rest?.allowProductionSecretKey && + process.env.CLERK_TESTING_ALLOW_PRODUCTION_SECRET_KEY !== 'true' + ) { + throw new Error( + 'Refusing to fetch a testing token using a production secret key. ' + + 'Pass { allowProductionSecretKey: true } to fetchEnvVars or set CLERK_TESTING_ALLOW_PRODUCTION_SECRET_KEY=true to override (use with extreme caution).', + ); + } + try { - const apiUrl = (rest as any)?.apiUrl || process.env.CLERK_API_URL; - const clerkClient = createClerkClient({ secretKey, apiUrl }); + const apiUrl = (rest as any)?.apiUrl || process.env.CLERK_API_URL; + const clerkClient = createClerkClient({ secretKey, apiUrl }); const tokenData = await clerkClient.testingTokens.createTestingToken(); testingToken = tokenData.token; } catch (err) { console.error('Failed to fetch testing token from Clerk API.'); throw err; } }Additionally update the options type to include the new flag (see separate type change below).
Also applies to: 2-2
8-8
: Avoid(any)
cast by typingapiUrl
on optionsProject guidelines disallow
any
. Type the option and remove the cast.Apply this diff:
- const { debug = false, dotenv: loadDotEnv = true, ...rest } = options || {}; + const { debug = false, dotenv: loadDotEnv = true, apiUrl, ...rest } = options || {}; @@ - const apiUrl = (rest as any)?.apiUrl || process.env.CLERK_API_URL; - const clerkClient = createClerkClient({ secretKey, apiUrl }); + const resolvedApiUrl = apiUrl || process.env.CLERK_API_URL; + const clerkClient = createClerkClient({ secretKey, apiUrl: resolvedApiUrl });Also extend
ClerkSetupOptions
to includeapiUrl?: string
(see type change below).Also applies to: 45-47
7-7
: Add JSDoc for the public APIPublic APIs must be documented. Add a brief description, parameter docs, return type, and thrown errors.
Apply this diff:
+/** + * Sets up Clerk testing environment variables for tooling and E2E helpers. + * - Resolves the publishable key and derives the Frontend API (CLERK_FAPI). + * - Uses a secret key to fetch a short‑lived testing token when one is not provided. + * + * @public + * @param options - Setup options (debug logging, dotenv loading, optional apiUrl, etc.) + * @returns An object with CLERK_FAPI and CLERK_TESTING_TOKEN to be injected into the test runtime. + * @throws If no publishable key is found, or if neither a secret key nor testing token is available. + */ export const fetchEnvVars = async (options?: ClerkSetupOptions): Promise<ClerkSetupReturn> => {
33-35
: ValidateparsePublishableKey
result and fail fast on invalid keysIf the key is malformed,
parsePublishableKey
can yield an undefined/invalidfrontendApi
, returning an unusableCLERK_FAPI
. Fail fast with a clear error.Apply this diff:
if (!publishableKey) { throw new Error('You need to set the CLERK_PUBLISHABLE_KEY environment variable.'); } @@ - return { - CLERK_FAPI: options?.frontendApiUrl || parsePublishableKey(publishableKey)?.frontendApi, - CLERK_TESTING_TOKEN: testingToken, - }; + const parsedPk = parsePublishableKey(publishableKey); + const frontendApi = options?.frontendApiUrl || parsedPk?.frontendApi; + if (!frontendApi) { + throw new Error( + 'Invalid Clerk publishable key format. Ensure a valid CLERK_PUBLISHABLE_KEY (or equivalent env) is set.', + ); + } + return { + CLERK_FAPI: frontendApi, + CLERK_TESTING_TOKEN: testingToken, + };Also applies to: 55-58
1-60
: Follow-up: Type additions to support the above changesAdd the new options to
ClerkSetupOptions
to avoid(any)
and enable the production-key override. This change is outside the current file.Add these fields to
packages/testing/src/common/types.ts
:export interface ClerkSetupOptions { readonly debug?: boolean; readonly dotenv?: boolean; readonly publishableKey?: string; readonly secretKey?: string; readonly frontendApiUrl?: string; // New: readonly apiUrl?: string; readonly allowProductionSecretKey?: boolean; }I can open a follow-up PR to wire this through if helpful.
19-20
: Load .env files sequentially instead of using an arrayThe
path
option fordotenv.config
only accepts a single string, so passing an array will be ignored. To load multiple files, calldotenv.config
for each file in order, or switch to a library likedotenv-flow
for built-in multi-file support.Affected location:
- File: packages/testing/src/common/setup.ts
Lines 19–20Suggested change:
- dotenv.config({ path: ['.env.local', '.env'] }); + // Load local overrides first, then base env + dotenv.config({ path: '.env.local' }); + dotenv.config({ path: '.env' });
🧹 Nitpick comments (1)
packages/testing/src/common/setup.ts (1)
49-52
: Type the caught error and log details for debuggingLog the error details and provide a recovery hint. Avoid untyped catches.
Apply this diff:
- } catch (err) { - console.error('Failed to fetch testing token from Clerk API.'); - throw err; - } + } catch (err: unknown) { + const message = err instanceof Error ? err.message : String(err); + console.error('Failed to fetch testing token from Clerk API.', { message }); + console.error('Hint: Verify CLERK_SECRET_KEY and (optionally) CLERK_API_URL target the correct instance.'); + throw err; + }
📜 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 settings in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/testing/src/common/setup.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/testing/src/common/setup.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/testing/src/common/setup.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/testing/src/common/setup.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/testing/src/common/setup.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}
: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidany
type - preferunknown
when type is uncertain, then narrow with type guards
Useinterface
for object shapes that might be extended
Usetype
for unions, primitives, and computed types
Preferreadonly
properties for immutable data structures
Useprivate
for internal implementation details
Useprotected
for inheritance hierarchies
Usepublic
explicitly for clarity in public APIs
Preferreadonly
for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertions
for literal types:as const
Usesatisfies
operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noany
types without justification
Proper error handling with typed errors
Consistent use ofreadonly
for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/testing/src/common/setup.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/testing/src/common/setup.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/testing/src/common/setup.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: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
Description
Introduce new functionality to our test helper that allows signing in with only an email:
fixes USER-3158
Checklist
pnpm test
runs as expected.pnpm build
runs as expected.Type of change
Summary by CodeRabbit
New Features
Tests
Refactor
Chores