jquense / react-formal

Sophisticated HTML form management for React
http://jquense.github.io/react-formal
MIT License
526 stars 52 forks source link

Error message is generated for field with `noValidate` when referenced by `alsoValidates` #146

Closed antmdvs closed 6 years ago

antmdvs commented 6 years ago

Given the following schema:

const validationSchema = yup.object({
  first: yup.string(),
  last: yup.string().required(),
});

And form:

    <Form schema={validationSchema}>
      <Form.Field name="first" alsoValidates="last" />
      <Form.Field name="last" noValidate />
    </Form>

When the user types a character into the first input (that "also validates" last), I would expect no error message to be generated for the last input because last has the noValidate prop set. However, I'm observing that an error message is generated, (although the meta.valid/invalid values are correct.)

This can be seen here: https://codesandbox.io/s/lpzkrw54zm

Note that if alsoValidates is removed from first, then last doesn't generate an error (as expected).

antmdvs commented 6 years ago

134

jquense commented 6 years ago

The behavior you're seeing here is expected. noValidate is specific to the the Field component itself not the value represented by the field. Notice that typing into last doesn't trigger any validation. Honestly i'm not sure what behavior you are expecting here, A message has to show up somewhere otherwise you might as well not have any validation on the field at the schema level. Or If you don't want want a message for the field I would suggest not rendering one.

antmdvs commented 6 years ago

Ok. It just seemed to me that noValidate should preempt validation, even when participating in an alsoValidates. I'm aware the example I provided seems kind of pointless, but I'm trying to assess my options for getting conditional validation (i.e. when()) between two or more <Field>s to work differently than the default behavior and I thought noValidate might be a piece of the puzzle (but setting it conditionally, not statically as above.)

jquense commented 6 years ago

Ok. It just seemed to me that noValidate should preempt validation, even when participating in an alsoValidates

You're thinking about how this is structure a bit wrong here. noValidate is about what triggers validation, it's localized to components that can trigger validation. If it was schema wide, you're alsoValidates would have no effect.

Maybe you should describe what you are trying to do more specifically? If you are trying to "globally" controll when validation happen I would do that completely at the schema level via a when and maybe a context value

antmdvs commented 6 years ago

Ahh, that clears it up. Admittedly, I had a misconception there. Thanks!

If it was schema wide, you're alsoValidates would have no effect.

To make sure I'm being semantically correct, did you actually mean "form"-wide, i.e. as a Form prop? :) If so, I did confirm that in the sandbox.

Maybe you should describe what you are trying to do more specifically?

Forthcoming...

antmdvs commented 6 years ago

Maybe you should describe what you are trying to do more specifically?

@jquense Sure, here's what I'm trying to do. I hope this makes sense and if you have any suggestions, I would certainly appreciate it. :)

There are two things at play:

  1. Conditionally displaying required field indicators (like the common "*" postfix)
  2. The treatment of the requirement field validation errors

I have several places where there are conditionally required fields (when..required) so the idea is to use yup schema as the single-source-of-truth for conditionally required fields (unless you think this is a bad idea).

I think the list below contains the requirements I'm trying to satisfy (using the first and last example, however there could also be multiple dependents, not just 1):

Given a form with these fields: first, last And last is conditionally required when first takes on a value

Scenarios:

  1. When first receives a non-blank value, display a required field indicator on last
  2. When first is cleared, remove required field indicator on last
  3. last should not display required field error adornment/message until it has been visited (onBlur)
  4. Given first has a value And last is blank And last has been visited (so it's showing required field indicator and validation error), When first's value is cleared, Then error message should be hidden But the required field indicator should remain displayed (until first is cleared per 2)

What I tried

In order to determine whether a field is required, I tried to leverage the eager validation provided by alsoValidates. This causes the errors object to be populated when a user starts keying into first so I can then inspect that (see below). However, this also eagerly produced validation errors and was therefore at odds with the lazy error display requirement (3 above). It also requires me to specify the dependent fields in the alsoValidates prop, which kind of defeats the purpose of using yup as the source-of-truth. Now, if I remove alsoValidates, then 3 is satisfied, but I don't have any errors data to inspect to determine required fields.

const isRequired = (meta, name) => {
  const errors = meta.errors[name];
  if (errors) {
    for (let error of errors) {
      if (error.type === 'required') {
        // we found a required field validation error so now we know this field is required
        return true;
      }
    }
  }
  return false;
}
const MyField = ({ name, alsoValidates, ...rest }) => (
  <Form.Field name={name}>
    {(props, Input) => (
          // ...
          {isRequired(props.meta, name) && '*'}
          <Input {...props} {...rest} />
          // ...

I'm guessing I should either not rely on the yup schema for required fields or use validate in onFocus to in order to generate the errors and then perform the inspection. Or I might need a touched value to add to my conditional error rendering.

Anyway, sorry this was long, but just wanted to be "specific." :)

jquense commented 6 years ago

so for checking if a field is required or not i'd do something like:

const isRequired = props.meta.schema.describe().tests.includes('required')

You might have to force the last field to update tho when first changes, you can do that manually or maybe passing the first value to it as a shouldComponentUpdate breaker

antmdvs commented 6 years ago

Great! I'll check that out. Thanks.