-
Couldn't load subscription status.
- Fork 402
feat(clerk-js,clerk-react,types): Signal SignUp #6568
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: c41f955 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.
|
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
📝 WalkthroughWalkthroughAdds an internal SignUpFuture wrapper and exposes it on SignUp via __internal_future and __internal_basePost; SignUp.fromJSON emits resource:update through eventBus. Introduces SignUp signals (resource, error, fetch, computed) paralleling SignIn and wires them into State with corresponding public properties and event handlers. Extends the React hook useClerkSignal to support 'signUp' and adds useSignUpSignal. Adds a SignUpFutureResource type and new SignInSignal/SignUpSignal interfaces in types. Includes a changeset for minor package bumps. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
packages/clerk-js/src/core/state.ts (1)
39-47: Bug: onResourceError dispatches resource instead of error for SignUpFor SignIn, errors are dispatched via signInErrorSignal({ error }). For SignUp, this path incorrectly dispatches the resource via signUpResourceSignal. This will break error consumers and mask failures.
Apply this diff to align with the SignIn behavior:
private onResourceError = (payload: { resource: BaseResource; error: unknown }) => { if (payload.resource instanceof SignIn) { this.signInErrorSignal({ error: payload.error }); } - if (payload.resource instanceof SignUp) { - this.signUpResourceSignal({ resource: payload.resource }); - } + if (payload.resource instanceof SignUp) { + this.signUpErrorSignal({ error: payload.error }); + } };
🧹 Nitpick comments (8)
.changeset/tasty-bears-sip.md (1)
1-8: Changeset LGTM; consider clarifying experimental scope in the noteMinor bumps and an explicit experimental note are appropriate for this API surface expansion.
If you want this to be extra clear in release notes, consider adding a one-liner about “internal plumbing only, not publicly supported” to set expectations for consumers.
packages/types/src/signUp.ts (1)
121-123: Add JSDoc and make the field readonly for consumer safety; align with SignIn counterpartPublic types should be documented, and readonly is preferred for consumer-facing resource snapshots.
Apply this diff:
-export interface SignUpFutureResource { - status: SignUpStatus | null; -} +/** + * @experimental Minimal future wrapper around the `SignUp` resource used by Signals. + * Exposes a stable snapshot for UI consumption that may change between renders. + */ +export interface SignUpFutureResource { + readonly status: SignUpStatus | null; +}Also verify this interface mirrors the semantics of
SignInFutureResourcefor consistency (readonly, docs).Would you like me to scan for
SignInFutureResourceand ensure parity across fields and docs?packages/react/src/hooks/useClerkSignal.ts (1)
43-45: LGTM: Snapshot path wired for SignUp; includesignalin deps to satisfy exhaustive-depsFunctionally correct. To avoid stale closures and satisfy react-hooks/exhaustive-deps, include
signalin the dependency arrays for the callbacks.Apply the following outside this range:
- Update subscribe callback deps:
- [clerk, clerk.loaded, clerk.__internal_state], + [clerk, clerk.loaded, clerk.__internal_state, signal],
- Update getSnapshot deps:
- }, [clerk.__internal_state]); + }, [clerk.__internal_state, signal]);packages/types/src/state.ts (1)
41-51: API shape mirrors SignIn; add experimental note to align with PR scopeThe addition looks good and consistent. Since the PR marks SignUp Signals as experimental, add an explicit JSDoc tag to convey that to downstream consumers of @clerk/types.
Apply this diff:
/** - * A Signal that updates when the underlying `SignUp` resource changes, including errors. + * @experimental This experimental API is subject to change. + * + * A Signal that updates when the underlying `SignUp` resource changes, including errors. */ signUpSignal: { (): { errors: Errors; fetchStatus: 'idle' | 'fetching'; signUp: SignUpFutureResource | null; }; };packages/clerk-js/src/core/state.ts (1)
25-29: Consider marking these as explicit public and add tests for the new signalsOptional: For clarity and to align with the guidelines (“Use public explicitly for clarity in public APIs”), consider adding the public modifier to these fields (and the SignIn ones for consistency). Also, there are no tests in this PR covering the new SignUp signals. Please add unit tests to assert:
- signUpResourceSignal updates on resource:update
- signUpErrorSignal updates on resource:error
- signUpFetchSignal updates on resource:fetch
- signUpSignal computed value shape
Apply this diff within the current hunk:
- signUpResourceSignal = signUpResourceSignal; - signUpErrorSignal = signUpErrorSignal; - signUpFetchSignal = signUpFetchSignal; - signUpSignal = signUpComputedSignal; + public signUpResourceSignal = signUpResourceSignal; + public signUpErrorSignal = signUpErrorSignal; + public signUpFetchSignal = signUpFetchSignal; + public signUpSignal = signUpComputedSignal;Additionally, consider updating the existing SignIn fields (outside this hunk) for consistency:
// elsewhere in this file public signInResourceSignal = signInResourceSignal; public signInErrorSignal = signInErrorSignal; public signInFetchSignal = signInFetchSignal; public signInSignal = signInComputedSignal;packages/clerk-js/src/core/resources/SignUp.ts (3)
82-88: Type the future field to the public interface to avoid leaking internal classTo keep the public surface aligned with the types package and avoid coupling to the internal class, prefer typing the field to SignUpFutureResource instead of SignUpFuture.
Apply this diff:
- __internal_future: SignUpFuture | null = new SignUpFuture(this); + __internal_future: SignUpFutureResource | null = new SignUpFuture(this);
89-96: Optional: add an explicit type to __internal_basePostNot required, but adding an explicit type improves readability and adheres to the guideline of explicit types. One option:
You could annotate as:
// keep initializer as-is __internal_basePost: this['_basePost'] = this._basePost.bind(this);If you prefer not to use this-indexed types, consider exposing a lightweight internal type alias for the call signature of _basePost.
472-478: Optional: add explicit return type to status getterMinor polish for clarity and to align with explicit typing guidance.
Apply this diff:
- get status() { + get status(): SignUpStatus | null { return this.resource.status; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
.changeset/tasty-bears-sip.md(1 hunks)packages/clerk-js/src/core/resources/SignUp.ts(5 hunks)packages/clerk-js/src/core/signals.ts(2 hunks)packages/clerk-js/src/core/state.ts(2 hunks)packages/react/src/hooks/useClerkSignal.ts(4 hunks)packages/types/src/signUp.ts(1 hunks)packages/types/src/state.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
.changeset/**
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Automated releases must use Changesets.
Files:
.changeset/tasty-bears-sip.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/signUp.tspackages/react/src/hooks/useClerkSignal.tspackages/types/src/state.tspackages/clerk-js/src/core/signals.tspackages/clerk-js/src/core/resources/SignUp.tspackages/clerk-js/src/core/state.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/signUp.tspackages/react/src/hooks/useClerkSignal.tspackages/types/src/state.tspackages/clerk-js/src/core/signals.tspackages/clerk-js/src/core/resources/SignUp.tspackages/clerk-js/src/core/state.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/types/src/signUp.tspackages/react/src/hooks/useClerkSignal.tspackages/types/src/state.tspackages/clerk-js/src/core/signals.tspackages/clerk-js/src/core/resources/SignUp.tspackages/clerk-js/src/core/state.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/signUp.tspackages/react/src/hooks/useClerkSignal.tspackages/types/src/state.tspackages/clerk-js/src/core/signals.tspackages/clerk-js/src/core/resources/SignUp.tspackages/clerk-js/src/core/state.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/signUp.tspackages/react/src/hooks/useClerkSignal.tspackages/types/src/state.tspackages/clerk-js/src/core/signals.tspackages/clerk-js/src/core/resources/SignUp.tspackages/clerk-js/src/core/state.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/signUp.tspackages/react/src/hooks/useClerkSignal.tspackages/types/src/state.tspackages/clerk-js/src/core/signals.tspackages/clerk-js/src/core/resources/SignUp.tspackages/clerk-js/src/core/state.ts
**/*
⚙️ CodeRabbit Configuration File
If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.
Files:
packages/types/src/signUp.tspackages/react/src/hooks/useClerkSignal.tspackages/types/src/state.tspackages/clerk-js/src/core/signals.tspackages/clerk-js/src/core/resources/SignUp.tspackages/clerk-js/src/core/state.ts
🧬 Code Graph Analysis (3)
packages/types/src/state.ts (1)
packages/types/src/signUp.ts (1)
SignUpFutureResource(121-123)
packages/clerk-js/src/core/signals.ts (1)
packages/clerk-js/src/core/resources/SignUp.ts (1)
SignUp(58-470)
packages/clerk-js/src/core/resources/SignUp.ts (2)
packages/clerk-js/src/core/events.ts (1)
eventBus(31-31)packages/types/src/signUp.ts (1)
SignUpFutureResource(121-123)
⏰ 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: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (7)
packages/react/src/hooks/useClerkSignal.ts (1)
23-26: LGTM: Subscribes to the SignUp signal pathwayMirrors the SignIn case and ensures effect tracks the right signal.
packages/clerk-js/src/core/state.ts (3)
7-17: Imports look consistent and aligned with SignIn patternNamed imports for the SignUp signals mirror existing SignIn usage. No issues spotted.
54-56: LGTM: resource:update handler for SignUpMirrors SignIn behavior as expected.
64-66: LGTM: resource:fetch handler for SignUpFetch status propagation is consistent with SignIn.
packages/clerk-js/src/core/resources/SignUp.ts (3)
19-20: Type import for SignUpFutureResource is correctUsing a type-only import aligns with tree-shaking and type hygiene.
49-49: Event bus integration import looks goodImport path aligns with the rest of core modules.
410-411: Unconditional event emission is consistent with SignIn.fromJSONVerified that in
packages/clerk-js/src/core/resources/SignIn.ts(from lines 451–466) thefromJSONmethod emitsresource:updateunconditionally—outside theif (data)block—exactly as inSignUp.fromJSON. No guarding diff is needed to maintain consistency.
| export const signUpResourceSignal = signal<{ resource: SignUp | null }>({ resource: null }); | ||
| export const signUpErrorSignal = signal<{ error: unknown }>({ error: null }); | ||
| export const signUpFetchSignal = signal<{ status: 'idle' | 'fetching' }>({ status: 'idle' }); | ||
|
|
||
| export const signUpComputedSignal = computed(() => { | ||
| const signUp = signUpResourceSignal().resource; | ||
| const error = signUpErrorSignal().error; | ||
| const fetchStatus = signUpFetchSignal().status; | ||
|
|
||
| const errors = errorsToParsedErrors(error); | ||
|
|
||
| return { errors, fetchStatus, signUp: signUp ? signUp.__internal_future : null }; | ||
| }); |
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
Good parity with SignIn signals; consider deduplication helper and verify integration with State
The SignUp signals mirror SignIn correctly and the computed unwraps __internal_future as expected.
To reduce duplication, consider a small factory to create the triple (resource, error, fetch) and computed:
function createAuthSignals<T extends { __internal_future: unknown }>() {
const resource = signal<{ resource: T | null }>({ resource: null });
const error = signal<{ error: unknown }>({ error: null });
const fetch = signal<{ status: 'idle' | 'fetching' }>({ status: 'idle' });
const computedSignal = computed(() => {
const r = resource().resource;
const e = error().error;
const s = fetch().status;
const errors = errorsToParsedErrors(e);
return { errors, fetchStatus: s, value: r ? r.__internal_future : null };
});
return { resource, error, fetch, computedSignal };
}
// Usage:
// const { resource: signUpResourceSignal, error: signUpErrorSignal, fetch: signUpFetchSignal, computedSignal: signUpComputedSignal } = createAuthSignals<SignUp>();Also, please add tests mirroring the SignIn computed behavior for SignUp (error parsing edge cases, fetch status transitions).
To double-check wiring with the internal state dispatchers and SignUp.__internal_future, run the following quick verification:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Check that SignUp state wiring exists and uses the new signals"
rg -nP -C3 --type=ts "signUp(ResourceSignal|ErrorSignal|FetchSignal|ComputedSignal|Signal)" packages/clerk-js/src/core
echo "Confirm SignUp.__internal_future is present and used"
rg -nP -C3 --type=ts "__internal_future" packages/clerk-js/src/core/resources
echo "Locate State implementation using signUp signals (clerk-js side)"
rg -nP -C3 --type=ts "\bsignUpSignal\s*\(" packages/clerk-js/src/core
echo "Ensure React hook is exported"
rg -nP -C2 --type=ts "export function useSignUpSignal" packages/reactLength of output: 5667
State wiring & computed signal verified; React hook missing; add tests & optionally dedupe
- ✅ Core signals and
Stateintegration confirmed inpackages/clerk-js/src/core/state.ts - ✅
SignUp.__internal_futureis present and correctly unwrapped insignUpComputedSignal ⚠️ No React hook found inpackages/react(e.g.useSignUpSignalor equivalent) — please add and export one⚠️ Add tests mirroring SignIn computed behavior for SignUp:
• Error parsing edge cases
• Fetch status transitions- ✨ Optional refactor: dedupe the triple of resource/error/fetch signals and computed into a factory:
function createAuthSignals<T extends { __internal_future: unknown }>() {
const resource = signal<{ resource: T | null }>({ resource: null });
const error = signal<{ error: unknown }>({ error: null });
const fetch = signal<{ status: 'idle' | 'fetching' }>({ status: 'idle' });
const computedSignal = computed(() => {
const r = resource().resource;
const e = error().error;
const s = fetch().status;
const errors = errorsToParsedErrors(e);
return { errors, fetchStatus: s, value: r ? r.__internal_future : null };
});
return { resource, error, fetch, computedSignal };
}
// Usage:
// const { resource: signUpResourceSignal, error: signUpErrorSignal, fetch: signUpFetchSignal, computedSignal: signUpComputedSignal } = createAuthSignals<SignUp>();🤖 Prompt for AI Agents
In packages/clerk-js/src/core/signals.ts around lines 22 to 34, the signUp
computed signal exists but there is no corresponding React hook exported (e.g.,
useSignUpSignal) and there are no tests for SignUp analogous to SignIn; add a
React hook in packages/react that subscribes to signUpComputedSignal and exports
it for consumers, and add unit tests mirroring SignIn computed behavior to cover
error parsing edge cases and fetch status transitions; optionally refactor by
extracting the resource/error/fetch/computed trio into a reusable factory to
dedupe and then replace the signUp signal trio with a factory instantiation.
| function useClerkSignal(signal: 'signIn' | 'signUp') { | ||
| useAssertWrappedByClerkProvider('useClerkSignal'); | ||
|
|
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
Annotate return types (public API consumers depend on them); add overloads for better DX
Even though useClerkSignal is internal, the exported hooks rely on it. Add explicit return types and overloads to keep types precise and satisfy lint rules about explicit returns.
Apply this diff to annotate the implementation’s return type:
-function useClerkSignal(signal: 'signIn' | 'signUp') {
+function useClerkSignal(
+ signal: 'signIn' | 'signUp',
+): ReturnType<State['signInSignal']> | ReturnType<State['signUpSignal']> | null {Additionally, add the following outside this range:
- A type-only import at the top of the file:
import type { State } from '@clerk/types';- Overload signatures immediately above the implementation:
function useClerkSignal(signal: 'signIn'): ReturnType<State['signInSignal']> | null;
function useClerkSignal(signal: 'signUp'): ReturnType<State['signUpSignal']> | null;🤖 Prompt for AI Agents
In packages/react/src/hooks/useClerkSignal.ts around lines 6 to 8, the hook
implementation lacks explicit return types and overloads; add a type-only import
for State from '@clerk/types' at the top of the file, add two overload
signatures immediately above the implementation for signal 'signIn' and 'signUp'
returning ReturnType<State['signInSignal']> | null and
ReturnType<State['signUpSignal']> | null respectively, and annotate the
implementation signature with an explicit return type (the union
ReturnType<State['signInSignal']> | ReturnType<State['signUpSignal']> | null) so
consumers and the linter have precise types.
|
|
||
| export function useSignUpSignal() { | ||
| return useClerkSignal('signUp'); | ||
| } |
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
Expose explicit return type and mark as experimental; consider similar treatment for useSignInSignal
Public hooks should have explicit return types. Also surface the experimental nature of the new hook in JSDoc.
Apply this diff:
-export function useSignUpSignal() {
- return useClerkSignal('signUp');
-}
+/**
+ * @experimental A React hook that subscribes to the SignUp Signal.
+ * Returns a snapshot with `{ errors, fetchStatus, signUp }`, or `null` before Clerk state is ready.
+ */
+export function useSignUpSignal(): ReturnType<State['signUpSignal']> | null {
+ return useClerkSignal('signUp');
+}Additionally, for consistency, annotate useSignInSignal (outside this range):
/**
* A React hook that subscribes to the SignIn Signal.
* Returns a snapshot with `{ errors, fetchStatus, signIn }`, or `null` before Clerk state is ready.
*/
export function useSignInSignal(): ReturnType<State['signInSignal']> | null {
return useClerkSignal('signIn');
}And ensure the type-only import:
import type { State } from '@clerk/types';Lastly, please add or update tests to cover:
- Subscribing via
useSignUpSignaland receiving updated snapshots on sign-up state mutations. - Error propagation in the errors field.
- Proper null initial snapshot behavior pre-load.
Happy to scaffold react-testing-library tests that simulate state updates via the internal state adapter.
🤖 Prompt for AI Agents
In packages/react/src/hooks/useClerkSignal.ts around lines 58 to 61, the
exported hook useSignUpSignal lacks an explicit return type and JSDoc marking it
experimental; update the hook to include an explicit return type using State
(add a type-only import: import type { State } from '@clerk/types') and add an
experimental JSDoc comment describing the snapshot shape (e.g.,
ReturnType<State['signUpSignal']> | null). Also apply the analogous annotation
shown for useSignInSignal elsewhere in the file. Finally, add/update tests
(react-testing-library) to assert subscribing via useSignUpSignal receives
snapshots on sign-up state mutations, that errors propagate into the errors
field, and that the hook returns null as the initial snapshot before Clerk state
is ready.
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
♻️ Duplicate comments (2)
packages/react/src/hooks/useClerkSignal.ts (2)
62-64: Add explicit return type and mark useSignUpSignal as experimental in JSDocMirror the SignIn hook and surface the experimental nature of SignUp.
Apply this diff:
-export function useSignUpSignal() { - return useClerkSignal('signUp'); -} +/** + * @experimental A React hook that subscribes to the SignUp Signal. + * Returns a snapshot with `{ errors, fetchStatus, signUp }`, or `null` before Clerk state is ready. + */ +export function useSignUpSignal(): ReturnType<State['signUpSignal']> | null { + return useClerkSignal('signUp'); +}
58-60: Add explicit return type and JSDoc to useSignInSignal (public API)Hooks are public-facing; add JSDoc and an explicit return type using State['signInSignal'].
Apply this diff:
-export function useSignInSignal() { - return useClerkSignal('signIn'); -} +/** + * A React hook that subscribes to the SignIn Signal. + * Returns a snapshot with `{ errors, fetchStatus, signIn }`, or `null` before Clerk state is ready. + */ +export function useSignInSignal(): ReturnType<State['signInSignal']> | null { + return useClerkSignal('signIn'); +}
🧹 Nitpick comments (3)
packages/types/src/state.ts (1)
49-55: Mark signUpSignal as experimental in JSDocThe PR description states this is experimental and not intended for public use. Reflect that in the type-level docs.
Apply this diff:
/** - * A Signal that updates when the underlying `SignUp` resource changes, including errors. + * @experimental This experimental API is subject to change. + * A Signal that updates when the underlying `SignUp` resource changes, including errors. */ signUpSignal: SignUpSignal;packages/clerk-js/src/core/signals.ts (1)
22-25: SignUp resource/error/fetch signals mirror SignIn; consider dedup with a small factoryGood parity. To reduce duplication and potential drift, consider a helper that produces the trio plus computed.
A possible approach:
// outside the changed ranges; for illustration only function createAuthSignals<T extends { __internal_future: unknown }, R>( project: (r: T | null) => R | null, ) { const resource = signal<{ resource: T | null }>({ resource: null }); const error = signal<{ error: unknown }>({ error: null }); const fetch = signal<{ status: 'idle' | 'fetching' }>({ status: 'idle' }); const computedSignal = computed(() => { const r = resource().resource; const e = error().error; const s = fetch().status; const errors = errorsToParsedErrors(e); return { errors, fetchStatus: s, ...project(r) }; }); return { resource, error, fetch, computedSignal }; } // Usage example: // const { resource: signUpResourceSignal, error: signUpErrorSignal, fetch: signUpFetchSignal, computedSignal: signUpComputedSignal } = // createAuthSignals<SignUp, { signUp: SignUpFuture | null }>(r => ({ signUp: r ? r.__internal_future : null }));This keeps semantics identical while preventing drift between SignIn/SignUp wiring.
packages/react/src/hooks/useClerkSignal.ts (1)
1-1: Import State type to annotate public hooks’ return typesTo satisfy our guideline on explicit return types for public APIs and improve DX, bring in State for ReturnType<State['...']>.
Apply this diff:
-import type { SignInSignal, SignUpSignal } from '@clerk/types'; +import type { State, SignInSignal, SignUpSignal } from '@clerk/types';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/clerk-js/src/core/signals.ts(2 hunks)packages/react/src/hooks/useClerkSignal.ts(4 hunks)packages/types/src/state.ts(2 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/types/src/state.tspackages/clerk-js/src/core/signals.tspackages/react/src/hooks/useClerkSignal.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/state.tspackages/clerk-js/src/core/signals.tspackages/react/src/hooks/useClerkSignal.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/types/src/state.tspackages/clerk-js/src/core/signals.tspackages/react/src/hooks/useClerkSignal.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/state.tspackages/clerk-js/src/core/signals.tspackages/react/src/hooks/useClerkSignal.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/state.tspackages/clerk-js/src/core/signals.tspackages/react/src/hooks/useClerkSignal.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/state.tspackages/clerk-js/src/core/signals.tspackages/react/src/hooks/useClerkSignal.ts
**/*
⚙️ CodeRabbit Configuration File
If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.
Files:
packages/types/src/state.tspackages/clerk-js/src/core/signals.tspackages/react/src/hooks/useClerkSignal.ts
🧬 Code Graph Analysis (3)
packages/types/src/state.ts (2)
packages/types/src/signIn.ts (1)
SignInFutureResource(129-155)packages/types/src/signUp.ts (1)
SignUpFutureResource(121-123)
packages/clerk-js/src/core/signals.ts (3)
packages/clerk-js/src/core/resources/SignIn.ts (1)
SignIn(75-484)packages/types/src/state.ts (2)
SignInSignal(29-35)SignUpSignal(37-43)packages/clerk-js/src/core/resources/SignUp.ts (1)
SignUp(58-470)
packages/react/src/hooks/useClerkSignal.ts (1)
packages/types/src/state.ts (2)
SignInSignal(29-35)SignUpSignal(37-43)
⏰ 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). (9)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Unit Tests (22, **)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (7)
packages/types/src/state.ts (3)
2-2: Type import for SignUpFutureResource is correct and scoped as type-onlyThis keeps runtime bundles lean and aligns with our type-layer separation.
29-35: SignInSignal call-signature interface looks good and matches computed usageThe explicit function interface improves DX for ReturnType<> consumers (e.g., hooks).
37-43: SignUpSignal shape is consistent; confirm minimal SignUpFutureResource is intentionalThe symmetry with SignIn is good. The current SignUpFutureResource only exposes status (per signUp.ts). If additional operations (create/verify/etc.) are planned, confirm this minimal shape is intentional for the experimental phase.
If this minimal shape is intentional, no action needed. If not, we can expand SignUpFutureResource to include the expected methods for parity with SignIn. I can draft the type surface once you confirm scope.
packages/clerk-js/src/core/signals.ts (3)
2-2: Type-only imports for Errors/Signals maintain clean runtime; goodKeeps tree-shaking effective and avoids runtime deps from @clerk/types.
12-20: Typing signInComputedSignal as SignInSignal is precise and helpfulThe computed correctly unwraps __internal_future and preserves Errors/fetchStatus semantics.
26-34: SignUp computed signal is correct and mirrors SignIn’s behaviorUnwraps __internal_future, carries fetch status, and parses errors via errorsToParsedErrors.
packages/react/src/hooks/useClerkSignal.ts (1)
7-10: Overloads on useClerkSignal are good; explicit union return type helps inferenceThis keeps consumers strongly typed when selecting a specific signal.
| export const signUpComputedSignal: SignUpSignal = computed(() => { | ||
| const signUp = signUpResourceSignal().resource; | ||
| const error = signUpErrorSignal().error; | ||
| const fetchStatus = signUpFetchSignal().status; | ||
|
|
||
| const errors = errorsToParsedErrors(error); | ||
|
|
||
| return { errors, fetchStatus, signUp: signUp ? signUp.__internal_future : null }; | ||
| }); |
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
Add tests to cover SignUp signal behavior (parity with SignIn tests)
No tests are shown in this PR. Please add tests for:
- fetchStatus transitions ('idle' -> 'fetching' -> 'idle')
- error parsing mapping into field/global buckets
- exposing null vs non-null signUp values as the resource updates
I can scaffold these tests. Meanwhile, to check current coverage, run:
🏁 Script executed:
#!/bin/bash
# Verify if there are SignUp signal tests
rg -nP -C2 --type=ts "signUp(ResourceSignal|ErrorSignal|FetchSignal|ComputedSignal|Signal)" packages | sed -n '1,200p'
# Look for tests referring to SignUp signals
rg -nP -C2 --type=ts -g '!*node_modules*' "(useSignUpSignal|signUpComputedSignal)" packages | sed -n '1,200p'Length of output: 5847
Add unit tests for SignUp signal behavior
To achieve parity with existing SignIn tests, please add tests covering the following scenarios for signUpComputedSignal in packages/clerk-js (e.g. under a new __tests__ folder alongside signals.ts):
- fetchStatus transitions: verify it moves from
'idle'→'fetching'→'idle'when a sign-up request starts and finishes. - error parsing: exercise
errorsToParsedErrors(error)for both field-specific and global errors. - resource output: assert that before the
SignUpresource arrives you getsignUp: null, then once it’s set you get the correct__internal_futurevalue.
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/signals.ts around lines 26 to 34, add Jest unit
tests under packages/clerk-js/src/core/__tests__/signals.test.ts that cover
signUpComputedSignal: write a test that simulates fetchStatus transitions by
setting underlying signUpFetchSignal status from 'idle' → 'fetching' → 'idle'
and asserting computed fetchStatus follows those changes; write tests that call
errorsToParsedErrors with both field-specific and global error shapes and assert
signUpComputedSignal.errors matches parsed output; and write a test that asserts
signUpComputedSignal.signUp is null before signUpResourceSignal.resource is set
and equals the resource's __internal_future after you set the resource. Use the
same mocking/setup utilities and signal setters used by existing SignIn tests to
mutate the underlying signals and ensure tests clean up/reset signal state
between cases.
| function useClerkSignal(signal: 'signIn'): ReturnType<SignInSignal> | null; | ||
| function useClerkSignal(signal: 'signUp'): ReturnType<SignUpSignal> | null; | ||
| function useClerkSignal(signal: 'signIn' | 'signUp'): ReturnType<SignInSignal> | ReturnType<SignUpSignal> | null { | ||
| useAssertWrappedByClerkProvider('useClerkSignal'); |
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
Ensure hook is re-exported from the package entrypoint and add basic tests
Please confirm useSignUpSignal is exported from the public React package entry and add at least smoke tests covering initial null, update on state change, and error propagation.
Run the following to verify exports and test coverage placeholders:
Also applies to: 58-64
🏁 Script executed:
#!/bin/bash
# Check if useSignUpSignal is exported from the React package entrypoint(s)
fd -t f index.ts packages/react | xargs -I{} rg -nP -C2 "useSignUpSignal" {}
# Check for existing tests referencing the hook
rg -nP -C2 --type=ts "useSignUpSignal" packages/react
# If not present, I can scaffold react-testing-library tests analogous to existing SignIn hook tests.Length of output: 440
Export hooks from the public React entrypoint and add smoke tests
The useSignInSignal and useSignUpSignal implementations live in packages/react/src/hooks/useClerkSignal.ts but aren’t re-exported from the package’s main entrypoint, and there are no tests validating their behavior. Please:
- Re-export both hooks in your React package entrypoint (e.g.
packages/react/src/index.tsorpackages/react/index.ts):export { useSignInSignal, useSignUpSignal } from './hooks/useClerkSignal';
- Add basic smoke tests (e.g. in
packages/react/src/hooks/__tests__/useClerkSignal.test.tsx) that assert:
• The hook returnsnullbefore Clerk is initialized
• It updates when the underlying signal state changes
• It surfaces errors from the signal
You can mirror the existing Sign-In hook tests to keep consistency.
🤖 Prompt for AI Agents
In packages/react/src/hooks/useClerkSignal.ts around lines 7 to 10, the
sign-in/up hooks are implemented but not exported from the React package
entrypoint and there are no smoke tests; re-export useSignInSignal and
useSignUpSignal from the package entrypoint (packages/react/src/index.ts or
packages/react/index.ts) so they are publicly available, and add a new test file
packages/react/src/hooks/__tests__/useClerkSignal.test.tsx containing basic
smoke tests that assert: the hook returns null before Clerk is initialized, the
hook updates when the underlying signal state changes, and the hook surfaces
errors from the signal—mirror the existing Sign-In hook tests for structure and
assertions.
Description
This PR implements the plumbing for a Signal-based
SignUpusing the same patterns we've used for our SignIn. It's still experimental, and not intended for public use.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Chores