jaredpalmer / formik

Build forms in React, without the tears 😭
https://formik.org
Apache License 2.0
34k stars 2.79k forks source link

input type='number' actually returns empty string for empty input #1869

Open bhishp opened 5 years ago

bhishp commented 5 years ago

🚀 Feature request

Firstly, thanks Jared and formik community, it is an amazing lib that helps us extensively in our work.

I'm not sure if this is an intentional pattern or a bug but the Formik change handler for <input type=number /> will actually return an empty string for empty input, rather than an undefined.

Current Behavior

Given a form state like this:

interface Shape { amount?: number }

<Formik<Shape>
  initialValues={{ amount: undefined }}
>
  ...
  <Field type="number" name="amount" />

If a user clears all the content of an input field Formik will set amount to "" (empty string). Therefore, in Formik land, type="number" actually means non-zero number or empty string.

This happens because of https://github.com/jaredpalmer/formik/blob/24ac8ddbddc5717c57178b8a9e90f752fbaa6858/src/Formik.tsx#L519

The React changeEvent will spit out "" as a value, parseFloat returns NaN, therefore the result is empty string.

Desired Behavior

Given that Formik already abstracts on behalf of us to convert from a string value, to a numerical value (when type=number or range), to me it seems more logical to return undefined in this case, rather than empty string

This would change the TS type from number | '' to number | undefined.

Suggested Solution

((parsed = parseFloat(value)), isNaN(parsed) ? undefined : parsed)

Who does this impact? Who is this for?

This caused a weird bug for our app where we sent empty string over the wire, rather than undefined, which our server interprets as different things and blew-up. Not sure of wider impact.

Describe alternatives you've considered

None yet

Additional context

KrzysztofMadejski commented 4 years ago

+1. It's unintuitive to get string on an optional number field. Currently I have to manually remove such values before further processing.

Tananga commented 4 years ago

+1

samueldepooter commented 4 years ago

It follows the HTML standard spec, reference https://github.com/facebook/react/issues/13752#issuecomment-524264893

unformatt commented 4 years ago

I don't see much of the point of following the spec that pedantically as the whole reason for these libraries is that the spec and standard functionality of HTML/JS is not enough to suit our needs. Formik is here to add sugar on top of it and make our lives easier.

What's also tough about this is that if you specify .default(0) in your yup schema, then your yup validation test function will not get the empty string:

...
my_number:  Yup.number()
            .default(0)
            .test('my-check', 'required',
                function(value){ 
                    return value == '' ? false : true // never returns false because value == 0 when actual value is empty string
                }
             )

So then when you get the form values from Formik's onSubmit callback, it will have the empty string and now you have to do another step of validation or transformation.

I could remove .default(0) from my yup schema but then I need to set the default somewhere else. So with or without .default(0), I can't purely use yup + formik - I have to write some code before or after to massage the data.

johnrom commented 4 years ago

A zero manually inputted is very different than an empty string. Depending on your form, it could mean two different things, one meaning "no preference" or some other nullable condition.

Your typescript type should be { myNumber: number | '' } for this reason. To get around this, you can override onChange with a number formatting function that coerces empty strings to zero (or some other null placeholder), like the following. If you use something that doesn't directly translate to a string, you'll also have to format that value as a string.

<input
  onChange={event => formik.handleChange(field.name, parseInt(event.target.value, 10))}
  value={formatNumericValue(field.value)} 
/>

Or you can keep an eye on #2255 and see if that functionality gets merged in, which does the same thing with a more fluent API.

Unfortunately, we check for undefined when traversing the list of fields when marking all fields as touched on a submission, to show errors on untouched fields. To get around this, you might be able to use null, but I can't confirm right now.

crhistianramirez commented 1 year ago

To get around this, you might be able to use null

This doesn't work, you'll get the following warning when trying to use null:

Warning: value prop on input should not be null. Consider using an empty string to clear the component or undefined for uncontrolled components.

JonathanYon commented 1 year ago

has anyone found a way to make this work?

aagranovExtend commented 1 month ago

Also looking for a solution here.