-
Notifications
You must be signed in to change notification settings - Fork 22
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
refactor: expose setFieldTouched
#50
Conversation
packages/core/src/types.ts
Outdated
@@ -138,6 +138,7 @@ export interface UseFormReturn<Values extends FormValues> { | |||
setFieldValue: UseFormSetFieldValue<Values>; | |||
setErrors: (errors: FormErrors<Values>) => void; | |||
setFieldError: UseFormSetFieldError<Values>; | |||
setFieldTouched: (name: Values, touched?: boolean) => void; |
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.
The type definition here seems incorrect. See UseFormSetFieldError<Values>
for details on this section.
vorms/packages/core/src/types.ts
Lines 110 to 115 in 009ff62
export type UseFormSetFieldError<Values extends FormValues> = < | |
Name extends Path<Values>, | |
>( | |
name: Name, | |
error: FormErrors<PathValue<Values, Name>> | string | string[], | |
) => void; |
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.
corrected but I don't know if it is necessary to return errors :)
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.
Thank you for your contribution. Regarding returning errors, do you think we should be consistent with setFieldValue
, setFieldError
?
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.
On the one hand, it's easier, on the other hand we don't need the extra, so I suggest we keep it that way for now (i.e. forever).
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.
I'm not sure I understand what you mean. Do you think something like below is better?
export type UseFormSetFieldTouched<Values extends FormValues> = <
Name extends Path<Values>,
>(
name: Name,
touched: boolean,
) => void
Because it will be consistent with other functions.
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.
yes maybe better, could have corrected it as you like ?)
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.
Thank you so much for your assistance in clarifying the issue. Considering the situation, I believe it might be more appropriate to return void
for the time being.
packages/core/src/types.ts
Outdated
@@ -114,6 +114,13 @@ export type UseFormSetFieldError<Values extends FormValues> = < | |||
error: FormErrors<PathValue<Values, Name>> | string | string[], | |||
) => void; | |||
|
|||
export type UseFormsSetFieldTouched<Values extends FormValues> = < |
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.
It would be better to use UseFormSetFieldTouched
instead of UseFormSetsFieldTouched
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
π Linked issue
β Type of change
π Description
π Checklist