Skip to content

Conversation

dstaley
Copy link
Member

@dstaley dstaley commented Sep 12, 2025

Description

This PR adds support for signing in with an email link to our Signal implementation.

Checklist

  • pnpm test runs as expected.
  • pnpm build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

Summary by CodeRabbit

  • New Features

    • Experimental email-link sign-in: send a sign-in link, wait for verification, and auto-detect completion via URL.
    • Expanded verification details: status (verified, expired, failed, client_mismatch), created session ID, and whether verification came from the same client.
    • React state: signIn.firstFactorVerification and signIn.emailLink (sendLink, waitForVerification, verification).
  • Chores

    • Minor package version bumps to publish the experimental feature.
    • Increased bundle size threshold for legacy browser build.

Copy link

changeset-bot bot commented Sep 12, 2025

🦋 Changeset detected

Latest commit: a0c4aae

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 22 packages
Name Type
@clerk/clerk-js Minor
@clerk/clerk-react Minor
@clerk/types Minor
@clerk/chrome-extension Patch
@clerk/clerk-expo Patch
@clerk/elements Patch
@clerk/nextjs Patch
@clerk/react-router Patch
@clerk/remix Patch
@clerk/tanstack-react-start Patch
@clerk/agent-toolkit Patch
@clerk/astro Patch
@clerk/backend Patch
@clerk/expo-passkeys Patch
@clerk/express Patch
@clerk/fastify Patch
@clerk/localizations Patch
@clerk/nuxt Patch
@clerk/shared Patch
@clerk/testing Patch
@clerk/themes Patch
@clerk/vue Patch

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

Copy link

vercel bot commented Sep 12, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
clerk-js-sandbox Ready Ready Preview Comment Sep 15, 2025 3:08pm

Copy link
Contributor

coderabbitai bot commented Sep 12, 2025

Walkthrough

Adds an experimental email-link sign-in flow: SignInFuture.emailLink (sendLink, waitForVerification, verification), exposes firstFactorVerification, surfaces email-link APIs in the React state proxy, updates types for the new APIs, and adds a changeset bump for three packages.

Changes

