jaredpalmer / formik

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

Yup's .defined() causes problems with validation logic in Formik when used with Typescript #2528

Open patricknick opened 4 years ago

patricknick commented 4 years ago

I have some trouble getting my head around a breaking change introduces in Yup with version '0.29.0' in combination with Formik.

Namely it's about the following: the types ([@types/yup](https://www.npmjs.com/package/@types/yup)) have been updated to include 'undefined' by default. TypeScript users can add '.defined()' to their schemas to account for this.

This change is rather simple and straightforward. For example, if you want to specify the following form values:

type InputValues = {
  simpleInput: string;
  simpleInput2: string;
};

const initialValues: InputValues = {
  simpleInput: "abc",
  simpleInput2: ""
};

const schema = Yup.object<InputValues>({
  simpleInput: Yup.string().required("Required"),
  simpleInput2: Yup.string().defined()
});

simpleInput is required, therefore always defined. simpleInput2 is optional, but always defined with '' according to the above. But when you now submit such a form, you receive the following error from Yup: simpleInput2 must be defined.

After some research I found the following code in Formik, which changes empty string to undefined which then fails of course in the defined() validation. https://github.com/jaredpalmer/formik/blob/849ed497e24afbc1c9d41dbd0ad84db8231a48c2/packages/formik/src/Formik.tsx#L1108

Full working example here: https://codesandbox.io/s/great-surf-3x642?file=/src/App.tsx

I suppose there is a good reason behind this line of code. But I'm not sure it is intended behaviour in combination with Yup. Should this be adapted on Formik side or on user-side and ideally be documented?


donaldpipowitch commented 4 years ago

@jaredpalmer As formik is actively written in TypeScript and advertising the usage with yup would this something you'd change inside formik? Would you accept a MR to remove this line? Thank you.

clawoflight commented 4 years ago

Please stop converting empty strings to undefined as mentioned in https://github.com/formium/formik/issues/2047. This makes formik + yup unusable for me.

burtek commented 3 years ago

+1 for this. Also see https://github.com/formium/formik/issues/1697 and https://github.com/formium/formik/issues/1697#issuecomment-726772584

cc: @maddhruv @jaredpalmer

maddhruv commented 3 years ago

I've raised a PR fixing the same - https://github.com/formium/formik/pull/2902