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
Add parse, format, formatOnBlur to getFieldProps() #2255
Add parse, format, formatOnBlur to getFieldProps() #2255
Changes from 7 commits
b4b1dc0
5bb4e7c
303ec51
9be5333
d89ed12
a28f1e8
ca194f7
10c9cd0
f01855b
91fd5e0
5efd691
18f82f8
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.
I think the parse function provides the perfect opportunity to decouple the checkbox and radio parsing from handleChange, convertingvalue: unknown
to a custom shape. Then if a user wants to override this logic, they can easily.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.
Problem is that checkbox relies on current state for getValuesForCheckbox(). Technically we would need to provide that. What do you think?
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.
If that's the case, I think we can providefield.defaultParseFn
or something.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.
Oh, you mean in the event the user wants togetValuesForCheckbox
in userland? hmm.. maybe we'll need to pass formikBag as a third argument, or expose the field's complete value in some way.If you can figure out a way to useparse
from outside of Formik (with as little Formik surface area as possible...) to update a multi checkbox field, I think that's something that would be required of the parse function. I use that in my onParse function for a custom CheckboxGroup that I use.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've revised my thoughts here, and I agree that onChange should handle the array parsing for checkboxes and multi-selects. But getValueOrEvent should return an unparsed single value, with the default parse function for a number or range field running parseFloat for backwards compatibility.
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.
TLDR: keep values as strings for the default.
FWIW currently, in the app, I'm working on I have to both serialize and parse numbers to make sure they're expected values when Formik is transforming them correctly and GraphQL knows its an Int.
I get
initialValues
from query parameters in the URL, which are all strings. Then I parse them into Numbers if they're defined, and if they're not, I need to cast them to''
. Next, I need to make sure they're numbers, to then tell GraphQL this is an int or fallback tonull
to not use the variable. This would have been a lot easier had Formik not changed the values to a number. because then I could just cast them when making the query to GraphQL.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.
@lifeiscontent you can override this functionality yourself with
parse={value => value}
we need to keep the default parsing ints for backwards compatibility for now.