-
Notifications
You must be signed in to change notification settings - Fork 391
feat(clerk-js,clerk-react,types): Add support for additional props #6897
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
…es to Signal SignIn/SignUp
🦋 Changeset detectedLatest commit: 70b0710 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 many read-only getters to SignInFuture and SignUpFuture mirroring underlying resources, expands sign-in/sign-up type interfaces (including second-factor data), updates React StateProxy to expose these properties with safe defaults, and adds a changeset declaring minor package bumps and an experimental property support note. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as App/Components
participant SP as StateProxy (React)
participant F as Future (SignIn/SignUp)
participant R as Underlying Resource
UI->>SP: access getter (e.g., supportedFirstFactors, id)
alt Clerk loaded (browser)
SP->>F: gateProperty -> invoke getter
F->>R: read corresponding resource property
R-->>F: value
F-->>SP: value
SP-->>UI: value
else Not loaded / SSR
SP-->>UI: return default value or guarded stub (throws on guarded internals)
end
note over F,R: New getters mirror resource fields including second-factor data and metadata
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (8)**/*.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
**/*.{jsx,tsx}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
integration/**📄 CodeRabbit inference engine (.cursor/rules/global.mdc)
Files:
integration/**/*📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Files:
**/*.{js,ts,tsx,jsx}📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Files:
**/*.tsx📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
Files:
⏰ 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). (28)
🔇 Additional comments (1)
Comment |
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: 4
📜 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 (6)
.changeset/weak-deer-tie.md
(1 hunks)packages/clerk-js/src/core/resources/SignIn.ts
(2 hunks)packages/clerk-js/src/core/resources/SignUp.ts
(1 hunks)packages/react/src/stateProxy.ts
(2 hunks)packages/types/src/signInFuture.ts
(3 hunks)packages/types/src/signUpFuture.ts
(3 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}
: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/clerk-js/src/core/resources/SignUp.ts
packages/react/src/stateProxy.ts
packages/clerk-js/src/core/resources/SignIn.ts
packages/types/src/signUpFuture.ts
packages/types/src/signInFuture.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/react/src/stateProxy.ts
packages/clerk-js/src/core/resources/SignIn.ts
packages/types/src/signUpFuture.ts
packages/types/src/signInFuture.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/react/src/stateProxy.ts
packages/clerk-js/src/core/resources/SignIn.ts
packages/types/src/signUpFuture.ts
packages/types/src/signInFuture.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/react/src/stateProxy.ts
packages/clerk-js/src/core/resources/SignIn.ts
packages/types/src/signUpFuture.ts
packages/types/src/signInFuture.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/react/src/stateProxy.ts
packages/clerk-js/src/core/resources/SignIn.ts
packages/types/src/signUpFuture.ts
packages/types/src/signInFuture.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/react/src/stateProxy.ts
packages/clerk-js/src/core/resources/SignIn.ts
packages/types/src/signUpFuture.ts
packages/types/src/signInFuture.ts
.changeset/**
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Automated releases must use Changesets.
Files:
.changeset/weak-deer-tie.md
🧬 Code graph analysis (2)
packages/types/src/signUpFuture.ts (1)
packages/types/src/signUpCommon.ts (2)
SignUpStatus
(25-25)SignUpField
(27-27)
packages/types/src/signInFuture.ts (2)
packages/types/src/signInCommon.ts (3)
SignInFirstFactor
(74-85)SignInSecondFactor
(87-87)UserData
(89-94)packages/types/src/verification.ts (1)
VerificationResource
(7-20)
⏰ 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)
🔇 Additional comments (5)
.changeset/weak-deer-tie.md (1)
1-7
: LGTM! Changeset properly formatted.The changeset correctly declares minor version bumps for the affected packages and appropriately flags this as an experimental feature.
packages/types/src/signInFuture.ts (1)
130-179
: LGTM! Type definitions are well-documented.The new fields are properly typed with appropriate JSDoc comments and readonly modifiers. The types align correctly with the imported definitions from
signInCommon.ts
.packages/react/src/stateProxy.ts (1)
48-86
: Good use of safe defaults withgateProperty
.The new getters consistently use
gateProperty
with appropriate fallback values, ensuring safe behavior when Clerk is not loaded or in non-browser contexts.Also applies to: 149-196
packages/types/src/signUpFuture.ts (1)
74-135
: LGTM! Comprehensive type definitions with proper documentation.The new fields are well-documented with JSDoc comments and properly typed. The readonly modifiers are consistently applied, and the types align correctly with the imported definitions.
packages/clerk-js/src/core/resources/SignUp.ts (1)
554-621
: Additional getters look solid.They mirror the underlying
SignUp
resource fields 1:1, so the expandedSignUpFutureResource
shape stays accurate and consumers now get parity with the main API surface.
get id() { | ||
return this.resource.id; | ||
} | ||
|
||
get identifier() { | ||
return this.resource.identifier; | ||
} | ||
|
||
get createdSessionId() { | ||
return this.resource.createdSessionId; | ||
} | ||
|
||
get userData() { | ||
return this.resource.userData; | ||
} |
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 explicit return types and JSDoc comments for new public getters.
These new getters lack explicit return type annotations and JSDoc documentation. Per coding guidelines, public APIs should have explicit return types and comprehensive JSDoc comments.
Apply this diff to add return types and documentation:
+ /**
+ * The unique identifier for the current sign-in attempt.
+ */
- get id() {
+ get id(): string | undefined {
return this.resource.id;
}
+ /**
+ * The identifier for the current sign-in attempt.
+ */
- get identifier() {
+ get identifier(): string | null {
return this.resource.identifier;
}
+ /**
+ * The created session ID for the current sign-in attempt.
+ */
- get createdSessionId() {
+ get createdSessionId(): string | null {
return this.resource.createdSessionId;
}
+ /**
+ * The user data for the current sign-in attempt.
+ */
- get userData() {
+ get userData(): UserData {
return this.resource.userData;
}
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.
get id() { | |
return this.resource.id; | |
} | |
get identifier() { | |
return this.resource.identifier; | |
} | |
get createdSessionId() { | |
return this.resource.createdSessionId; | |
} | |
get userData() { | |
return this.resource.userData; | |
} | |
/** | |
* The unique identifier for the current sign-in attempt. | |
*/ | |
get id(): string | undefined { | |
return this.resource.id; | |
} | |
/** | |
* The identifier for the current sign-in attempt. | |
*/ | |
get identifier(): string | null { | |
return this.resource.identifier; | |
} | |
/** | |
* The created session ID for the current sign-in attempt. | |
*/ | |
get createdSessionId(): string | null { | |
return this.resource.createdSessionId; | |
} | |
/** | |
* The user data for the current sign-in attempt. | |
*/ | |
get userData(): UserData { | |
return this.resource.userData; | |
} |
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/resources/SignIn.ts around lines 611 to 625, the
four new public getters lack explicit return type annotations and JSDoc; update
each getter to declare its explicit return type (use the exact types from the
SignIn resource/interface—e.g., id: string | undefined, identifier: string |
undefined, createdSessionId: string | undefined, userData: Record<string,
unknown> | undefined—or the precise types defined on the SignIn resource) and
add a brief JSDoc block above each getter describing what the property
represents and its return type; reference the existing SignIn resource/interface
for exact type names and keep wording consistent with other resource getters.
get supportedFirstFactors() { | ||
return this.resource.supportedFirstFactors ?? []; | ||
} | ||
|
||
get supportedSecondFactors() { | ||
return this.resource.supportedSecondFactors ?? []; | ||
} |
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 explicit return type and JSDoc for supportedSecondFactors
getter.
The new supportedSecondFactors
getter is missing an explicit return type annotation and JSDoc documentation.
Apply this diff:
- get supportedFirstFactors() {
+ get supportedFirstFactors(): SignInFirstFactor[] {
return this.resource.supportedFirstFactors ?? [];
}
+ /**
+ * The list of second-factor strategies that are available for the current sign-in attempt.
+ */
- get supportedSecondFactors() {
+ get supportedSecondFactors(): SignInSecondFactor[] {
return this.resource.supportedSecondFactors ?? [];
}
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.
get supportedFirstFactors() { | |
return this.resource.supportedFirstFactors ?? []; | |
} | |
get supportedSecondFactors() { | |
return this.resource.supportedSecondFactors ?? []; | |
} | |
get supportedFirstFactors(): SignInFirstFactor[] { | |
return this.resource.supportedFirstFactors ?? []; | |
} | |
/** | |
* The list of second-factor strategies that are available for the current sign-in attempt. | |
*/ | |
get supportedSecondFactors(): SignInSecondFactor[] { | |
return this.resource.supportedSecondFactors ?? []; | |
} |
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/resources/SignIn.ts around lines 632 to 638, the
supportedSecondFactors getter lacks an explicit return type and JSDoc; add a
JSDoc block describing the getter and its return value and give the getter the
same explicit return type as supportedFirstFactors (e.g., the appropriate array
type used by supportedFirstFactors) so the signature becomes documented and
type-annotated consistently with supportedFirstFactors.
get secondFactorVerification() { | ||
return this.resource.secondFactorVerification; | ||
} |
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 explicit return type and JSDoc for secondFactorVerification
getter.
Missing explicit return type and documentation for this public getter.
Apply this diff:
+ /**
+ * The second-factor verification for the current sign-in attempt.
+ */
- get secondFactorVerification() {
+ get secondFactorVerification(): VerificationResource {
return this.resource.secondFactorVerification;
}
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.
get secondFactorVerification() { | |
return this.resource.secondFactorVerification; | |
} | |
/** | |
* The second-factor verification for the current sign-in attempt. | |
*/ | |
get secondFactorVerification(): VerificationResource { | |
return this.resource.secondFactorVerification; | |
} |
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/resources/SignIn.ts around lines 660 to 662, the
public getter secondFactorVerification is missing an explicit return type and
JSDoc; add a JSDoc block above the getter describing its purpose and usage and
add an explicit return type that matches the type of
this.resource.secondFactorVerification (e.g., the existing
SecondFactorVerificationResource type or its optional variant) and include an
@returns tag describing the returned resource.
get secondFactorVerification() { | ||
return gateProperty(target, 'secondFactorVerification', { | ||
status: null, | ||
error: null, | ||
expireAt: null, | ||
externalVerificationRedirectURL: null, | ||
nonce: null, | ||
attempts: null, | ||
message: null, | ||
strategy: null, | ||
verifiedAtClient: null, | ||
verifiedFromTheSameClient: () => false, | ||
__internal_toSnapshot: () => { | ||
throw new Error('__internal_toSnapshot called before Clerk is loaded'); | ||
}, | ||
pathRoot: '', | ||
reload: () => { | ||
throw new Error('__internal_toSnapshot called before Clerk is loaded'); | ||
}, | ||
}); |
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.
Fix error messages in secondFactorVerification
default object.
The error messages on lines 70 and 74 are incorrect. Line 70 throws for __internal_toSnapshot
but line 74 throws the same message for reload
.
Apply this diff:
__internal_toSnapshot: () => {
throw new Error('__internal_toSnapshot called before Clerk is loaded');
},
pathRoot: '',
reload: () => {
- throw new Error('__internal_toSnapshot called before Clerk is loaded');
+ throw new Error('reload called before Clerk is loaded');
},
📝 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.
get secondFactorVerification() { | |
return gateProperty(target, 'secondFactorVerification', { | |
status: null, | |
error: null, | |
expireAt: null, | |
externalVerificationRedirectURL: null, | |
nonce: null, | |
attempts: null, | |
message: null, | |
strategy: null, | |
verifiedAtClient: null, | |
verifiedFromTheSameClient: () => false, | |
__internal_toSnapshot: () => { | |
throw new Error('__internal_toSnapshot called before Clerk is loaded'); | |
}, | |
pathRoot: '', | |
reload: () => { | |
throw new Error('__internal_toSnapshot called before Clerk is loaded'); | |
}, | |
}); | |
get secondFactorVerification() { | |
return gateProperty(target, 'secondFactorVerification', { | |
status: null, | |
error: null, | |
expireAt: null, | |
externalVerificationRedirectURL: null, | |
nonce: null, | |
attempts: null, | |
message: null, | |
strategy: null, | |
verifiedAtClient: null, | |
verifiedFromTheSameClient: () => false, | |
__internal_toSnapshot: () => { | |
throw new Error('__internal_toSnapshot called before Clerk is loaded'); | |
}, | |
pathRoot: '', | |
reload: () => { | |
throw new Error('reload called before Clerk is loaded'); | |
}, | |
}); | |
} |
🤖 Prompt for AI Agents
In packages/react/src/stateProxy.ts around lines 57 to 76, the default
secondFactorVerification object uses the same error message for both
__internal_toSnapshot and reload; update the reload throw to a descriptive
message (e.g., throw new Error('reload called before Clerk is loaded')) instead
of duplicating the __internal_toSnapshot message so each function reports the
correct error context.
@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: |
Description
This PR adds additional properties to our Signal-based SignIn/SignUp resources, bringing them to parity with the existing API.
Checklist
pnpm test
runs as expected.pnpm build
runs as expected.Type of change
Summary by CodeRabbit
New Features
Chores