-
Notifications
You must be signed in to change notification settings - Fork 374
feat(clerk-js,clerk-react,types): Signal errors #6495
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
Changes from all commits
837fdc2
7f1dcc9
5256fc2
4b3f44f
4d9b742
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
--- | ||
'@clerk/clerk-js': minor | ||
'@clerk/clerk-react': minor | ||
'@clerk/types': minor | ||
--- | ||
|
||
[Experimental] Signal Errors |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,17 +1,96 @@ | ||
import { isClerkAPIResponseError } from '@clerk/shared/error'; | ||
import type { Errors } from '@clerk/types'; | ||
import { computed, signal } from 'alien-signals'; | ||
|
||
import type { SignIn } from './resources/SignIn'; | ||
|
||
export const signInSignal = signal<{ resource: SignIn | null }>({ resource: null }); | ||
export const signInErrorSignal = signal<{ errors: unknown }>({ errors: null }); | ||
export const signInErrorSignal = signal<{ error: unknown }>({ error: null }); | ||
|
||
export const signInComputedSignal = computed(() => { | ||
const signIn = signInSignal().resource; | ||
const errors = signInErrorSignal().errors; | ||
const error = signInErrorSignal().error; | ||
|
||
const errors = errorsToParsedErrors(error); | ||
|
||
if (!signIn) { | ||
return { errors: null, signIn: null }; | ||
return { errors, signIn: null }; | ||
} | ||
|
||
return { errors, signIn: signIn.__internal_future }; | ||
}); | ||
|
||
/** | ||
* Converts an error to a parsed errors object that reports the specific fields that the error pertains to. Will put | ||
* generic non-API errors into the global array. | ||
*/ | ||
function errorsToParsedErrors(error: unknown): Errors { | ||
const parsedErrors: Errors = { | ||
fields: { | ||
firstName: null, | ||
lastName: null, | ||
emailAddress: null, | ||
identifier: null, | ||
phoneNumber: null, | ||
password: null, | ||
username: null, | ||
code: null, | ||
captcha: null, | ||
legalAccepted: null, | ||
}, | ||
raw: [], | ||
global: [], | ||
}; | ||
|
||
if (!isClerkAPIResponseError(error)) { | ||
parsedErrors.raw.push(error); | ||
parsedErrors.global.push(error); | ||
return parsedErrors; | ||
} | ||
|
||
parsedErrors.raw.push(...error.errors); | ||
|
||
error.errors.forEach(error => { | ||
if ('meta' in error && error.meta && 'paramName' in error.meta) { | ||
switch (error.meta.paramName) { | ||
case 'first_name': | ||
parsedErrors.fields.firstName = error; | ||
break; | ||
case 'last_name': | ||
parsedErrors.fields.lastName = error; | ||
break; | ||
case 'email_address': | ||
parsedErrors.fields.emailAddress = error; | ||
break; | ||
case 'identifier': | ||
parsedErrors.fields.identifier = error; | ||
break; | ||
case 'phone_number': | ||
parsedErrors.fields.phoneNumber = error; | ||
break; | ||
case 'password': | ||
parsedErrors.fields.password = error; | ||
break; | ||
case 'username': | ||
parsedErrors.fields.username = error; | ||
break; | ||
case 'code': | ||
parsedErrors.fields.code = error; | ||
break; | ||
case 'captcha': | ||
parsedErrors.fields.captcha = error; | ||
break; | ||
case 'legal_accepted': | ||
parsedErrors.fields.legalAccepted = error; | ||
break; | ||
default: | ||
parsedErrors.global.push(error); | ||
break; | ||
} | ||
} else { | ||
parsedErrors.global.push(error); | ||
} | ||
}); | ||
|
||
return parsedErrors; | ||
} | ||
Comment on lines
+27
to
+96
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add unit tests for the new error parsing functionality The
🤖 Prompt for AI Agents
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,10 +1,9 @@ | ||||||||||||||||||||||||||||
import type { SignInFutureResource } from '@clerk/types'; | ||||||||||||||||||||||||||||
import { useCallback, useSyncExternalStore } from 'react'; | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
import { useIsomorphicClerkContext } from '../contexts/IsomorphicClerkContext'; | ||||||||||||||||||||||||||||
import { useAssertWrappedByClerkProvider } from './useAssertWrappedByClerkProvider'; | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
function useClerkSignal(signal: 'signIn'): { errors: unknown; signIn: SignInFutureResource | null } | null { | ||||||||||||||||||||||||||||
function useClerkSignal(signal: 'signIn') { | ||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Restore explicit return types for public APIs Per guidelines, exported hooks should have explicit return types. You can capture the core’s type without duplicating shapes via +import type { State as ClerkState } from '@clerk/types';
-function useClerkSignal(signal: 'signIn') {
+function useClerkSignal(signal: 'signIn'): ReturnType<ClerkState['signInSignal']> | null {
…
}
-export function useSignInSignal() {
+export function useSignInSignal(): ReturnType<ClerkState['signInSignal']> | null {
return useClerkSignal('signIn');
} 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||
useAssertWrappedByClerkProvider('useSignInSignal'); | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
const clerk = useIsomorphicClerkContext(); | ||||||||||||||||||||||||||||
|
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.
Field errors may be overwritten when multiple errors exist for the same field
The current implementation assigns errors directly to field properties (e.g.,
parsedErrors.fields.firstName = error
). If the API returns multiple errors for the same field, only the last one will be retained, potentially losing important validation information.Consider either:
Example of potential data loss:
🤖 Prompt for AI Agents