Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

isValid behavior #1133

Closed
motiazu opened this issue Nov 25, 2018 · 34 comments
Closed

isValid behavior #1133

motiazu opened this issue Nov 25, 2018 · 34 comments

Comments

@motiazu
Copy link

motiazu commented Nov 25, 2018

馃悰 Bug report

Current Behavior

When initializing a form with initialValues, isValid state is determined by isInitialValid.
If a user did not set isInitialValid, it will still be applied with false.
Even if the values have changed but still return (after multiple edits) back to the same initialValues, the form is deemed not dirty and the error object is ignored in deciding the validity.

Expected behavior

If I decided not use isInitialValid explicitly, I expect the validity of the form to be decided by the errors object only.
If form is dirty or not, I want to errors to determine validity.

Reproducible example

https://codesandbox.io/s/jn630ymxjv
Notice how the initial value of id is GOOD_INPUT, and how any value that isn't empty or GOOD_INPUT is accepted even though GOOD_INPUT is good input.

Suggested solution(s)

I would suggest to not default isInitialValid to false, or add a prop to indicate whether I want it to determine my validation.
isInitialValid by itself is a pretty simple solution but it puts a lot more work on the dev. When using Formik I can just provide a Yup schema and the lib handles the rest, unless I set a possibly valid initial value ;(
In which case I will have to trigger my own validation, handle it's async nature by myself just to get the initialValue of something the lib gets right in any other case.

For our team I wrapped Formik and overriden the calculation of isValid, and triggered initial render on mount (we don't use SSR, I've seen this solution suggested in one of the issues). This helps avoid whatever boilerplate manual pre-validation we would have to do pretty much for every Formik use.
Edit: Since writing this I managed to create a better solution here

/**
 * WrappedFormik was created for two reasons:
 * 1. Formik doesn't validate initialValues, so we trigger a validation on mount.
 * 2. Formik calculates isValid by whether there are any errors (good) or by a prop named
 * isInitialValid. This can be found in their github under Formik.tsx in
 * the getFormikComputedProps method. Since WE don't use isInitialValid, this logic assumes all
 * our initialValues are invalid by default. So we override their isValid calculation to
 * only account for found errors.
 */
const RenderWrapper = compose(
  lifecycle({
    componentDidMount() {
      this.props.data.validateForm();
    },
  }),
  withProps(props => {
    const errors = props.data.errors;
    return { isValid: Object.keys(errors).length === 0 };
  }),
)(({ render, data, isValid }) => render({ ...data, isValid }));
export const WrappedFormik = ({ render, ...props }) => (
  <Formik {...props} render={data => <RenderWrapper data={data} render={render} />} />
);

Your environment

Software Version(s)
Formik 1.3.1
React 15.6.2
TypeScript -
Browser Chrome
npm/Yarn Both
Operating System OSX
@jaredpalmer
Copy link
Owner

Thanks for taking the time to write this up. I agree this is a bit awkward.

I think this is a symptom of not having initialErrors prop. If we had that, we could just compute as follows:

const hasErrors = (errors) => {
  return Object.keys(errors).some(key => {
    const value = errors[key]

    if (value && typeof value === 'object') {
      return hasErrors(value)
    }

    return typeof value !== 'undefined'
  })
}

Related discussions #1116 #742 #614

@motiazu
Copy link
Author

motiazu commented Dec 3, 2018

That would indeed help but this is assuming the initialErrors/isInitialValid are the desired solution for every case, which I think they are not. I would rather the library to not consider isInitialValid at all while calculating isValid in the case where I didn't set a value for isInitialValid explicitly (default to undefined instead of false).

That's what my workaround did and it fits our use case very well.
isInitialValid is not easy to calculate especially when considering validation in most cases is async. I rather render twice to get validation on mount, and the way the lib uses isInitialValid forces me to wrap it and override that behavior which I'd rather not do.

I'd be happy to submit a PR if you think that's a good idea. I understand that this is a breaking change since the previous behavior of isInitialValid would be changed when it's not explicitly set. This could be worked around by adding a prop that indicates this behavior (pristine form=isInitialValid) is unwanted.

@scottc-netflix
Copy link

This is a big problem for me as well. I have lots of forms you can open and close, and the UI needs to maintain the form values that were entered when switching between forms. Currently valid forms have their submit cleared.

I don't understand why there is the option to hardcode a prop for if the form is valid or not on init. A callback I could understand, but this actually seems really hacky and completely inconsistent with how you'd expect a form to function. Instead there should be validateOnInit, which I would imagine should default to false, get rid of isInitialValid entirely, problem solved. Then if it's true, Formik just validates on the initial values.

@ufolux
Copy link

ufolux commented Jan 18, 2019

Same problem, validateOnInit is needed.

@sarahsmo
Copy link

@jaredpalmer @motiazu Is anyone making steps to resolve this? It is quite an annoyance that isValid doesn't work. To me, it makes the most sense if isValid returns true when all the fields are filled out (no errors). I don't understand why isValid uses dirty in its logic.

@mjangir
Copy link

mjangir commented Feb 16, 2019

@sarahsmo I also don't rely on isValid as its behaviours is inconsistent. Rather I simply use the following to disable my submit button:

disabled={!_.isEmpty(errors)}

@mpmprudencio
Copy link

Hi @mjangir your suggestion looks clear enough but this gives me an error, what exactly are you using as _ ?

@dfalling
Copy link

@mpmprudencio lodash or underscore probably.

@mjangir
Copy link

mjangir commented Feb 28, 2019

Hi @mjangir your suggestion looks clear enough but this gives me an error, what exactly are you using as _ ?

I'm using lodash.

@rally25rs
Copy link

rally25rs commented Mar 4, 2019

Just ran into this as well. Was quite surprised that if a yup validation schema is provided, it isn't used to determine isValid when there are no changes. 馃槥

Without underscore/lodash you could use the plain JS:

isValid = Object.keys(errors).length === 0;

I think the approach I'm going with is to extend or wrap Formik into my own <Form> tag, which will give me a place to substitute the above logic for the default. Something like this seems to work:

export default function Form(props) {
  const {children, ...rest} = props;

  return (
    <Formik {...rest}>
      {formikBag => {
        formikBag.isValid = Object.keys(formikBag.errors).length === 0;

        return (
          {children(formikBag)}
        );
      }}
    </Formik>
  );
}

Form.propTypes = {
  children: PropTypes.func.isRequired,
};

then the usage stays just like normal formik:

      <Form
        validationSchema={modelSchema.validations}
        initialValues={Object.assign({}, modelSchema.defaults, data)}
        onSubmit={onSubmit}
      >
        {({isValid, errors}) => (
          ...

**Edit: **

There seemed to be cases where errors was not set on initial render. It looks like if you have a ref to Formik you can call .getFormikActions().validateForm() on that ref.

I updated my wrapper around Formik to this:

const Form = React.forwardRef(function FormImpl(props, ref) {
  const {children, ...rest} = props;
  const formikRef = React.useRef();

  React.useImperativeHandle(ref, () => {
    return formikRef.current;
  });

  React.useEffect(() => {
    formikRef.current.getFormikActions().validateForm();
  }, []);

  return (
    <Formik ref={formikRef} {...rest}>
      {formikBag => {
        return (
          <form noValidate onSubmit={formikBag.handleSubmit}>
            {children(formikBag)}
          </form>
        );
      }}
    </Formik>
  );
});

This does have an issue where on initial render it will try to show the validation messages, so you have to make sure your validation messages are also tied to touched to see if the fields have been focused.

@sisp
Copy link

sisp commented Mar 13, 2019

I'm facing the same problem where my forms can have initially invalid values and Formik currently doesn't check initial values for errors. Just to add another workaround that I'm using (inspired by this issue) based on React Hooks (Typescript) in case anyone finds it useful, too:

const FormikValidateInitialValues: React.FC<{ form: FormikProps<any> }> = ({ form, children }) => {
  React.useLayoutEffect(() => {
    form.validateForm()
  }, [form.initialValues])
  // [form.initialValues] triggers the effect whenever initial values change.
  // This is necessary for triggering validation when you switch between the
  // same form with different initial values.
  return <>{children}</>
}

const MyForm: React.FC<...> = props = (
  <Formik ...>
    {form => (
      <FormikValidateInitialValues form={form}>
        ...
      </FormikValidateInitialValues>
    )}
  </Formik>
)

@motiazu
Copy link
Author

motiazu commented Mar 13, 2019

@sisp I eventually found out I could just pass isInitialValid a function that runs a synchronous Yup validation for me. I could do that because my validations were never async. If you have a similar case I would go for that.

@sisp
Copy link

sisp commented Mar 13, 2019

@motiazu Do you have a minimal example?

This is what I tried and it doesn't work:

<Formik ... isInitialValid={props => props.myValidator(props.initialValues) === undefined}>
  ...
</Formik>

Perhaps I misunderstood your suggestion.

@motiazu
Copy link
Author

motiazu commented Mar 14, 2019

Something along these lines -

function isInitialValid(props) {
  if (!props.validationSchema) return true;
  return props.validationSchema.isValidSync(props.initialValues);
}
<Formik ... isInitialValid={isInitialValid}>
  ...
</Formik>

I created a wrapped Formik component for us to reuse throughout the rest of our code with this modification.

@jgillich
Copy link

Even if the values have changed but still return (after multiple edits) back to the same initialValues, the form is deemed not dirty and the error object is ignored in deciding the validity.

The opposite is also true, if you have valid initialValues, change them and submit, and then reenter the original values, you get isValid == false and cannot submit again.

@motiazu
Copy link
Author

motiazu commented Mar 17, 2019

@jgillich that would be true if isInitialValid is not set to true. Either way you can use the workaround I suggested here in case your validations are all synchronous.

@helloanil
Copy link

I had the same issue while trying to create a multistep form. I managed to overcome this issue with this approach:

  • When you submit the form, add another value called isSubmitted to wherever you store the form values. (In my case, I store the step values in Redux on submit and submit action adds this extra value as true)
  • Next time you land on the form (or step), initialValues will be taken from the store and you'll have isSubmitted value true.
  • However the touched prop coming from Formik will be an empty object so you can do something like this:
disableNext={values.isSubmitted && _.isEmpty(touched) ? false : !isValid}

So basically I'm forcing my Next button to be enabled only if the form is already submitted and no field is touched. Otherwise it keeps deciding with using isValid.

@zanehiredevs
Copy link

zanehiredevs commented May 5, 2019

@motiazu solution worked for me. Just wanted to add that you can use the withFormik HOC instead as a solution.

I was able to run my validators on mount when I was using the withFormik HOC with this.props.validateForm() but not after I switched to using the tag and a render child form prop.

For some reason validateForm just wouldn't run when it mounted even though the function was available in my child form component and I didn't get any errors it just didn't run the validation on mount anymore.

Edit: actually validateForm does work on mount if I pass in the values like this.props.validateForm(this.props.values) but it also seems I also need the isInitialValid also to get this for validateForm to work on mount.

@motiazu
Copy link
Author

motiazu commented May 6, 2019

@zanehiredevs This is because the actual validation function does not actually validate if the form isn't dirty, it just uses isInitialValid. So setting a function is still the best solution Iv'e found unless you upgrade to the next Formik.

@gothraven
Copy link

should we wait to the V2 so we can have this update fix ?

@mohsinulhaq
Copy link

Any update on this? react-final-form seems to do the right thing here.

@jaredpalmer
Copy link
Owner

v2 fixes this.

@jaredpalmer
Copy link
Owner

and keeps backwards compat if you already have done stuff with isInitalValid

@motiazu
Copy link
Author

motiazu commented Jul 1, 2019

Well providing a function right now seems to be a pretty good solution plus it's fixed in v2, I don't think we expect anymore changes to this issue. Closed.

@motiazu motiazu closed this as completed Jul 1, 2019
@Enzyoh
Copy link

Enzyoh commented Aug 23, 2019

@jaredpalmer i am on v2.0.1-rc12 . The behavior of 'isValid' still does not take into account the initial values. Maybe I missed something but what is the nature of the fix in v2 ? Or rather is it not in this rc version?

@pastinepolenta
Copy link

pastinepolenta commented Sep 3, 2019

Well providing a function right now seems to be a pretty good solution plus it's fixed in v2, I don't think we expect anymore changes to this issue. Closed.

Shouldn't a bug, especially if important, be fixed/backported on the current major stable version (v1) instead of having to wait/force the upgrade to v2?

@harrisgeo88
Copy link

I just had the same problem on version 2.0.8. Apparently setting validateOnMount to true fixed this issue for me 馃帀

@demeralde
Copy link

demeralde commented Dec 17, 2019

validateOnMount works for me, but it's throwing Yup.required errors once the form is submitted (even though these fields have been filled in). Looks like it's validating based on the values from initialValues (not the actual values from the form).

When validateOnMount is false this doesn't happen.

@Snowklop
Copy link

Have the same problem as @dspacejs , @jaredpalmer what is the proper solution to fix this?

@EQuimper
Copy link

Same error as @dspacejs the only way I was able to fix this issue. Is by removing validateOnMount and use initialErrors={{ phoneNumber: 'Required' }} but ...

@TLadd
Copy link

TLadd commented Jan 20, 2020

@EQuimper @dspacejs I think y'all are probably running into #2046. validateOnMount currently runs on every render if the initialValues aren't memoized.

@EQuimper
Copy link

@TLadd yes thank you, this is the same issue. Work with useMemo

@ryandagg
Copy link

@EQuimper @dspacejs I think y'all are probably running into #2046. validateOnMount currently runs on every render if the initialValues aren't memoized.

Thank you. This is the only thing that I've found that prevented an issue with validateOnMount + yup leading to a field always saying it was required, even when the value was a non-empty string.

@dsgriffin
Copy link

dsgriffin commented Jan 21, 2020

Only way it works for me:

<Formik 
    validateOnMount={true} 
    initialValues={useMemo(() => { return { email: '' } }, [])}
    ... 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests