-
Notifications
You must be signed in to change notification settings - Fork 397
feat(clerk-js,types): Add signals support for passkeys #6997
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: dae144f The changes in this PR will be included in the next version bump. This PR includes changesets to release 22 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds passkey (WebAuthn) sign-in support: a new SignInFuture.passkey() runtime flow and corresponding type surface changes, plus a changeset bump marking experimental passkey support for Clerk packages. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant App as Client (SignInFuture)
participant Clerk as Clerk Backend
participant WebAuthn as Browser WebAuthn
User->>App: initiate sign-in with passkey
App->>App: verify WebAuthn availability
App->>Clerk: create/prepare passkey factor (include flow?)
Clerk-->>App: firstFactorVerification (nonce, publicKeyOptions)
App->>WebAuthn: navigator.credentials.create(publicKeyOptions)
alt credential created
WebAuthn-->>App: credential (attestation/assertion)
App->>Clerk: POST attempt_first_factor (serialized credential)
Clerk-->>App: sign-in result (success/error)
else missing options or error
App-->>User: surface error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
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
🧹 Nitpick comments (2)
packages/types/src/signInFuture.ts (2)
218-226: Clarify default behavior for flowConsider noting that omitting flow runs the “existing factor” path and requires a prior SignIn.create({ strategy: 'passkey' }).
Apply:
export interface SignInFuturePasskeyParams { /** * The flow to use for the passkey sign-in. * * - 'autofill': The client prompts your users to select a passkey before they interact with your app. * - 'discoverable': The client requires the user to interact with the client. + * + * If omitted, the existing-factor flow is used and requires a prior + * `SignIn.create({ strategy: 'passkey' })`. */ flow?: 'autofill' | 'discoverable'; }
445-452: API doc reads well; consider noting return contractYou may add one sentence that the Promise resolves with
{ error: unknown }and emits resource events via runAsyncResourceTask.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.changeset/honest-insects-deny.md(1 hunks)packages/clerk-js/src/core/resources/SignIn.ts(2 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/honest-insects-deny.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.tspackages/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.tspackages/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.tspackages/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.tspackages/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
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/types/src/signInFuture.tspackages/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.tspackages/clerk-js/src/core/resources/SignIn.ts
🧬 Code graph analysis (2)
packages/types/src/signInFuture.ts (1)
packages/types/src/strategies.ts (1)
PasskeyStrategy(5-5)
packages/clerk-js/src/core/resources/SignIn.ts (5)
packages/types/src/signInFuture.ts (1)
SignInFuturePasskeyParams(218-226)packages/shared/src/errors/webAuthNError.ts (1)
ClerkWebAuthnError(17-27)packages/clerk-js/src/utils/runAsyncResourceTask.ts (1)
runAsyncResourceTask(8-31)packages/types/src/factors.ts (1)
PasskeyFactor(52-54)packages/clerk-js/src/core/errors.ts (2)
clerkVerifyPasskeyCalledBeforeCreate(69-73)clerkMissingWebAuthnPublicKeyOptions(125-129)
⏰ 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)
.changeset/honest-insects-deny.md (1)
2-6: Changeset looks goodMinor bumps with a clear experimental note. No issues.
packages/clerk-js/src/core/resources/SignIn.ts (1)
41-41: Type import LGTMImporting SignInFuturePasskeyParams aligns runtime with types.
packages/types/src/signInFuture.ts (1)
4-4: Strategy/type surface updated correctlyAdding PasskeyStrategy to imports and to SignInFutureCreateParams.strategy is consistent with the new flow.
Also applies to: 17-17
| async passkey(params?: SignInFuturePasskeyParams): Promise<{ error: unknown }> { | ||
| const { flow } = params || {}; | ||
|
|
||
| /** | ||
| * The UI should always prevent from this method being called if WebAuthn is not supported. | ||
| * As a precaution we need to check if WebAuthn is supported. | ||
| */ | ||
|
|
||
| const isWebAuthnSupported = SignIn.clerk.__internal_isWebAuthnSupported || isWebAuthnSupportedOnWindow; | ||
| const webAuthnGetCredential = SignIn.clerk.__internal_getPublicCredentials || webAuthnGetCredentialOnWindow; | ||
| const isWebAuthnAutofillSupported = | ||
| SignIn.clerk.__internal_isWebAuthnAutofillSupported || isWebAuthnAutofillSupportedOnWindow; | ||
|
|
||
| if (!isWebAuthnSupported()) { | ||
| throw new ClerkWebAuthnError('Passkeys are not supported', { | ||
| code: 'passkey_not_supported', | ||
| }); | ||
| } | ||
|
|
||
| return runAsyncResourceTask(this.resource, async () => { | ||
| if (flow === 'autofill' || flow === 'discoverable') { | ||
| await this._create({ strategy: 'passkey' }); | ||
| } else { | ||
| const passKeyFactor = this.supportedFirstFactors.find(f => f.strategy === 'passkey') as PasskeyFactor; | ||
|
|
||
| if (!passKeyFactor) { | ||
| clerkVerifyPasskeyCalledBeforeCreate(); | ||
| } | ||
| await this.resource.__internal_basePost({ | ||
| body: { strategy: 'passkey' }, | ||
| action: 'prepare_first_factor', | ||
| }); | ||
| } | ||
|
|
||
| const { nonce } = this.firstFactorVerification; | ||
| const publicKeyOptions = nonce ? convertJSONToPublicKeyRequestOptions(JSON.parse(nonce)) : null; | ||
|
|
||
| if (!publicKeyOptions) { | ||
| clerkMissingWebAuthnPublicKeyOptions('get'); | ||
| } | ||
|
|
||
| let canUseConditionalUI = false; | ||
|
|
||
| if (flow === 'autofill') { | ||
| /** | ||
| * If autofill is not supported gracefully handle the result, we don't need to throw. | ||
| * The caller should always check this before calling this method. | ||
| */ | ||
| canUseConditionalUI = await isWebAuthnAutofillSupported(); | ||
| } | ||
|
|
||
| // Invoke the navigator.create.get() method. | ||
| const { publicKeyCredential, error } = await webAuthnGetCredential({ | ||
| publicKeyOptions, | ||
| conditionalUI: canUseConditionalUI, | ||
| }); | ||
|
|
||
| if (!publicKeyCredential) { | ||
| throw error; | ||
| } | ||
|
|
||
| await this.resource.__internal_basePost({ | ||
| body: { | ||
| publicKeyCredential: JSON.stringify(serializePublicKeyCredentialAssertion(publicKeyCredential)), | ||
| strategy: 'passkey', | ||
| }, | ||
| action: 'attempt_first_factor', | ||
| }); | ||
| }); | ||
| } |
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.
Harden error handling when WebAuthn returns no credential
Avoid throwing possibly undefined; provide a fallback error.
Apply:
- // Invoke the navigator.create.get() method.
+ // Invoke navigator.credentials.get()
const { publicKeyCredential, error } = await webAuthnGetCredential({
publicKeyOptions,
conditionalUI: canUseConditionalUI,
});
if (!publicKeyCredential) {
- throw error;
+ throw (error ?? new Error('Failed to obtain WebAuthn credential'));
}- Optional: simplify presence check; no need to cast when only checking existence.
- const passKeyFactor = this.supportedFirstFactors.find(f => f.strategy === 'passkey') as PasskeyFactor;
- if (!passKeyFactor) {
+ const hasPasskeyFactor = this.supportedFirstFactors.some(f => f.strategy === 'passkey');
+ if (!hasPasskeyFactor) {
clerkVerifyPasskeyCalledBeforeCreate();
}- Optional: dedupe with SignIn.authenticateWithPasskey by extracting a shared helper to reduce drift.
📝 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.
| async passkey(params?: SignInFuturePasskeyParams): Promise<{ error: unknown }> { | |
| const { flow } = params || {}; | |
| /** | |
| * The UI should always prevent from this method being called if WebAuthn is not supported. | |
| * As a precaution we need to check if WebAuthn is supported. | |
| */ | |
| const isWebAuthnSupported = SignIn.clerk.__internal_isWebAuthnSupported || isWebAuthnSupportedOnWindow; | |
| const webAuthnGetCredential = SignIn.clerk.__internal_getPublicCredentials || webAuthnGetCredentialOnWindow; | |
| const isWebAuthnAutofillSupported = | |
| SignIn.clerk.__internal_isWebAuthnAutofillSupported || isWebAuthnAutofillSupportedOnWindow; | |
| if (!isWebAuthnSupported()) { | |
| throw new ClerkWebAuthnError('Passkeys are not supported', { | |
| code: 'passkey_not_supported', | |
| }); | |
| } | |
| return runAsyncResourceTask(this.resource, async () => { | |
| if (flow === 'autofill' || flow === 'discoverable') { | |
| await this._create({ strategy: 'passkey' }); | |
| } else { | |
| const passKeyFactor = this.supportedFirstFactors.find(f => f.strategy === 'passkey') as PasskeyFactor; | |
| if (!passKeyFactor) { | |
| clerkVerifyPasskeyCalledBeforeCreate(); | |
| } | |
| await this.resource.__internal_basePost({ | |
| body: { strategy: 'passkey' }, | |
| action: 'prepare_first_factor', | |
| }); | |
| } | |
| const { nonce } = this.firstFactorVerification; | |
| const publicKeyOptions = nonce ? convertJSONToPublicKeyRequestOptions(JSON.parse(nonce)) : null; | |
| if (!publicKeyOptions) { | |
| clerkMissingWebAuthnPublicKeyOptions('get'); | |
| } | |
| let canUseConditionalUI = false; | |
| if (flow === 'autofill') { | |
| /** | |
| * If autofill is not supported gracefully handle the result, we don't need to throw. | |
| * The caller should always check this before calling this method. | |
| */ | |
| canUseConditionalUI = await isWebAuthnAutofillSupported(); | |
| } | |
| // Invoke the navigator.create.get() method. | |
| const { publicKeyCredential, error } = await webAuthnGetCredential({ | |
| publicKeyOptions, | |
| conditionalUI: canUseConditionalUI, | |
| }); | |
| if (!publicKeyCredential) { | |
| throw error; | |
| } | |
| await this.resource.__internal_basePost({ | |
| body: { | |
| publicKeyCredential: JSON.stringify(serializePublicKeyCredentialAssertion(publicKeyCredential)), | |
| strategy: 'passkey', | |
| }, | |
| action: 'attempt_first_factor', | |
| }); | |
| }); | |
| } | |
| async passkey(params?: SignInFuturePasskeyParams): Promise<{ error: unknown }> { | |
| const { flow } = params || {}; | |
| /** | |
| * The UI should always prevent from this method being called if WebAuthn is not supported. | |
| * As a precaution we need to check if WebAuthn is supported. | |
| */ | |
| const isWebAuthnSupported = SignIn.clerk.__internal_isWebAuthnSupported || isWebAuthnSupportedOnWindow; | |
| const webAuthnGetCredential = SignIn.clerk.__internal_getPublicCredentials || webAuthnGetCredentialOnWindow; | |
| const isWebAuthnAutofillSupported = | |
| SignIn.clerk.__internal_isWebAuthnAutofillSupported || isWebAuthnAutofillSupportedOnWindow; | |
| if (!isWebAuthnSupported()) { | |
| throw new ClerkWebAuthnError('Passkeys are not supported', { | |
| code: 'passkey_not_supported', | |
| }); | |
| } | |
| return runAsyncResourceTask(this.resource, async () => { | |
| if (flow === 'autofill' || flow === 'discoverable') { | |
| await this._create({ strategy: 'passkey' }); | |
| } else { | |
| const hasPasskeyFactor = this.supportedFirstFactors.some(f => f.strategy === 'passkey'); | |
| if (!hasPasskeyFactor) { | |
| clerkVerifyPasskeyCalledBeforeCreate(); | |
| } | |
| await this.resource.__internal_basePost({ | |
| body: { strategy: 'passkey' }, | |
| action: 'prepare_first_factor', | |
| }); | |
| } | |
| const { nonce } = this.firstFactorVerification; | |
| const publicKeyOptions = nonce ? convertJSONToPublicKeyRequestOptions(JSON.parse(nonce)) : null; | |
| if (!publicKeyOptions) { | |
| clerkMissingWebAuthnPublicKeyOptions('get'); | |
| } | |
| let canUseConditionalUI = false; | |
| if (flow === 'autofill') { | |
| /** | |
| * If autofill is not supported gracefully handle the result, we don't need to throw. | |
| * The caller should always check this before calling this method. | |
| */ | |
| canUseConditionalUI = await isWebAuthnAutofillSupported(); | |
| } | |
| // Invoke navigator.credentials.get() | |
| const { publicKeyCredential, error } = await webAuthnGetCredential({ | |
| publicKeyOptions, | |
| conditionalUI: canUseConditionalUI, | |
| }); | |
| if (!publicKeyCredential) { | |
| throw (error ?? new Error('Failed to obtain WebAuthn credential')); | |
| } | |
| await this.resource.__internal_basePost({ | |
| body: { | |
| publicKeyCredential: JSON.stringify(serializePublicKeyCredentialAssertion(publicKeyCredential)), | |
| strategy: 'passkey', | |
| }, | |
| action: 'attempt_first_factor', | |
| }); | |
| }); | |
| } |
| } | ||
|
|
||
| return runAsyncResourceTask(this.resource, async () => { | ||
| if (flow === 'autofill' || flow === 'discoverable') { |
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.
Could we have called this.resource.authenticateWithPasskey() instead of duplicating ?
Is the only difference that we don't return the result ?
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.
This is following the same pattern we've used in other methods, which is not to create strong interdependencies between SignIn and SignInFuture, since the goal is to reverse the structure in the next clerk-js major. It also allows us to implement divergent behavior between the new APIs and the legacy APIs.
In this case the difference is that SignInFuture.passkey() returns a { error } object rather than the SignIn resource.
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: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.changeset/honest-insects-deny.md(1 hunks)packages/react/src/stateProxy.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/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/react/src/stateProxy.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
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/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
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/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/react/src/stateProxy.ts
.changeset/**
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Automated releases must use Changesets.
Files:
.changeset/honest-insects-deny.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
.changeset/honest-insects-deny.md (1)
1-7: LGTM! Changeset follows the correct format.The changeset correctly documents the minor version bumps for the three affected packages and appropriately marks the feature as experimental.
| 'verifyBackupCode', | ||
| ] as const), | ||
| ticket: this.gateMethod(target, 'ticket'), | ||
| passkey: this.gateMethod(target, 'passkey'), |
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.
🛠️ Refactor suggestion | 🟠 Major
Add JSDoc documentation for the new public API method.
The passkey method follows the established pattern and is correctly implemented. However, public API methods should include JSDoc documentation per the coding guidelines.
Consider adding JSDoc documentation above the method, similar to how other public methods might be documented:
+ /**
+ * Initiates passkey-based authentication flow.
+ * @param params - Optional passkey authentication parameters
+ * @returns Promise that resolves with authentication result
+ * @experimental This API is experimental and subject to change
+ */
passkey: this.gateMethod(target, 'passkey'),As per coding guidelines.
📝 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.
| passkey: this.gateMethod(target, 'passkey'), | |
| /** | |
| * Initiates passkey-based authentication flow. | |
| * @param params - Optional passkey authentication parameters | |
| * @returns Promise that resolves with authentication result | |
| * @experimental This API is experimental and subject to change | |
| */ | |
| passkey: this.gateMethod(target, 'passkey'), |
🤖 Prompt for AI Agents
In packages/react/src/stateProxy.ts around line 134, the new public API method
`passkey: this.gateMethod(target, 'passkey'),` is missing JSDoc; add a JSDoc
block immediately above that line following the project’s existing pattern for
public methods — include a short description of what `passkey` does, annotate
parameters (e.g., target and any inputs if applicable), the return type, and any
relevant tags like @public or @since to match neighboring method docs so the
method is properly documented for consumers.
🧩 Analysis chain
Verify underlying implementation and test coverage.
Ensure that:
- The TypeScript types for
SignInFuturePasskeyParamsare properly defined and exported from@clerk/types - The underlying
SignInFuture.passkey()method is implemented inpackages/clerk-js/src/core/resources/SignIn.ts - Tests are added for the new passkey functionality
Run the following script to verify the implementation:
🏁 Script executed:
#!/bin/bash
# Verify passkey implementation and types
echo "=== Checking type definitions ==="
# Check if SignInFuturePasskeyParams type exists
rg -nP --type=ts 'SignInFuturePasskeyParams' packages/types/
echo -e "\n=== Checking SignIn implementation ==="
# Check if passkey method exists in SignInFuture
ast-grep --pattern $'class $_ {
$$$
passkey($$$) {
$$$
}
$$$
}'
echo -e "\n=== Checking for test files ==="
# Look for test files related to passkey
fd -e test.ts -e test.tsx -e spec.ts -e spec.tsx | xargs rg -l 'passkey'
echo -e "\n=== Checking stateProxy test coverage ==="
# Check if stateProxy tests exist
fd 'stateProxy.*test|stateProxy.*spec' packages/react/Length of output: 789
Please verify that SignInFuture.passkey() is implemented in the core resources:
🏁 Script executed:
#!/bin/bash
echo "=== Inspecting SignInFuture implementation in SignIn.ts ==="
# Locate the SignInFuture class
rg -nP 'class SignInFuture' -C3 packages/clerk-js/src/core/resources/SignIn.ts
# Search for the passkey() method
rg -nP '\bpasskey\s*\(' -n packages/clerk-js/src/core/resources/SignIn.tsLength of output: 497
Add missing tests for passkey proxy in stateProxy.ts
No tests currently cover the new passkey entry in packages/react/src/stateProxy.ts; add unit tests to validate its proxy behavior.
🤖 Prompt for AI Agents
In packages/react/src/stateProxy.ts around line 134, add unit tests covering the
newly added passkey proxy entry: create a test that stubs/mocks gateMethod and
asserts it's called with (target, 'passkey') when accessing state.passkey; add
tests that verify passkey read returns the value gateMethod returns, that
writing to state.passkey invokes the proxy/set behavior (or throws) consistent
with other proxied keys, and include edge cases (undefined target, absent
gateMethod) to mirror existing proxy tests for other keys; place tests alongside
existing stateProxy tests, use the same test helpers/mocks and assertions for
consistency.
Description
This PR adds support for sign-in with passkeys to our experimental Signals APIs.
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Chores