formik icon indicating copy to clipboard operation
formik copied to clipboard

[v2]: RFC: Remove built-in support for async validation

Open jaredpalmer opened this issue 5 years ago ā€¢ 30 comments

Before you tell me I'm crazy, hear me out....

When Ian and I first built Formik, we chose Yup because it was the best validation library at the time. Since Yup was async, we had to make validation async from the beginning. However, Yup now has sync API and also an API to validate fields independently.

Async validation is the source of almost all of the complexity inside of Formik and yet, AFAICT, the vast majority of forms do not actually use async validation

So what if we just dropped baked-in support for async validation and moved it to userland?

It turns out that this is actually not that bad of a breaking change. With hooks, this seems quite lovely both at field-level and at form-level....

Field Level Async Validation

Current Behavior

// Let's say you want to validate that a username is available
// by checking your on each keystroke (Note: IRL you would debounce)

const UsernameField = () => {
    const formik = useFormikContext()
    const [field, meta] = useField('username', {
       validate: async value => {
          if (value.trim().length > 3) {
		    const exists = await MyApi.checkUsername(value)
		    return exists && 'Unavailable'
          }
       }
    })
   return (
    <div> 
       <input {...field} />
       {meta.error && field.values.length > 3 ? meta.error : null}
    </div>
 )
}

Suggested Behavior

const UsernameField = () => {
    const formik = useFormikContext()
    const [field, meta] = useField('username')
	const handleChange = (e) => {		
        field.handleChange(e)
        if (e.target.value.trim().length > 3) {
		  MyApi.checkUsername(e.target.value)
		    .then((exists) => 
              formik.setFieldError('username', exists ? 'Unavailable' : undefined)
            )
        }
    }

   return (
    <div> 
       <input {...field} handleChange={handleChange} />
       {meta.error && field.values.length > 3 ? meta.error : null}
    </div>
 )
}

Form-level Async Validation

Current Behavior

<Formik 
  validate={async values => {
     // do something async
  }}
/> 

Suggested Behavior(s)

// Move async form-level validation to submission function
<Formik 
  validate={values => {
     // only sync stuff here
  }}
  onSubmit={async values => {
    // validate async in userland, thrown errors become submitErrors
  }}
/> 

// or make a `validateOnSubmit` that could be async

<Formik 
  validate={values => {
     // only sync stuff here
  }}
  validateOnSubmit={async values => {
  
  }}
  onSubmit={async values => {
    // validate async in userland, thrown errors become submitErrors
  }}
/> 

// if you need async validation onChange or onBlur, you could make a
// component with useFormikContext + React.useEffect()

const ValidateAsyncOnChange = ({ validate }) => {
   const formik = useFormikContext()
   React.useEffect(() => {
      let isCurrent = true
      validate(formik.values).then((errors) => 
        if (isCurrent) formik.setErrors(errors)
      )
      return () => {
        isCurrent = false
      }
   }, [formik.values, formik.setErrors])
  return null
}

// and then
const myAsyncValidate = async () => // ....

// You / we could reimplement the old api as well on top of all of this...
const FormikLegacy = ({ validate, validationSchema, children, ...props }) => 
<Formik 
  {...props}
  onSubmit={async (values, { fields }) => {
    // validate async in userland, thrown errors become errors
    const errors = await validate(values)
    const fieldErrors = await Promise.all(fields.current.map(...))
    const schemaErrors = await validationSchema(values).catch(yupToFormErrors)
    const error = deepmerge([validateError, fieldErros, schemaErrors])
    if (errors) {
      throw errors
    }
  }}
>
 <ValidateAsyncOnChange validate={myAsyncValidate} />
{/* ... children render prop stuff... */}
</Formik> 

Tradeoffs

Obviously the field-level stuff is a bit less ergonomic and uglier. TBH, my guess is that A LOT of people are already incorrectly putting validation in a wrapped handleChange/handleBlur (instead of on <Field validate>). This seems fine when you are writing it, but folks hopefully realize that Formik will only run validate methods during pre-submit (and will not loop through onBlur).

The benefits could be fairly massive internally. We can make Formik fast as hell for the majority of forms with this new sync constraint. We will dramatically reduce bundle size as well since most of the codebase is more or less Promise magic. We can teach the userbase about thinking of formik more as store and less of a coordination tool. To be fair, this is indeed one of the core value propositions of Formik.

Externally, this constraint would make testing Formik much easier. Since handleChange and handleBlur can be sync by default, Formik will work as expected when you go to simulate change and blur events with enzyme or rtl.

