jurassix / react-validation-mixin

Simple validation mixin (HoC) for React.
MIT License
282 stars 38 forks source link

Provide an option to validate only one field #3

Closed ivan-kleshnin closed 9 years ago

ivan-kleshnin commented 9 years ago

May be useful if we want to validate every field onBlur (with or without with validation on submit)

jurassix commented 9 years ago

Can you provide more details on what you would like to see?

You can currently validate a single field or all fields through the isValid([fieldName]), and attaching that revalidation call to an onBlur handler is straightforward.

jurassix commented 9 years ago

I'm still a little fuzzy on your requirement here. If you can explain in a little more detail that would help me understand. Thanks.

ivan-kleshnin commented 9 years ago

isValid calls full validation set independently of field argument:

isValid: function(field) {
  return ValidationFactory.isValid(this.validate(), field); // `this.validate()` passes through every item in `validatorTypes`
}

It just returns one field message.

There are many validation strategies on frontend like validate whole form on submit, validate single field on blur, validate field on typing with debounce and their combinations. I think react-validation-mixin better be agnostic to those strategies and provide low-to-middle level toolkit to make each one of them possible. What's your opinion on this?

jurassix commented 9 years ago

I agree that this mixin is to provide a low-to-mid level toolkit and expose the minimal API that has maximum flexibility.

If you checkout the tests for this mixin you will see that the isValid is an overloaded API and will test the validity of only a single field if one is provided or the validity of all fields if no fieldName is provided.

Example:

this.isValid('username'); // returns boolean for validity of only this field

this.isValid(); // returns boolean for validity of all fields in schema

In the above example the this.isValid('username') call will only validate the single field 'username' and return true|false if that single field is valid.

However, when you call the function without any fieldName, this.isValid(), then the entire Object Schema is validated and the return true|false is the validity of the entire form.

getValidationMessages API works exactly the same way.

Does this help? I believe this to be a very flexible implementation.

ivan-kleshnin commented 9 years ago
My sentence was based on older version of react-validation-mixin. I should give a look to this new caching stuff.
jurassix commented 9 years ago

This functionality was existing before the caching layer, but my documentation is probably not very clear.

If you wanna update the README to help explain the API please do.

ivan-kleshnin commented 9 years ago

Yeah, I still don't get it.

1) isValid calls this.validate() independently of field argument >>>

isValid: function(field) {
    return ValidationFactory.isValid(this.validate(), field);
  }

2) validate method applies full Joi set (validatorTypes) to full state this.state.

...
this.__cachedValidationResults = ValidationFactory.validate(validatorTypes, this.state);
...

I was talking about applying one key-value pair of validatorTypes on demand. That's what I mean by validating single field. So, basically, I'm expecting of this.validate to accept field argument and validate only that one field.

The argument for this may be the choice of displaying all current errors above the form. If you validate all them at once you'll get (and print!) all errors at once. Even on not-yet-reviewed fields. With this strategy error list should fill gradually, once field proves it's wrong (loses focus, etc.)

jurassix commented 9 years ago

OK now I see. Let me examine the options Joi provides for accessing a single Key from the Object Schema. If I can gain access to each individual key's schema then this should be straightforward.

ivan-kleshnin commented 9 years ago

Joi accepts both Joi objects and vanilla dicts. Both of them allow to access individual keys. So this should be easy. But it may require to refactor other parts of library... Let me ask you why you extracted ValidationFactory from mixin. To make testing easier?

jurassix commented 9 years ago

Factory provides an abstraction for the validation engine. Joi is isolated inside the Factory and can easily be swapped out, with out the need to modify the Mixin. Mixin uses a generic interface to the Factory, which keeps the coupling loose.

I've almost got this working. Should be pushed by tomorrow, depending on how much time I have left today.

ivan-kleshnin commented 9 years ago

Ok, that's right, imo. Still don't understand about the name. The term factory means object to create other objects and currently that factory is only a bunch of namespaced methods. It does not construct objects.

jurassix commented 9 years ago

In previous versions of this mixin, the factory made more sense. Originally, I had this mixin as the React api and separate repositories that contained the various validation Strategies. I eventually dropped this idea in favor of just supporting Joi.

While working on this feature I've added the Factory/Strategy pattern back in. It will now match your assumptions of being a Factory.

jurassix commented 9 years ago

While researching this issue, I've realized that for the simplicities sake, when I validate a users state against the users defined validatorTypes I have to include the entire set of both. The examples below will illustrate the potential issues we'd face by segmenting the state and validatorTypes; how would you handle Joi refs, with, without blocks? You need the full schema and the full state to satisfy these validations.

I'm still working on this issue but am sharing my finding in case you have an option:

In these simple examples you need to provide the entire schema and entire state to satisfy all validations, trying to create special cases for ref etc will get quite harry, and something I don't think we want.

validatorTypes: {
      newPassword: Joi.string().regex(/[a-zA-Z0-9]{3,30}/),
      verifyPassword: Joi.ref('newPassword')
}
validatorTypes: function() {
  return Joi.object().keys({
    username: Joi.string().alphanum().min(3).max(30).required(),
    password: Joi.string().regex(/[a-zA-Z0-9]{3,30}/),
    access_token: [Joi.string(), Joi.number()],
    birthyear: Joi.number().integer().min(1900).max(2013),
    email: Joi.string().email()
  }).with('username', 'birthyear').without('password', 'access_token');
}
jurassix commented 9 years ago

I think this is a great issue.

We will provide a way to differentiate between a single field validation via validate('username') and a full form validation via validate(). When a user requests this mixin to validate('username') we will validate the username and then subsequent calls to isValid('username') will display any errors; All other fields will not display errors unless they have been explicitly validated. This way onBlur, etc we give the user control over which individual fields have been validated.

Also, when a user requests this mixin to validate the entire form via validate() all fields will be populated with appropriate errors where applicable. Giving the onSubmit, etc API to the user.

I have gotten the Factory and Strategies to work this way as of now, I'm still working on the API layer to expose this to the user.

jurassix commented 9 years ago

fixed in version 3.0.0

Many changes to this release. Please read over the documentation and let me know if this is what you were expecting. You can now lazily validate individual fields or validate the entire form.