Beautify constructors

[ruimarinho]

This PR introduces a new simplified API to reduce the object construction weight of the current asserts.

The new API is completely optional and loosely inspired by the micro validation library is.js. The idea is to improve how asserts are created.

Before:

var Assert = require('validator.js').Assert;

var object = {
  name: 'foo',
  email: 'bar',
  firstName: '',
  phone: null
},
constraint = {
  name: [new Assert().NotBlank(), new Assert().Length({ min: 4, max: 25 })],
  email: new Assert().Email(),
  firstName: new Assert().NotBlank(),
  phone: new Assert().NotBlank()
};

After:

var is = require('validator.js').Assert;

var object = {
  name: 'foo',
  email: 'bar',
  firstName: '',
  phone: null
},
constraint = {
  name: [is.notBlank(), is.ofLength({ min: 4, max: 25 })],
  email: is.email(),
  firstName: is.notBlank(),
  phone: is.notBlank()
};

So, in short, just sugar for a more natural and concise API, resulting in more readable code (in my opinion).

In the process of maturing this approach, some unrelated issues have also been fixed or changed:

• All asserts can be used in their camelCase form, e.g. NotBlank/notBlank. Special cases have also been handled (e.g. IPv4/ipv4 instead of iPv4).
• Added the following aliases for a more fluent API:
  • IsString -> String (is.string())
  • HaveProperty -> PropertyDefined (is.propertyDefined())
  • Length -> OfLength (is.ofLength())
• The extra asserts available via extras.js were manipulating the validator.js module prototype directly instead of using the module's own extend (BC):

Before:

( function ( validator ) {
  validator.Assert.prototype.Eql = function ( eql ) {
    this.__class__ = 'Eql';

After:

( function ( validator ) {
  return validator.Assert.extend({
    Eql: function ( eql ) {
      this.__class__ = 'Eql';
    }
  })

• The Assert object which had been the result of an extension (i.e. instance of Extended) would not be extendable since static methods were not being copied over.

Before:

var Assert = Validator.Assert;
var ExtendedAssert = Assert.extend({
  Integer: Number.isInteger,
  NaN: Number.isNaN
});

// TypeError
ExtendedAssert.extend({
  Finite: Number.isFinite
});

After:

var Assert = Validator.Assert;
var ExtendedAssert = Assert.extend({
  Integer: Number.isInteger,
  NaN: Number.isNaN
});

var ExtendedAssertFinite = ExtendedAssert.extend({
  Finite: Number.isFinite
});

expect( validate( 10, new ExtendedAssertFinite().Finite() ).to.be( true );

• Constructors can now be instantiated using the upper or lower capital case wording and without the new keyword. This should allow more elegant module requiring (unless ES2015 modules are used).

Before:

var Validator = require('validator.js');
var validator = new Validator.Validator();

After:

var validator = require('validator.js').validator();

Tests are also passing.

[guillaumepotier]

That looks fantastic!

I'll try to have a deeper look this week or next one :)

Best

[ruimarinho]

Awesome :) updated the README on a separate commit to discuss whether to showcase the new API there or not.

[ruimarinho]

Sorry to ping on this, but I'm eager to try this out :)

[ruimarinho]

Hey @guillaumepotier - do you have any chance of looking at this soon?

[fixe]

ping @guillaumepotier

[ruiquelhas]

This looks great! I hope it gets merged soon.

[promag]

Wouldn't it be better to have an alias to do: var is = require('validator.js').is;?

[promag]

Finally we can drop new Assert(). all over the place.

[ruimarinho]

@guillaumepotier can you provide some feedback regarding this issue? Would you be open to accept new maintainers?

[guillaumepotier]

Hi @ruimarinho,

Again, 10000x sorry for my unacceptable late answer, pro and perso life have been quite hectic these days, too much things to do..

Now I'm back, and all seems pretty awesome, except my only one comment about browser support. We'll have too choose if either we support IE8 and rework this small piece of code or if we drop it. I think we could go with it since IE8 support is over and this is not a good thing to encourage its (too long) agony. So, if ok for you this is ok for mec.

Regarding your last question about maintainers, yes I'm perfectly open to that (already did on Parsley.js and it works like a charm) and if you're interested, looking to what you've done here, I'd be glad to have you on board to help here.

Best and again, sorry for the late answer :(

[ruimarinho]

@guillaumepotier, no worries at all! I know exactly how those days are :) I'm glad you're back though!

Thank you for reviewing the PR - I think regarding browser support, I'm not even sure if this should be a use case officially supported by the library. Have you considered simply exporting a CommonJS module for node and let browserify and/or webpack deal with any necessary shims? I find it quite hard to maintain a double purpose module as the technologies advance in very different paces.

I would be glad to provide some help here certainly along with my co-workers @fixe and @nunofgs as they have been fundamental to my contributions besides their own too. We should discuss the future path for the library, namely ES2015 internals or not, possible introduction of babel for transpiling to older platforms, etc, etc., so that we're all in agreement too.

[guillaumepotier]

Have you considered simply exporting a CommonJS module for node and let browserify and/or webpack deal with any necessary shims?

I admit that I did not looked deeper in that way and my knowledge on that cool kids tools is yet a bit limited :) in fact, I'd be interested to see the usage of the lib, if it is done more on the browser side or the server side. But I think this should not be reserved for only the backend or only the browser, could be handy in both cases.

I've added you as a repo contributor and merged this awesome PR, so feel free to open discussions with your collaborators and improvements that could be done here, I'm open minded, just lacking some time to do things :)

Best

[guillaumepotier]

EDIT: I'll let you the pleasure to merge your own PR that I made you wait already too long :) Best

[ruimarinho]

@guillaumepotier I'll ask @nunofgs and @fixe to review this PR and merge it. Can you add them as collaborators too?

Regarding the browser debate, we might get away with simply making sure any of this new tools (webpack, browserify, etc) are compatible with the server side code.

[guillaumepotier]

@guillaumepotier I'll ask @nunofgs and @fixe to review this PR and merge it. Can you add them as collaborators too?

Done. Enjoy

[madeiras]

@ruimarinho I was turning my head around an error I had while using this branch. Under the same conditions and code, this branch threw Maximum call stack size exceeded when having a nested is.collection(), here's an example:

'use strict';

/**
 * Module dependencies.
 */

const Constraint = require('validator.js').Constraint;
const Validator = require('validator.js').Validator;
const Violation = require('validator.js').Violation;
const is = require('validator.js').Assert;

/**
 * Instances.
 */

const validator = new Validator();

/**
 * Validate data using constraints.
 */

function validate(data, constraints) {
  const errors = validator.validate(data, new Constraint(constraints, { deepRequired: true }));

  if (errors !== true) {
    throw new Error();
  }
}

const etc = [{
  foo: 'qux',
  biz: [{ foo: 'bar' }]
}]

validate({ etc }, {
  etc: is.collection({
    foo: is.notBlank(),
    biz: is.collection({
      foo: is.notBlank()
    })
  })
})

Please take a look at this before merging :)

Other than this, I've been using this for almost a month and I haven't had any problems with the other varied asserts.

[fixe]

Thank you @guillaumepotier.

[ruimarinho]

Heads up - a fix has been pushed which solves the issue @JoaoHenriquePereira was experiencing. Thanks for the isolated test case! @fixe @nunofgs, this is ready for review and merge.

[ruimarinho]

@guillaumepotier can you cut a new release for this? 2.0.0 due to the Object.keys change?

[guillaumepotier]

Indeed. Done

[ruimarinho]

Thank you 🚀 awesome!