Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
RFC:
Form
primitive #1977RFC:
Form
primitive #1977Changes from 3 commits
8f357c8
9304d23
a490271
797c4a7
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
isValid
seems like a boolean prop at first glance, instead of a function. How aboutvalidator
?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.
That's a good point, we can check prior art too.
react-hook-forms
seem to usevalidate
for custom validation, so perhaps we can go with this too, it definitly is better as a verb I agree.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.
Radix got so far without having to resort to render props — why add this pattern now?
Couldn't it be a component instead?
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.
Hey Paco, sure a component-based API could be created to render based on certain state matching (pretty much a simpler version of
ClientMessage
) but sometimes you do need more access.ie. the case you highlighed above where your DS expects a prop for some variant or state.
Worth noting also that although we we haven't historically used render props, we are not completely opposed to it. It depends what the use-case is and if there are other technical solution with less API surface. For most of the states, we do believe a controlled prop fulfills the need.
With validity though, that isn't something that can be "controlled". However, we could follow the same logic and expose a callback for when the validity of a field changes so you have access to it potentially?
I do feel like a render prop is a good fit for this particular case though.
Do you have specific concerns about it?
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.
Q. How should I express different validations when one field is dependent on another?
For example, if I have a
country
field and anage
field, as shown below, and I need the max/min value of age to be different depending on the value of the country field, how can I use it?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.
you can use react state as normal:
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.
Or you could also do it using a custom validation function as it has access to the entire form data so it can check other fields values.
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 kind words!
As you said, can implement it intuitively using react state, but it would be more convenient to be able to handle dependent forms in an uncontrolled way :)