guillaumepotier / validator.js

Powerful objects and strings validation in javascript for Node and the browser
http://validatorjs.org
MIT License
255 stars 39 forks source link

Add When assert #53

Closed franciscocardoso closed 8 years ago

franciscocardoso commented 8 years ago

This PR adds a When assert, inspired by joi. In order to achieve this we've needed to expose the validation context.

guillaumepotier commented 8 years ago

Seems great. But could you provide a real world example for this assert? I cannot find a real case it could suit me for example ;)

franciscocardoso commented 8 years ago

Hi,

Sure. Picture the following object:

{
  country: 'US',
  state: 'California'
}

The validation context makes possible to access the country value in a hypothetical State assert and match the value against a country specific list.

 assert = {
   state: new Assert().State(/* value of country available here  */)
 }

Regarding the assert. Lets say you just want to allow registration of users from California, New York and London. You can do something like:

assert = {
  country: new Assert().Choice([ 'UK', 'US' ]),
  state: new Assert().When( 'country', {
    is: [ new Assert().EqualTo( 'US' ) ],
    otherwise: new Assert().Choice( [ 'London' ] ),
    then: new Assert().Choice( [ 'California', 'New York' ] )
  } )
}

WDYT?

guillaumepotier commented 8 years ago

Hum,

I personally find it quite complicated and a bit flawed in your above example: what if you have many more countries?

I'd find more useful or elegant the usage of a second Choice assert like:

choicesAssert = {'UK': ['London'], 'US': ['New York', 'California']};
assert = {
  country: new Assert().Choice([ 'UK', 'US' ]),
  state: new Assert().Choice(choicesAssert(dynamicvalueofcountry)),
}

But dunno how to do it :)

franciscocardoso commented 8 years ago

Maybe the example I gave was a little bit "academic". For complex situations we can use Assert.extend() to add a custom assert, maybe something like a StateByCountryAssert, or something like that, in this country/state situation.

The when assert is/would be useful to avoid the necessity of creating new asserts. Imagine (in the same example/object) you want to make state required for US users only:

assert = {
  state: new Assert().When( 'country', {
    is: [ new Assert().EqualTo( 'US' ) ],
    then: new Assert().Required()
  } )
}

Does this make more sense?

franciscocardoso commented 8 years ago

Hi @guillaumepotier,

Today I run in another use case, also country/state related, when developing a third party integration.

Check out the following requirements from the partner api documentation:

Field Type Length Required Description
country string 3 ISO 3166-1 alpha-3 code. Defaults to USA if not specified.
state string 2/55 ? If country is USA or CAN, 2 digit state code required.

Solutions

Using When

We would be able to do something like:

assert = {
  country: [new Assert().String(), new Assert().Length({ min: 3, max: 3 })],
  state: new Assert().When( 'country', {
    is: [ new Assert().Choice( ['USA', 'CAN'] ) ],
    then: [new Assert().Required(), new Assert().String(), new Assert().Length({ min: 2, max: 2 })],
    otherwise: [new Assert().String(), new Assert().Length({ min: 2, max: 55 })]
  } )
}

Using a CustomAssert

The alternative is to create a custom assert to validate state if country exists. Something like:

Please note that this is all pseudo-code and the real solution will require even more code:

export default function stateAssert(country) {
  this.country = country;

  this.validate = value => {
    if (!this.country) {
      return true;
    }

    if (typeof value !== 'string') {
      throw new Violation(this, value, { value: Validator.errorCode.must_be_a_string });
    }

    if (country === 'USA' || country === 'CAN') {
      return value.length === 2;
    }

    return value.length > 2 && value.length < 55;
  };

  return this;
}

Then we need to extend the validator asserts with this custom assert:

import { Assert as BaseAssert } from 'validator.js'

Assert = BaseAssert.extend({ StateAssert })

export Assert;

And then we could use it:

assert = {
  country: [new Assert().String(), new Assert().Length({ min: 3, max: 3 })],
  state: new Assert().State(country) // Note that we need to implicitly pass the country as we don't have access to the context.
}

This solution requires extra tests to the custom assert, etc.

WDYT?

guillaumepotier commented 8 years ago

Ok,

The above examples make much more sense now :)

franciscocardoso commented 8 years ago

Hi @guillaumepotier,

Do you think you'll have time to review/merge this PR soon? I'm really needing the assert.

Thanks in advance and best regards.

guillaumepotier commented 8 years ago

When assert is good. I'm just a bit perplex for the multiples context passing in various methods. This is not wrong, but feels a bit inelegant. Do not have much time to think about it and did not find a prettier solution :(

ruiquelhas commented 8 years ago

@guillaumepotier about your latest concern, I also don't see an easier solution without a major rewrite. Do you still think this is not a useful feature? If not, I would urge you to merge it as is, and maybe improve it later.

guillaumepotier commented 8 years ago

Yup you can go ahead and merge it.

Best

fixe commented 8 years ago

Examples could be better but we can improve documentation later on.

fixe commented 8 years ago

@guillaumepotier can you tag a new release?

guillaumepotier commented 8 years ago

Should be good now.

franciscocardoso commented 8 years ago

Thanks 🚀

jods4 commented 8 years ago

@guillaumepotier It seems the contents of dist have not been built since 2.0.0? When I get the lib with bower, either the 2.0.2 version or directly the github tag, I receive an old lib version that does not contain When.

guillaumepotier commented 8 years ago

Ok @jods4 my bad. 2.0.3 should have the proper built dist files now.

Best