-
Notifications
You must be signed in to change notification settings - Fork 36
add async form config article #105
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
base: main
Are you sure you want to change the base?
Conversation
Cybertron01Z
left a 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.
gotta approve the PR so the preview page is created
| @@ -0,0 +1,288 @@ | |||
| --- | |||
| title: Asynchronous form configuration | |||
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.
| title: Asynchronous form configuration | |
| title: Asynchronous Form Configuration |
for titles we should/can use capitalisation on nouns
| @@ -0,0 +1,288 @@ | |||
| --- | |||
| title: Asynchronous form configuration | |||
| description: "Define zod form schema with asynchronous data" | |||
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.
| description: "Define zod form schema with asynchronous data" | |
| description: "Define Zod form schema with asynchronous data" |
Zod is a proper noun and they also use capitalisation on their own website, so we should do that too throughout the article (not highlighting every occurrence ;) )
| criteria. Meaning, I need to get the async value before setting up the form. | ||
|
|
||
| In this blog post, I will show the example using React and TypeScript. | ||
| To manage my forms and form validations I work with react-hook-form and zod. |
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.
suggestion: it might be good to have 1-2 sentences about what Zod is
While Zod's fairly popular it's still good to give a little context to those who haven't heard of it yet. Also to understand how it's relevant to the problem presented (react-hook-form's obvious name helps with not requiring this as much).
|
|
||
| ## Basic Form Configuration | ||
|
|
||
| Let's consider a form with two (required) datetime form fields: `dateStart` and `dateEnd`. |
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.
| Let's consider a form with two (required) datetime form fields: `dateStart` and `dateEnd`. | |
| Let's consider a form with two required datetime form fields: `dateStart` and `dateEnd`. |
unnecessary parenthesis imo
| className={ !!form.formState.errors?.dateStart ? 'error' : '' } | ||
| /> | ||
| { form.formState.errors?.dateStart && ( | ||
| <p>{ form.formState.errors.dateStart.message }</p> // <p>Start date is required</p> |
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.
question: where is the actual text that's shown in the comment coming from? is that automatic from react-hook-form, or is that a configuration that's left out for brevity?
| For best practice, the submitted values should be also validated in the backend and/or maybe this time range criteria is | ||
| used for other use cases as well. In addition, the time range criteria needs to be configurable at deployment time. | ||
| Therefore, it would make sense to set this time range value in some configuration file, which both the frontend and | ||
| backend can read. |
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.
thought: I don't have good recommendations off the top of my head, but I notice that a lot of your sentences start with a transition word/phrase. In this case it's every sentence, which makes the structure feel repetitive.
|
|
||
| To satisfy the rule "a hook cannot be called conditionally", I simply call the `useForm()` hook | ||
| from a functional component that is rendered only after a successfully fetched data response, | ||
| otherwise I show some loading screen. |
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.
thought: After finishing reading I feel a little bit tricked. 😄
Based on the title I was expecting some elaborate solution/hack to make Zod work with async parameters/options, but then the async aspect is solved "outside", in a manner where it doesn't really matter that there is a form and form validation.
Don't get me wrong, this doesn't mean the article isn't interesting or the solution bad. On the contrary, but maybe we should frame the article slightly differently? I think the custom combined field validation with a parameter is the actual hero of the story, but maybe that's just because I haven't used Zod myself yet. 😅
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.
thanks for the review, all valid points! 👍🏻
yes, i had the feeling that the title and the content do not really match..
i am glad i let this article being reviewed by some fresh eyes! 😄
as i was addressing your feedback, i figured that this PR is actually two mini-articles in one, as i am still seeing it as a single user story..
i will keep this PR pending for the moment and will post separate PRs to propose the two mini-articles. i will let you decide if they are too short and we should keep them as one article.
will keep you posted!
Define zod form schema with asynchronous data