Cohort / File(s) Summary
Release metadata
./.changeset/free-rockets-flow.md
Adds a changeset marking minor version bumps for @clerk/clerk-js, @clerk/clerk-react, and @clerk/types with subject "[Experimental] Signal email link support".
Core sign-in workflow (clerk-js)
packages/clerk-js/src/core/resources/SignIn.ts, packages/clerk-js/bundlewatch.config.json
Implements email-link support in SignInFuture: adds emailLink (sendLink, waitForVerification, verification via URL params), sendEmailLink, waitForEmailLinkVerification, firstFactorVerification getter, and __internal_baseGet; finalization now reloads client and activates session when created; updates legacy bundle size threshold.
React state proxy
packages/react/src/stateProxy.ts
Surfaces signIn.firstFactorVerification with gated defaults; adds signIn.emailLink struct exposing sendLink, waitForVerification, and verification getter; introduces private wrapStruct helper and binds gateProperty for new members.
Public types
packages/types/src/signInFuture.ts
Adds SignInFutureEmailLinkSendParams; extends SignInFutureResource with firstFactorVerification and emailLink API; expands verification shape to include status (`'verified'

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant U as User
  participant A as App (React)
  participant S as SignInFuture (clerk-js)
  participant B as Clerk Backend
  participant E as Email Provider

  rect rgb(200,230,255)
    U->>A: Submit email
    A->>S: emailLink.sendLink({ email, redirectUrl })
    S->>B: prepare sign-in (email_link)
    B-->>E: send magic link
    E-->>U: deliver email link
  end

  Note over U,A: User clicks link → redirectUrl with query params

  rect rgb(220,255,220)
    U->>A: Open redirected URL
    A->>S: read emailLink.verification (URL params)
    A->>S: emailLink.waitForVerification()
    S->>B: poll verification status
    B-->>S: { status, createdSessionId, verifiedFromTheSameClient }
    S-->>A: verification result
    A->>S: finalize()
    S->>B: reload client / setActive session
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Pre-merge checks (3 passed)

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title is concise, follows conventional-commit style, and accurately summarizes the primary change — adding email-link sign-in support for the Signal implementation across clerk-js, clerk-react, and types, which makes the PR intent clear to reviewers and in the project history.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Poem

I hop a tiny coded tune,
I chase a link beneath the moon.
I sniff the params, find the lead,
Clicked, verified—sessions freed.
A happy hop, new flows in bloom. 🐇✨

✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ds.feat/signals-email-link

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

pkg-pr-new bot commented Sep 12, 2025

Open in StackBlitz

@clerk/agent-toolkit

npm i https://pkg.pr.new/@clerk/agent-toolkit@6766

@clerk/astro

npm i https://pkg.pr.new/@clerk/astro@6766

@clerk/backend

npm i https://pkg.pr.new/@clerk/backend@6766

@clerk/chrome-extension

npm i https://pkg.pr.new/@clerk/chrome-extension@6766

@clerk/clerk-js

npm i https://pkg.pr.new/@clerk/clerk-js@6766

@clerk/dev-cli

npm i https://pkg.pr.new/@clerk/dev-cli@6766

@clerk/elements

npm i https://pkg.pr.new/@clerk/elements@6766

@clerk/clerk-expo

npm i https://pkg.pr.new/@clerk/clerk-expo@6766

@clerk/expo-passkeys

npm i https://pkg.pr.new/@clerk/expo-passkeys@6766

@clerk/express

npm i https://pkg.pr.new/@clerk/express@6766

@clerk/fastify

npm i https://pkg.pr.new/@clerk/fastify@6766

@clerk/localizations

npm i https://pkg.pr.new/@clerk/localizations@6766

@clerk/nextjs

npm i https://pkg.pr.new/@clerk/nextjs@6766

@clerk/nuxt

npm i https://pkg.pr.new/@clerk/nuxt@6766

@clerk/clerk-react

npm i https://pkg.pr.new/@clerk/clerk-react@6766

@clerk/react-router

npm i https://pkg.pr.new/@clerk/react-router@6766

@clerk/remix

npm i https://pkg.pr.new/@clerk/remix@6766

@clerk/shared

npm i https://pkg.pr.new/@clerk/shared@6766

@clerk/tanstack-react-start

npm i https://pkg.pr.new/@clerk/tanstack-react-start@6766

@clerk/testing

npm i https://pkg.pr.new/@clerk/testing@6766

@clerk/themes

npm i https://pkg.pr.new/@clerk/themes@6766

@clerk/types

npm i https://pkg.pr.new/@clerk/types@6766

@clerk/upgrade

npm i https://pkg.pr.new/@clerk/upgrade@6766

@clerk/vue

npm i https://pkg.pr.new/@clerk/vue@6766

commit: a0c4aae

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (4)
packages/types/src/signInFuture.ts (2)

5-5: Use a type-only import for VerificationResource.

Prevents emitting an unnecessary runtime import.

-import { VerificationResource } from './verification';
+import type { VerificationResource } from './verification';

39-42: Document redirectUrl semantics (absolute vs relative).

Callers need clarity that relative URLs are accepted and resolved against window.location.origin.

-export interface SignInFutureEmailLinkSendParams {
-  email: string;
-  redirectUrl: string;
-}
+/**
+ * Parameters for sending an email link first-factor.
+ *
+ * - redirectUrl: Absolute http(s) URL or a path relative to the current origin (e.g. "/verify").
+ */
+export interface SignInFutureEmailLinkSendParams {
+  email: string;
+  redirectUrl: string;
+}
packages/react/src/stateProxy.ts (1)

48-69: Fix copy/paste error in fallback error message and keep message accurate.

The reload fallback throws with the wrong method name.

             reload: () => {
-              throw new Error('__internal_toSnapshot called before Clerk is loaded');
+              throw new Error('reload called before Clerk is loaded');
             },
packages/clerk-js/src/core/resources/SignIn.ts (1)

755-774: Stop polling on failure as well and reuse reload for clarity.

Poll until verified, expired, or failed/client_mismatch; reload keeps instance state consistent.

   async waitForEmailLinkVerification(): Promise<{ error: unknown }> {
     return runAsyncResourceTask(this.resource, async () => {
       const { run, stop } = Poller();
       await new Promise((resolve, reject) => {
         void run(async () => {
           try {
-            const res = await this.resource.__internal_baseGet();
+            const res = await this.resource.reload();
             const status = res.firstFactorVerification.status;
-            if (status === 'verified' || status === 'expired') {
+            if (status === 'verified' || status === 'expired' || status === 'failed') {
               stop();
               resolve(res);
             }
           } catch (err) {
             stop();
             reject(err);
           }
         });
       });
     });
   }
📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 526d505 and f0a0a23.

📒 Files selected for processing (4)
  • .changeset/free-rockets-flow.md (1 hunks)
  • packages/clerk-js/src/core/resources/SignIn.ts (8 hunks)
  • packages/react/src/stateProxy.ts (3 hunks)
  • packages/types/src/signInFuture.ts (4 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
.changeset/**

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Automated releases must use Changesets.

Files:

  • .changeset/free-rockets-flow.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/types/src/signInFuture.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
  • packages/react/src/stateProxy.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/types/src/signInFuture.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
  • packages/react/src/stateProxy.ts
packages/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

TypeScript is required for all packages

Files:

  • packages/types/src/signInFuture.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
  • packages/react/src/stateProxy.ts
packages/**/*.{ts,tsx,d.ts}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Packages should export TypeScript types alongside runtime code

Files:

  • packages/types/src/signInFuture.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
  • packages/react/src/stateProxy.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
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly 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
Use const assertions for literal types: as const
Use satisfies 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 ...
No any types without justification
Proper error handling with typed errors
Consistent use of readonly 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/types/src/signInFuture.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
  • packages/react/src/stateProxy.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.

Files:

  • packages/types/src/signInFuture.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
  • packages/react/src/stateProxy.ts
🧬 Code graph analysis (2)
packages/types/src/signInFuture.ts (1)
packages/types/src/verification.ts (1)
  • VerificationResource (7-20)
packages/clerk-js/src/core/resources/SignIn.ts (4)
packages/clerk-js/src/utils/runtime.ts (1)
  • inBrowser (1-3)
packages/clerk-js/src/utils/getClerkQueryParam.ts (1)
  • getClerkQueryParam (37-40)
packages/types/src/signInFuture.ts (1)
  • SignInFutureEmailLinkSendParams (39-42)
packages/clerk-js/src/utils/runAsyncResourceTask.ts (1)
  • runAsyncResourceTask (8-30)
⏰ 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: semgrep-cloud-platform/scan
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
.changeset/free-rockets-flow.md (1)

1-7: Changeset looks good.

Minor bumps for js/react/types match a non-breaking, experimental addition. Title is clear.

packages/react/src/stateProxy.ts (1)

179-198: wrapStruct helper looks solid.

Nice generic typing and property gating; reads clean and reusable.

packages/clerk-js/src/core/resources/SignIn.ts (1)

146-153: Exposing __internal_baseGet is fine, but keep internal surface consistent.

Matches existing __internal_basePost pattern. No issues spotted.

Comment on lines 553 to 579
emailLink = {
sendLink: this.sendEmailLink.bind(this),
waitForVerification: this.waitForEmailLinkVerification.bind(this),
get verification() {
if (!inBrowser()) {
return null;
}

const status = getClerkQueryParam('__clerk_status') as 'verified' | 'expired' | 'failed' | 'client_mismatch';
const createdSessionID = getClerkQueryParam('__clerk_created_session');

if (!status || !createdSessionID) {
return null;
}

const verifiedFromTheSameClient =
status === 'verified' &&
typeof SignIn.clerk.client !== 'undefined' &&
SignIn.clerk.client.sessions.some(s => s.id === createdSessionID);

return {
status,
createdSessionID,
verifiedFromTheSameClient,
};
},
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Align verification field casing and minor cleanup.

  • Use createdSessionId (lowercase d) to match the rest of the API.
  • Minor local rename for consistency.
   emailLink = {
     sendLink: this.sendEmailLink.bind(this),
     waitForVerification: this.waitForEmailLinkVerification.bind(this),
     get verification() {
       if (!inBrowser()) {
         return null;
       }
 
-      const status = getClerkQueryParam('__clerk_status') as 'verified' | 'expired' | 'failed' | 'client_mismatch';
-      const createdSessionID = getClerkQueryParam('__clerk_created_session');
+      const status = getClerkQueryParam('__clerk_status') as
+        | 'verified'
+        | 'expired'
+        | 'failed'
+        | 'client_mismatch';
+      const createdSessionId = getClerkQueryParam('__clerk_created_session');
 
-      if (!status || !createdSessionID) {
+      if (!status || !createdSessionId) {
         return null;
       }
 
       const verifiedFromTheSameClient =
         status === 'verified' &&
         typeof SignIn.clerk.client !== 'undefined' &&
-        SignIn.clerk.client.sessions.some(s => s.id === createdSessionID);
+        SignIn.clerk.client.sessions.some(s => s.id === createdSessionId);
 
       return {
         status,
-        createdSessionID,
+        createdSessionId,
         verifiedFromTheSameClient,
       };
     },
   };

After applying, run:


🏁 Script executed:

#!/bin/bash
rg -n -C2 '\bcreatedSessionID\b' packages | cat

Length of output: 1632


Rename createdSessionID → createdSessionId and update types

  • Replace createdSessionID with createdSessionId in:
    • packages/clerk-js/src/core/resources/SignIn.ts (local variable and returned field)
    • packages/types/src/signInFuture.ts (exported property)
  • This is a public/type-level name change — update callers, tests and docs, and run a repo-wide search to ensure no remaining createdSessionID references.
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/resources/SignIn.ts around lines 553 to 579,
rename the local variable and returned field createdSessionID to
createdSessionId and update its TypeScript type usage accordingly; also update
the exported property name in packages/types/src/signInFuture.ts to
createdSessionId. Change every reference in this block (assignment from
getClerkQueryParam, the verifiedFromTheSameClient check, and the returned
object) to use createdSessionId, update the signInFuture type definition to
match, then run a repo-wide search/replace for createdSessionID →
createdSessionId to update callers, tests and docs so all usages and imports
align.

Comment on lines 726 to 753
async sendEmailLink(params: SignInFutureEmailLinkSendParams): Promise<{ error: unknown }> {
const { email, redirectUrl } = params;
return runAsyncResourceTask(this.resource, async () => {
if (!this.resource.id) {
await this.create({ identifier: email });
}

const emailLinkFactor = this.resource.supportedFirstFactors?.find(f => f.strategy === 'email_link');

if (!emailLinkFactor) {
throw new Error('Email code factor not found');
}

const { emailAddressId } = emailLinkFactor;

let verifyUrl = redirectUrl;
try {
new URL(verifyUrl);
} catch (err) {
verifyUrl = window.location.origin + verifyUrl;
}

await this.resource.__internal_basePost({
body: { emailAddressId, redirectUrl: verifyUrl, strategy: 'email_link' },
action: 'prepare_first_factor',
});
});
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Validate and normalize redirectUrl; fix message typo.

  • Security: Allow only http(s) absolute URLs or resolve relative paths against the current origin. Reject other schemes.
  • Robust join: new URL(relativeOrAbsolute, window.location.origin) avoids missing slashes.
  • Message: Should say "Email link factor not found".
   async sendEmailLink(params: SignInFutureEmailLinkSendParams): Promise<{ error: unknown }> {
     const { email, redirectUrl } = params;
     return runAsyncResourceTask(this.resource, async () => {
       if (!this.resource.id) {
         await this.create({ identifier: email });
       }
 
       const emailLinkFactor = this.resource.supportedFirstFactors?.find(f => f.strategy === 'email_link');
 
       if (!emailLinkFactor) {
-        throw new Error('Email code factor not found');
+        throw new Error('Email link factor not found');
       }
 
       const { emailAddressId } = emailLinkFactor;
 
-      let verifyUrl = redirectUrl;
-      try {
-        new URL(verifyUrl);
-      } catch (err) {
-        verifyUrl = window.location.origin + verifyUrl;
-      }
+      // Normalize and validate redirectUrl
+      const url = new URL(redirectUrl, window.location.origin);
+      if (url.protocol !== 'http:' && url.protocol !== 'https:') {
+        throw new Error('redirectUrl must be an http(s) URL or a relative path');
+      }
+      const verifyUrl = url.toString();
 
       await this.resource.__internal_basePost({
         body: { emailAddressId, redirectUrl: verifyUrl, strategy: 'email_link' },
         action: 'prepare_first_factor',
       });
     });
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async sendEmailLink(params: SignInFutureEmailLinkSendParams): Promise<{ error: unknown }> {
const { email, redirectUrl } = params;
return runAsyncResourceTask(this.resource, async () => {
if (!this.resource.id) {
await this.create({ identifier: email });
}
const emailLinkFactor = this.resource.supportedFirstFactors?.find(f => f.strategy === 'email_link');
if (!emailLinkFactor) {
throw new Error('Email code factor not found');
}
const { emailAddressId } = emailLinkFactor;
let verifyUrl = redirectUrl;
try {
new URL(verifyUrl);
} catch (err) {
verifyUrl = window.location.origin + verifyUrl;
}
await this.resource.__internal_basePost({
body: { emailAddressId, redirectUrl: verifyUrl, strategy: 'email_link' },
action: 'prepare_first_factor',
});
});
}
async sendEmailLink(params: SignInFutureEmailLinkSendParams): Promise<{ error: unknown }> {
const { email, redirectUrl } = params;
return runAsyncResourceTask(this.resource, async () => {
if (!this.resource.id) {
await this.create({ identifier: email });
}
const emailLinkFactor = this.resource.supportedFirstFactors?.find(f => f.strategy === 'email_link');
if (!emailLinkFactor) {
throw new Error('Email link factor not found');
}
const { emailAddressId } = emailLinkFactor;
// Normalize and validate redirectUrl
const url = new URL(redirectUrl, window.location.origin);
if (url.protocol !== 'http:' && url.protocol !== 'https:') {
throw new Error('redirectUrl must be an http(s) URL or a relative path');
}
const verifyUrl = url.toString();
await this.resource.__internal_basePost({
body: { emailAddressId, redirectUrl: verifyUrl, strategy: 'email_link' },
action: 'prepare_first_factor',
});
});
}
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/resources/SignIn.ts around lines 726 to 753, the
code must validate and normalize redirectUrl and fix the error message: replace
the typo'd error with "Email link factor not found"; normalize the redirectUrl
by constructing it with new URL(redirectUrl, window.location.origin) so relative
paths resolve correctly; after parsing, ensure the resulting URL.protocol is
either "http:" or "https:" and throw a clear error if not; use the validated
URL.toString() (or href) as the redirect value when calling __internal_basePost.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

♻️ Duplicate comments (3)
packages/clerk-js/src/core/resources/SignIn.ts (3)

553-579: Rename createdSessionID → createdSessionId for consistency.
Follows camelCase across the codebase and prior comments.

Apply:

-      const createdSessionID = getClerkQueryParam('__clerk_created_session');
+      const createdSessionId = getClerkQueryParam('__clerk_created_session');

-      if (!status || !createdSessionID) {
+      if (!status || !createdSessionId) {
         return null;
       }

       const verifiedFromTheSameClient =
         status === 'verified' &&
         typeof SignIn.clerk.client !== 'undefined' &&
-        SignIn.clerk.client.sessions.some(s => s.id === createdSessionID);
+        SignIn.clerk.client.sessions.some(s => s.id === createdSessionId);

       return {
         status,
-        createdSessionID,
+        createdSessionId,
         verifiedFromTheSameClient,
       };

Also update the corresponding type in packages/types to use createdSessionId.


877-891: Replace console.log with debugLogger.
Console logs will fail linting and are noisy in production.

-    console.log('finalize', this.resource.createdSessionId);
+    debugLogger.debug('SignIn.finalize', { sessionId: this.resource.createdSessionId });
     const { navigate } = params || {};
     return runAsyncResourceTask(this.resource, async () => {
-      console.log('run async finalize', this.resource.createdSessionId);
+      debugLogger.debug('SignIn.finalize: start', { sessionId: this.resource.createdSessionId });
       if (!this.resource.createdSessionId) {
-        console.log('no created session id');
+        debugLogger.debug('SignIn.finalize: missing createdSessionId');
         throw new Error('Cannot finalize sign-in without a created session.');
       }
 
-      console.log('reloading client');
-      await SignIn.clerk.client?.reload();
-
-      console.log('calling setActive');
+      debugLogger.debug('SignIn.finalize: reloading client');
+      await SignIn.clerk.client?.reload();
+      debugLogger.debug('SignIn.finalize: calling setActive');
       await SignIn.clerk.setActive({ session: this.resource.createdSessionId, navigate });
     });

726-753: Validate/normalize redirectUrl and fix error string; make SSR-safe.

  • Message should say “Email link factor not found”.
  • Current URL handling can: (a) drop the leading slash, (b) accept non-http(s) schemes, (c) throw on SSR due to window usage.

Apply:

   async sendEmailLink(params: SignInFutureEmailLinkSendParams): Promise<{ error: unknown }> {
     const { email, redirectUrl } = params;
     return runAsyncResourceTask(this.resource, async () => {
       if (!this.resource.id) {
         await this.create({ identifier: email });
       }

       const emailLinkFactor = this.resource.supportedFirstFactors?.find(f => f.strategy === 'email_link');

       if (!emailLinkFactor) {
-        throw new Error('Email code factor not found');
+        throw new Error('Email link factor not found');
       }

       const { emailAddressId } = emailLinkFactor;

-      let verifyUrl = redirectUrl;
-      try {
-        new URL(verifyUrl);
-      } catch {
-        verifyUrl = window.location.origin + verifyUrl;
-      }
+      // Normalize and validate redirectUrl. Allow http(s) absolute URLs or relative paths (in browser).
+      let verifyUrl: string;
+      if (inBrowser()) {
+        const url = new URL(redirectUrl, window.location.origin);
+        if (url.protocol !== 'http:' && url.protocol !== 'https:') {
+          throw new Error('redirectUrl must be an http(s) URL or a relative path');
+        }
+        verifyUrl = url.toString();
+      } else {
+        // SSR: require absolute http(s) URL
+        const url = new URL(redirectUrl);
+        if (url.protocol !== 'http:' && url.protocol !== 'https:') {
+          throw new Error('redirectUrl must be an absolute http(s) URL when running outside the browser');
+        }
+        verifyUrl = url.toString();
+      }

       await this.resource.__internal_basePost({
         body: { emailAddressId, redirectUrl: verifyUrl, strategy: 'email_link' },
         action: 'prepare_first_factor',
       });
     });
   }

Consider adding brief JSDoc for sendEmailLink describing params and validation.

🧹 Nitpick comments (2)
packages/clerk-js/src/core/resources/SignIn.ts (2)

146-153: Reconsider exposing raw __internal_baseGet; prefer reload() where used.
You added __internal_baseGet but later call it with no args. Unless _baseGet has a sane default path, this is brittle. Using resource.reload() is clearer and already used elsewhere.

If you keep __internal_baseGet, please confirm its call signature and default path are correct for SignIn.


755-774: Poll using reload(); treat failed as terminal.
Using reload() aligns with existing patterns and avoids ambiguity of __internal_baseGet(). Also stop on failed to prevent indefinite polling.

   async waitForEmailLinkVerification(): Promise<{ error: unknown }> {
     return runAsyncResourceTask(this.resource, async () => {
       const { run, stop } = Poller();
       await new Promise((resolve, reject) => {
         void run(async () => {
           try {
-            const res = await this.resource.__internal_baseGet();
+            const res = await this.resource.reload();
             const status = res.firstFactorVerification.status;
-            if (status === 'verified' || status === 'expired') {
+            if (status === 'verified' || status === 'expired' || status === 'failed') {
               stop();
               resolve(res);
             }
           } catch (err) {
             stop();
             reject(err);
           }
         });
       });
     });
   }

Please confirm that firstFactorVerification.status can be 'failed' for email_link; if not, keep only the terminal statuses your backend emits.

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between aa9a320 and b0dce13.

📒 Files selected for processing (1)
  • packages/clerk-js/src/core/resources/SignIn.ts (8 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels

Files:

  • packages/clerk-js/src/core/resources/SignIn.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use Prettier for consistent code formatting

Files:

  • packages/clerk-js/src/core/resources/SignIn.ts
packages/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

TypeScript is required for all packages

Files:

  • packages/clerk-js/src/core/resources/SignIn.ts
packages/**/*.{ts,tsx,d.ts}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Packages should export TypeScript types alongside runtime code

Files:

  • packages/clerk-js/src/core/resources/SignIn.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
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly 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
Use const assertions for literal types: as const
Use satisfies 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 ...
No any types without justification
Proper error handling with typed errors
Consistent use of readonly for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)

Files:

  • packages/clerk-js/src/core/resources/SignIn.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.

Files:

  • packages/clerk-js/src/core/resources/SignIn.ts
🧬 Code graph analysis (1)
packages/clerk-js/src/core/resources/SignIn.ts (4)
packages/clerk-js/src/utils/runtime.ts (1)
  • inBrowser (1-3)
packages/clerk-js/src/utils/getClerkQueryParam.ts (1)
  • getClerkQueryParam (37-40)
packages/types/src/signInFuture.ts (1)
  • SignInFutureEmailLinkSendParams (39-42)
packages/clerk-js/src/utils/runAsyncResourceTask.ts (1)
  • runAsyncResourceTask (8-30)
⏰ 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: Analyze (javascript-typescript)
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (3)
packages/clerk-js/src/core/resources/SignIn.ts (3)

35-35: Type-only import for new params looks correct.
Consistent with project style.


66-66: Importing getClerkQueryParam is appropriate here.
Matches the browser-only usage guarded below.


625-627: Expose firstFactorVerification on SignInFuture — LGTM.
Useful for custom flows.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

♻️ Duplicate comments (2)
packages/types/src/signInFuture.ts (1)

142-176: Add public API JSDoc for emailLink; status alias optional.

  • Briefly describe the flow and when verification is non-null.
  • Optional: introduce a reusable EmailLinkVerificationStatus type to avoid repeating the union.
   /**
-   *
+   * Email link sign-in flow helpers.
+   * - `sendLink` initiates email-link delivery.
+   * - `waitForVerification` polls until verification is `verified` or `expired`.
+   * - `verification` reflects the result in-browser (reads query params); `null` outside the browser or until available.
    */
   emailLink: {
@@
-    /**
-     * The verification status
-     */
+    /**
+     * The verification status and context, when available.
+     */
     verification: {
       /**
        * The verification status
        */
-      status: 'verified' | 'expired' | 'failed' | 'client_mismatch';
+      status: 'verified' | 'expired' | 'failed' | 'client_mismatch';
@@
       verifiedFromTheSameClient: boolean;
     } | null;
   };
#!/bin/bash
# Ensure no lingering createdSessionID casing remains
rg -n -C1 --hidden --no-ignore -S '\bcreatedSessionID\b' || echo "OK: no createdSessionID found"
packages/clerk-js/src/core/resources/SignIn.ts (1)

726-753: Fix redirectUrl normalization and enforce http(s) scheme.

Current concatenation breaks on relative paths without a leading slash and allows non-http(s) schemes. Normalize with URL(base) and validate protocol.

-      let verifyUrl = redirectUrl;
-      try {
-        new URL(verifyUrl);
-      } catch {
-        verifyUrl = window.location.origin + verifyUrl;
-      }
+      const url = new URL(redirectUrl, window.location.origin);
+      if (url.protocol !== 'http:' && url.protocol !== 'https:') {
+        throw new Error('redirectUrl must be an http(s) URL or a relative path');
+      }
+      const verifyUrl = url.toString();
🧹 Nitpick comments (3)
packages/types/src/signInFuture.ts (1)

39-42: Document params and clarify redirectUrl semantics.

Add concise JSDoc so developers know acceptable values (absolute http(s) URL or relative path).

 export interface SignInFutureEmailLinkSendParams {
-  email: string;
-  redirectUrl: string;
+  /**
+   * Email address to send the sign-in link to.
+   */
+  email: string;
+  /**
+   * Destination to redirect the user after clicking the link.
+   * Must be an absolute http(s) URL or a relative path that will be resolved against the current origin.
+   */
+  redirectUrl: string;
 }
packages/clerk-js/src/core/resources/SignIn.ts (2)

553-579: Harden status parsing; avoid blind cast.

Validate __clerk_status against the allowed set before using it.

-      const status = getClerkQueryParam('__clerk_status') as 'verified' | 'expired' | 'failed' | 'client_mismatch';
+      const statusParam = getClerkQueryParam('__clerk_status');
       const createdSessionId = getClerkQueryParam('__clerk_created_session');
 
-      if (!status || !createdSessionId) {
+      const allowed = new Set(['verified', 'expired', 'failed', 'client_mismatch'] as const);
+      if (!statusParam || !allowed.has(statusParam as any) || !createdSessionId) {
         return null;
       }
+      const status = statusParam as 'verified' | 'expired' | 'failed' | 'client_mismatch';

755-774: Allow cancellation/timeout for polling.

waitForEmailLinkVerification can hang indefinitely. Consider accepting an optional AbortSignal or timeout to stop the poller.

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between b0dce13 and e80fb61.

📒 Files selected for processing (2)
  • packages/clerk-js/src/core/resources/SignIn.ts (8 hunks)
  • packages/types/src/signInFuture.ts (4 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels

Files:

  • packages/types/src/signInFuture.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use Prettier for consistent code formatting

Files:

  • packages/types/src/signInFuture.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
packages/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

TypeScript is required for all packages

Files:

  • packages/types/src/signInFuture.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
packages/**/*.{ts,tsx,d.ts}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Packages should export TypeScript types alongside runtime code

Files:

  • packages/types/src/signInFuture.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development.mdc)

Use proper TypeScript error types

**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly 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
Use const assertions for literal types: as const
Use satisfies 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 ...
No any types without justification
Proper error handling with typed errors
Consistent use of readonly 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/types/src/signInFuture.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)

Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.

Files:

  • packages/types/src/signInFuture.ts
  • packages/clerk-js/src/core/resources/SignIn.ts
🧬 Code graph analysis (2)
packages/types/src/signInFuture.ts (1)
packages/types/src/verification.ts (1)
  • VerificationResource (7-20)
packages/clerk-js/src/core/resources/SignIn.ts (4)
packages/clerk-js/src/utils/runtime.ts (1)
  • inBrowser (1-3)
packages/clerk-js/src/utils/getClerkQueryParam.ts (1)
  • getClerkQueryParam (37-40)
packages/types/src/signInFuture.ts (1)
  • SignInFutureEmailLinkSendParams (39-42)
packages/clerk-js/src/utils/runAsyncResourceTask.ts (1)
  • runAsyncResourceTask (8-30)
⏰ 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 (7)
packages/types/src/signInFuture.ts (2)

5-5: Type-only import is correct.

Good use of import type for VerificationResource.


114-114: Expose firstFactorVerification on the surface.

Type and mutability look right; aligns with runtime getter.

packages/clerk-js/src/core/resources/SignIn.ts (5)

1-1: Browser gating import is appropriate.

inBrowser correctly guards the verification getter.


35-36: Type import alignment.

Correctly imports SignInFutureEmailLinkSendParams from types.


66-66: Query param helper import looks good.

Matches usage in the verification getter.


146-153: Expose __internal_baseGet for the wrapper.

Matches the pattern of __internal_basePost; keep @internal JSDoc to avoid public exposure in docs.


883-885: Client reload before setActive is sensible.

Prevents missing the newly created session; matches the comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants