-
Notifications
You must be signed in to change notification settings - Fork 381
fix(clerk-js,types): Update signUp.password
to allow for emailAddress and phoneNumber params
#6698
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
…ss and phoneNumber params
🦋 Changeset detectedLatest commit: 1239490 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis PR updates sign-up parameter handling. It removes mutual exclusivity between email and phone in the password flow, adds optional username support, and introduces an optional channel for phone code sending. Type definitions are updated accordingly. A changeset metadata file is added. No API endpoints or exports are removed. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as Client SDK (SignUpFuture)
participant API as Backend API
Note over UI: Password strategy sign-up
User->>UI: Provide password (+ email and/or phone and/or username)
UI->>UI: Build body with strategy=password<br/>+ captcha fields<br/>+ optional email/phone/username
UI->>API: POST /signups/password
alt Success
API-->>UI: 200 Created / updated sign-up state
UI-->>User: Continue sign-up flow
else Error
API-->>UI: 4xx/5xx with error
UI-->>User: Show error
end
sequenceDiagram
actor User
participant UI as Client SDK (SignUpFuture)
participant API as Backend API
Note over UI: Send phone code with optional channel
User->>UI: Request phone code (phoneNumber?, channel?)
UI->>API: POST /signups/phone_code_send { phoneNumber, channel? }
API-->>UI: Response
UI-->>User: Prompt for verification code
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 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
Status, Documentation and Community
|
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/types/src/signUpFuture.ts (1)
90-94
: Docstring is now inaccurate; update to reflect phoneNumber/both.Adjust the method’s JSDoc to avoid misleading consumers.
/** - * Used to sign up using an email address and password. + * Used to sign up with a password and an identifier. + * Supports `{ password, emailAddress }`, `{ password, phoneNumber }`, or `{ password, emailAddress, phoneNumber }`. */ password: (params: SignUpFuturePasswordParams) => Promise<{ error: unknown }>;
🧹 Nitpick comments (1)
packages/types/src/signUpFuture.ts (1)
12-14
: Add JSDoc and consider a readability utility (RequireAtLeastOne).
- Add JSDoc for this public type per guidelines.
- Optional: Replace the union-of-two-shapes with a reusable AtLeastOne utility for clarity.
+/** + * Parameters for `SignUpFuture.password`. + * - `password` is required. + * - At least one of `emailAddress` or `phoneNumber` must be provided; both are allowed. + * - Allowed call forms: + * - `{ password, emailAddress }` + * - `{ password, phoneNumber }` + * - `{ password, emailAddress, phoneNumber }` + */ export type SignUpFuturePasswordParams = { password: string; } & ({ emailAddress: string; phoneNumber?: string } | { phoneNumber: string; emailAddress?: string });Optional alternative (if a utility exists or you’re willing to add one in
packages/types/src/utils.ts
):+// type RequireAtLeastOne<T, Keys extends keyof T = keyof T> = +// Keys extends keyof T ? (Required<Pick<T, Keys>> & Partial<Omit<T, Keys>>) : never; +// type AtLeastOneIdentifier = RequireAtLeastOne<{ emailAddress?: string; phoneNumber?: string }, 'emailAddress' | 'phoneNumber'>; - export type SignUpFuturePasswordParams = { - password: string; - } & ({ emailAddress: string; phoneNumber?: string } | { phoneNumber: string; emailAddress?: string }); + export type SignUpFuturePasswordParams = { password: string } & AtLeastOneIdentifier;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.changeset/tiny-spoons-unite.md
(1 hunks)packages/clerk-js/src/core/resources/SignUp.ts
(0 hunks)packages/types/src/signUpFuture.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- packages/clerk-js/src/core/resources/SignUp.ts
🧰 Additional context used
📓 Path-based instructions (7)
.changeset/**
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Automated releases must use Changesets.
Files:
.changeset/tiny-spoons-unite.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/signUpFuture.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/signUpFuture.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/types/src/signUpFuture.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/signUpFuture.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/types/src/signUpFuture.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/signUpFuture.ts
🪛 LanguageTool
.changeset/tiny-spoons-unite.md
[grammar] ~1-~1: Hier könnte ein Fehler sein.
Context: --- ---
(QB_NEW_DE)
⏰ 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 (3)
packages/types/src/signUpFuture.ts (3)
12-14
: Type change correctly allows both identifiers and enforces at least one.This shape is sound and matches the PR intent (password required; emailAddress and/or phoneNumber allowed).
12-14
: Add compile-time type tests for allowed/denied shapes.Recommend tsd tests to lock this API:
// packages/types/test/signUpFuture-password.test-d.ts import { expectType } from 'tsd'; import type { SignUpFuturePasswordParams } from '../src/signUpFuture'; declare function accepts(p: SignUpFuturePasswordParams): void; accepts({ password: 'p', emailAddress: 'a@b.com' }); accepts({ password: 'p', phoneNumber: '+15555550123' }); accepts({ password: 'p', emailAddress: 'a@b.com', phoneNumber: '+15555550123' }); // @ts-expect-error - missing identifier accepts({ password: 'p' }); // @ts-expect-error - missing password accepts({ emailAddress: 'a@b.com' });
12-14
: I’m running a deeper grep to inspect both thecreate
andpassword
methods for any exclusivity guards.
--- | ||
--- |
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.
💡 Verification agent
🧩 Analysis chain
Empty changeset frontmatter/body will break release automation.
Populate the affected packages and a concise summary. Example:
---
----
+ '@clerk/clerk-js': patch
+ '@clerk/types': patch
---
+
+fix(types,clerk-js): signUp.password accepts { password } with emailAddress and/or phoneNumber. `password` remains required. Documents allowed call forms.
Run to confirm actual package names before committing:
🏁 Script executed:
#!/bin/bash
fd package.json packages -x jq -r '.name + " -> " + (try .version catch "no version")' {}
Length of output: 922
🏁 Script executed:
#!/bin/bash
set -e
# Ensure we have the latest main branch to diff against
git fetch origin main
# Identify changed package directories
changed_dirs=$(git diff --name-only origin/main...HEAD | awk -F/ '/^packages\// {print $2}' | sort -u)
# Output the package names for each changed directory
for dir in $changed_dirs; do
pkg_json="packages/$dir/package.json"
if [ -f "$pkg_json" ]; then
jq -r .name "$pkg_json"
fi
done
Length of output: 581
Populate changeset frontmatter in .changeset/tiny-spoons-unite.md
Add the two affected packages and a concise summary:
---
'@clerk/clerk-js': patch
'@clerk/types': patch
---
fix(types,clerk-js): signUp.password accepts { password } with emailAddress and/or phoneNumber. `password` remains required. Documents allowed call forms.
🧰 Tools
🪛 LanguageTool
[grammar] ~1-~1: Hier könnte ein Fehler sein.
Context: --- ---
(QB_NEW_DE)
🤖 Prompt for AI Agents
In .changeset/tiny-spoons-unite.md around lines 1 to 2, the changeset
frontmatter is empty; add YAML frontmatter listing the two affected packages and
a brief summary. Populate the top with "'@clerk/clerk-js': patch" and
"'@clerk/types': patch" under --- delimiters, then add the provided one-line
summary describing the fix for signUp.password (keep password required and
document allowed call forms) in the body.
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.
Done ✅ |
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/types/src/signUpFuture.ts (1)
95-99
: Update public API JSDoc: email/phone/username now allowed (and email+phone together).Current text says “email address and password”. Please reflect the new accepted params and examples.
- /** - * Used to sign up using an email address and password. - */ + /** + * Sign up with a password plus at least one identifier. + * Allowed forms: + * - { password, emailAddress } + * - { password, phoneNumber } + * - { password, username } + * - { password, emailAddress, phoneNumber } + * - Any combination where password is present and at least one of emailAddress/phoneNumber/username is provided. + */packages/clerk-js/src/core/resources/SignUp.ts (2)
609-619
: Bug: truthy checks drop intentionally empty strings (allowed by API).Using
if (params.emailAddress)
/if (params.phoneNumber)
/if (params.username)
skips empty-string values, conflicting with the PR’s allowed call forms that show '' as valid placeholders.- if (params.phoneNumber) { + if (params.phoneNumber !== undefined) { body.phoneNumber = params.phoneNumber; } - if (params.emailAddress) { + if (params.emailAddress !== undefined) { body.emailAddress = params.emailAddress; } - if (params.username) { + if (params.username !== undefined) { body.username = params.username; }
644-657
: Input validation: require phoneNumber when creating a new resource before sending phone code.If
this.resource.id
is missing andphoneNumber
is not provided, the first POST creates a sign-up without a phone number, and the subsequent prepare step likely fails.async sendPhoneCode(params: SignUpFuturePhoneCodeSendParams): Promise<{ error: unknown }> { const { phoneNumber, channel = 'sms' } = params; return runAsyncResourceTask(this.resource, async () => { if (!this.resource.id) { + if (phoneNumber === undefined) { + throw new ClerkRuntimeError('phoneNumber is required when initiating phone verification without an existing sign-up.'); + } const { captchaToken, captchaWidgetType, captchaError } = await this.getCaptchaToken(); await this.resource.__internal_basePost({ path: this.resource.pathRoot, body: { phoneNumber, captchaToken, captchaWidgetType, captchaError }, }); } await this.resource.__internal_basePost({ body: { strategy: 'phone_code', channel }, action: 'prepare_verification', });
🧹 Nitpick comments (3)
packages/types/src/signUpFuture.ts (2)
13-19
: Stop dropping docs; consider simpler, maintainable type for “at least one identifier”.Good widening. Add JSDoc and consider replacing the verbose union with an AtLeastOne utility for maintainability.
Proposed change within this block:
-export type SignUpFuturePasswordParams = { - password: string; -} & ( - | { emailAddress: string; phoneNumber?: string; username?: string } - | { emailAddress?: string; phoneNumber: string; username?: string } - | { emailAddress?: string; phoneNumber?: string; username: string } -); +export type SignUpFuturePasswordParams = { + /** + * Required in all forms. + */ + password: string; +} & AtLeastOne<{ + /** + * Optional but at least one of emailAddress/phoneNumber/username must be provided. + */ + emailAddress: string; + phoneNumber: string; + username: string; +}>;Add this utility (once per types package, or reuse if it already exists):
/** * Require at least one key of T to be present. */ export type AtLeastOne<T> = { [K in keyof T]-?: Required<Pick<T, K>> & Partial<Omit<T, K>>; }[keyof T];
21-24
: Document the new “channel” parameter and its default.Add JSDoc to clarify valid values and default behavior (e.g., 'sms' if omitted). Keeps public API self-explanatory.
export interface SignUpFuturePhoneCodeSendParams { - phoneNumber?: string; - channel?: PhoneCodeChannel; + /** + * Phone number to verify. If omitted, must already exist on the sign-up resource. + */ + phoneNumber?: string; + /** + * Delivery channel for the code (e.g., 'sms', 'whatsapp'). Defaults to 'sms'. + */ + channel?: PhoneCodeChannel; }packages/clerk-js/src/core/resources/SignUp.ts (1)
617-620
: Confirm server support and update docs for username.Username wasn’t noted in the PR summary. Please confirm FAPI accepts
username
in the password flow and ensure public docs/types examples reflect this.Happy to prepare a changeset + docs snippet once confirmed.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/clerk-js/src/core/resources/SignUp.ts
(1 hunks)packages/types/src/signUpFuture.ts
(1 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/SignUp.ts
packages/types/src/signUpFuture.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/SignUp.ts
packages/types/src/signUpFuture.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/core/resources/SignUp.ts
packages/types/src/signUpFuture.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/SignUp.ts
packages/types/src/signUpFuture.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/src/core/resources/SignUp.ts
packages/types/src/signUpFuture.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/SignUp.ts
packages/types/src/signUpFuture.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
Description
Allowed params:
password
is always required.Checklist
pnpm test
runs as expected.pnpm build
runs as expected.Type of change
Summary by CodeRabbit