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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow for custom validators/sanitizers #7

Open
mikestaub opened this issue Jul 21, 2018 · 5 comments
Open

allow for custom validators/sanitizers #7

mikestaub opened this issue Jul 21, 2018 · 5 comments

Comments

@mikestaub
Copy link
Contributor

I like the fact that this is just a declarative wrapper for the validator lib, but what if we have some custom logic we would like to apply to a field?
How should this be done? I will be happy to submit a PR based on your suggestion.

@sorodrigo
Copy link
Owner

sorodrigo commented Jul 22, 2018

I see three possible APIs for this:

  1. We add support for a key thats named customValidator and customSanitizer inside the property rules object. The main issue here is that we want to avoid clashes with validator.js (improbable but possible). So maybe we need to namespace this props to pvCustomSanitizer or pv_customSanitizer.
name: {
    isLength: {
      options: {
        min: 6
      },
      errorMessage: 'Minimum length 6 characters.'
    },
    isUppercase: true,
	pv_customValidator: function myCustomValidator(value) { ... }
  }
  1. The second option is to add a third schema where you define custom handlers. I don't like this that much cause entails adding a new step to the validation-sanitizing pipeline.
const validators = { ... };
const validators = { ... };
const customHandlers = {
  validator: { name: function myCustomValidator(value) { ... } },
  sanitizer: { name: function myCustomSanitizer(result) { ... } }
};

const ContactValidator = ProxyValidator(validators, sanitizers);
  1. The third option is maybe too smart but could work. We add support for validation/sanitizing rules as functions. Right now we check if it's a boolean or an object and we do different things depending. We could add another check to see if it's a function and just ignore validator.js in these cases.
name: {
    isLength: {
      options: {
        min: 6
      },
      errorMessage: 'Minimum length 6 characters.'
    },
    isUppercase: function myCustomValidator(value) { ... }
  }

@mikestaub
Copy link
Contributor Author

I think the 3rd options is the best. It should be straightforward to implement.

@sorodrigo
Copy link
Owner

sorodrigo commented Jul 22, 2018

Ok I agree. If we’re messing with the boolean/object value of the schemas. We might want to fix in the same PR an issue I just discovered. 😕

There can be validators that receive three arguments. We’re only covering the cases where there’s two. So maybe we could add support for the value inside options to be not just an object but an array too, this way we can spread it.

What do you think?

@mikestaub
Copy link
Contributor Author

I like the idea of it being an array to support multiple arguments. There should be some more tests to cover these use cases.

@sorodrigo
Copy link
Owner

I agree, we can work together on it. We can split the work.

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

2 participants