If I built Formik from scratch today (and had Yup's sync API available), I do not think I would support async validation directly. Among all of the apps I work on, I can count the number of times I have used async validation on one hand. Formik's mission is to kick ass on 80% of forms, and thus this async stuff really falls outside of that.

Open to suggestions and comment.

jaredpalmer avatar May 18 '19 16:05 jaredpalmer

If I were maintaining formik I'd remove the async validation from the current formik and rebuilt it into it's own separate hooks library built on top of proposed sync hooks. So instead of useField, you'd install formik-async-hooks and use useFieldAsync to have a filed with async validation.

I haven't used formik myself yet on a bigger project, but I am reviewing a PR which brings it into our big production SPA. We will need async validation, so it would be nice if we didn't have to reimplement it ourselves.

capaj avatar May 18 '19 17:05 capaj

Oops, I didnā€™t even know that it supported this. Last time I needed it, I ended up implementing async validation in my app.

If the benefits of speed and code size are clear then this is definitely a meaningful trade off.

paramaggarwal avatar May 18 '19 17:05 paramaggarwal

@paramaggarwal its for this exact reason why I think it should be removed. Most folks probably are rolling on their own already.

jaredpalmer avatar May 18 '19 18:05 jaredpalmer

Async validation is the source of almost all of the complexity inside of Formik

Can you elaborate more on that?

elado avatar May 18 '19 20:05 elado

@elado In order to support async validation, Formik actually wraps all validation functions with Promise. This causes a given update to values to usually trigger 2 renders. One for the values and then one when validation has finished. If you go into src/Formik.tsx and search for "Promise" you will see that it is everywhere. This also makes testing Formik harder than it should be since change and blur and submit events are all 100% sync in the browser (and in enzyme/rtl).

If all validation is sync, then we don't actually need #1522 either. This will make things much easier in Concurrent React.

jaredpalmer avatar May 18 '19 20:05 jaredpalmer

Async validation is mostly not needed, until it is.

Handling async validation in onSubmit looks conceptually.bad. Maybe offer an asyncValidate key that gets hooked in before onSubmit as part of that call.

mschipperheyn avatar May 18 '19 21:05 mschipperheyn

Go for it, v2 is not stable. If you should introduce this change, this is right time

Andreyco avatar May 18 '19 21:05 Andreyco

Thinking about user-land async validation: Would there be something wrong with exposing formiks errors as a prop, so that validation happens in controlled mode. Something like this

const [validationErros, setValidationErros] = React.useState();

return <Formik
   errors={validationErrors}
   onValidate={ values => fetch(/* ... */).then(errors=> setErrors(errors) }
   >
   { /* ...*/  }

</Formik>

Without having thought too much about it, it looks simple and clean to me.

jannikbuschke avatar May 18 '19 21:05 jannikbuschke

initialErrors already exists in v2, but there is whole other can of worms we can possibly open up if we go down the path of controlled formik.

jaredpalmer avatar May 18 '19 23:05 jaredpalmer

What happens if somebody submits the form during the async request? How does formik know to wait?

blocka avatar May 19 '19 03:05 blocka

@blocka it wouldnā€™t. Thatā€™s why you would need to ā€œre-validateā€ in your onSubmit function.

jaredpalmer avatar May 19 '19 05:05 jaredpalmer

What about making both callbacks run synchronously and provide formikHelpers for async stuff.

returning

  • an empty object => valid
  • an object with fields as keys and messages as values (as usual) => invalid
  • null, undefined, nothing or void => async process
    • in async flows formikHelpers need to be invoked by users to indicate results

sync:

<Formik
  onValidate={ (values, helpers) => {
     return validate(values).errors
  }
  onSubmit={ (values, helpers) => {
   const validationResult = validate(values)
   if(!validationResult.isValid){
      return validationResult.errors // returning synchronously validation errors in onSubmit
    }
    else(validationResult.isValid){
      helpers.setSubmitting(true)
      fetch("/post-values",values).then(response=>{
         if(response.ok) helpers.setSubmitting(false)
         else /* handle errors */
      }
   }
  }
}

async:

<Formik
  onValidate={ (values, helpers) => {
     fetch("/validate", values).then(errors => helpers.setErrors(errors)
  }
  onSubmit={ (values, helpers) => {
    helpers.setSubmitting(true);
    fetch("/post-values",values).then( response => {
      if(response.ok) helpers.setSubmitting(false)
      else if(!response.ok) /* handle errors */;
   }
  }
}

jannikbuschke avatar May 19 '19 10:05 jannikbuschke

This doesnā€™t solve the internal problem and is almost identical to the current API.

jaredpalmer avatar May 19 '19 15:05 jaredpalmer

Your suggested example usage has a race condition. That doesnā€™t necessarily mean you shouldnā€™t do this, but if someone as experienced as you are (presumably you dealt with race conditions in formikā€™s internals) can make this mistake, your users will make this mistake en-mass.

const UsernameField = () => {
    const formik = useFormikContext()
    const [field, meta] = useField('username')
    const handleChange = (e) => {		
        field.handleChange(e)
        if (e.target.value.trim().length > 3) {
		  MyApi.checkUsername(e.target.value)
		    .then((exists) => 
+ // TODO: You need to check that ā€œusernameā€ still has the same value
+ //       as what you passed to the ā€œcheckusernameā€ function here, it could
+ //       have changed.
                      formik.setFieldError('username', exists ? 'Unavailable' : undefined)
                    )
        }
    }

   return (
    <div> 
       <input {...field} handleChange={handleChange} />
       {meta.error && field.values.length > 3 ? meta.error : null}
    </div>
 )
}

more specifically, the problem is that you canā€™t rely on ā€œfetchā€ calls to return in order. Debouncing doesnā€™t actually fix this issue either, it just makes it less frequently occuring, but adding some space between the requests. They still can & do end up overlapping.

ForbesLindesay avatar May 19 '19 20:05 ForbesLindesay

Thatā€™s a fair point. We will need to educate accordingly.

jaredpalmer avatar May 19 '19 22:05 jaredpalmer

@jaredpalmer I went through this same hell with react-form 2 informed I finally decided that supporting async validation internally was overcomplicating the heck out of the lib. Removed it in V2 and it made things soo much better!

joepuzzo avatar May 28 '19 17:05 joepuzzo

Also i have been toying with the idea of adding a lock function to the form api that would allow the users async validation to "lock" the form from being submitted. This way you would not have to revalidate on submit. Adding some sort of async support back is something i have been thinking about a lot and its cool to see i'm not alone lol.

joepuzzo avatar May 28 '19 18:05 joepuzzo

i like this idea.

swyxio avatar Jul 29 '19 07:07 swyxio

@jaredpalmer I completely agree to remove the async validation support from the library but split the existing error object into sync and asyncErrors. The reason I'm asking this is that when it comes to FieldArrays and array manipulations, maintaining asyncError object outside formik leads to an inconsistent state. If you have a solution to that problem it would be really happy to hear about it.

@joepuzzo I've implemented the PromiseQueue for that purpose, you might wanna have a look, it cache's the last value, promise timeout, cache invalidation. Posting it here in case anyone is looking at how to implement the same.

devarsh avatar Dec 19 '19 11:12 devarsh

Yeah I have a branch that removes it and about to push it up.

jaredpalmer avatar Dec 19 '19 16:12 jaredpalmer

I am glad we're removing async support completely, but users should have a helper method to add async error messages to Formik internal state (this should be separate from existing error state, else we would be overwriting error message when sync validation fires on every render), also when FieldArray is being manipulated users have to write extra code to manage asyncError object otherwise outside of Formik, and this would cause the fields to rerender twice if that AsyncError object is also stored in a context and passed down in userland when Array manipulations happen.

Yes I agree there will be problems related to ongoing async validation when user try submitting the form, and a lock kind of mechanism should be in place, but that can be implemented in userland and handle the form submission with custom code.

I would love to hear some thoughts and what are the pros and cons of this approach.

devarsh avatar Dec 20 '19 09:12 devarsh

What's the state of this after some months now? Today I had a request to add some async validation to an existing formik form and stumbled upon this issue.

What's the recommended path forward for now? Should I stick with "legacy validation" or should I avoid it and roll my own even though it's still working on current releases?

nfantone avatar Feb 14 '20 12:02 nfantone

Making the dev perform the validation outside of the Formik validation API causes a huge issue when it comes time to submit, and for dependent fields. What happens when submitForm() calls validateForm()? The field-level validation cannot be called again to confirm everything is valid. And what about dependent fields?

It's hard to come up with examples for something I've never actually used, but here's a super contrived example combining a bunch of things that would be borderline unmanageable without a Field-level async validation API.

I tried to think of things that would be a security concern to pre-load in advance, like existing users and email domain whitelists, and would require separate requests to prevent brute forcing.

// A "super cool" form which auto-generates you a username based on your email
// First, check email is formatted like an email, then check if the domain is whitelisted
// then update the username autogenerated from the email
// and validate it doesn't already exist on the server
// if it is valid, set the username for the user. Clients have had weirder requests.
const MyForm = ({ emailIsValidAsync, usernameIsValidAsync }) => {
  const formik = useFormik({
    initialValues: { email: "", username: "" },
  });
  // useEventCallback returns a ref that never changes
  const handleEmailChange = useEventCallback(async (email) => {
    // if email is valid and user hasn't entered username
    if (formik.values.email && !formik.isValidating && !formik.errors.email && !formik.values.username) {
      const username = autoGenerateUsername(email);

      // only set the username if it doesn't already exist on the server
      // and the user doesn't edit the username before then
      if (await formik.validateForm({
        ...formik.values,
        username,
      }) && !formik.refs.values.current.username) { // doesn't exist yet, see #2828 
        setFieldValue('username', username);
      }
    }
  }, [formik.values, formik.isValidating, formik.errors.email, formik.values.username, formik.setFieldValue]);
  useEffect(() => {
    handleEmailChange();
  }, [formik.values.email, handleEmailChange /* never changes */ ])
  return (
    <FormikProvider value={formik}>
      <Field name="email" type="email" validate={async (value) => {
        if (!isEmailFormattedCorrectly(value)) {
          return "Please enter a valid email.";
        }

        const validationResult = await emailIsValidAsync(value);

        if (domainIsInvalid(validationResult)) {
          return `Sorry, users from ${validationResult.invalidDomain} are not allowed.`;
        }
      }} />
      <Field name="username" type="text" validate={async (value) => {
        if (!isUsernameFormattedCorrectly(value)) {
          return "Usernames can't contain special characters or spaces.";
        }

        const validationResult = await usernameIsValidAsync(value);

        if (usernameExists(validationResult)) {
          return `Sorry, that username is taken.`;
        }
      }} />
    </Formik>
  );
};

johnrom avatar Nov 11 '20 17:11 johnrom

Came up with a way to support this but without making the onSubmit a synchronous function. This way nothing internally calls await or anything async šŸ˜ https://joepuzzo.github.io/informed/?path=/story/validation--async-validation

joepuzzo avatar Nov 12 '20 15:11 joepuzzo

Because I will 100% back Jared up in saying that async internally has more cost than benefit. The capabilities I have been able to add to informed were been greatly increased once I removed internal support.

joepuzzo avatar Nov 12 '20 15:11 joepuzzo

@joepuzzo how would you accomplish the above without async validation? I agree it'd make life a million times easier from a maintenance point of view, but I think generally code across the board is becoming more async and not less.

johnrom avatar Nov 12 '20 20:11 johnrom

You still use async validation.. its just how its used. Did you see the example I linked ?

joepuzzo avatar Nov 12 '20 22:11 joepuzzo

In that example the user is responsible for anything async, and simply calls two methods on the formApi

joepuzzo avatar Nov 12 '20 22:11 joepuzzo

I guess I didn't get my point across in my example above. The problem is two-fold.

a) managing validation state within Formik (informed does this) b) using the same validation mechanism programatically, regardless of the relationship of any fields

One must be able to either let Formik do its own thing, managing its own isValidating, handling onChange or onBlur or whatever, and be able to get a validation result whenever they want from the API, without any knowledge of the combination of fields that generates a given validation result.

From what I can tell of your API, it would require polling or an effect. Maybe it doesn't and I read it wrong, but I don't really have time to dig into other libraries atm.

const useEffect(() => {
  if (values.username) return;

  // email changed, check to see if the form would be valid with an autogenerated username, 
  // and if so, update the value in the form
  const username = autogenerateUsername(values.email);
  validateForm({ ...values, username }).then(errors => {
    // check username is empty _in the future_, not for this render
    if (!errors.username && !formik.getState().values.username) {
      setFieldValue('username', username);; // this can hit APIs, whatever it needs to do
    }
  })
}, [values.email]);

I know in this contrived example I could always instead call validateUsernameViaApi(username); or whatever directly, but in highly dynamic scenarios where dependent fields might be composed from a json schema or CMS, this wouldn't help at all.

johnrom avatar Nov 12 '20 23:11 johnrom

As a recent user of async validation using yup, I would like to chime in with a "please do not remove the support" :-)

Being able to trivially add a async function for validating a field against a backend made it relatively easy to mix client-side and server-side validations (e.g https://runkit.com/embed/z85f3ou8wupu). I hope to find some knob in formik to help me disable certain buttons while the async validations are being executed to avoid users being allowed to for example click "save" before the async validation also have definitely passed.

Although of course, tradeoffs might still make it worth it.

andoks avatar Apr 12 '24 14:04 andoks