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

Incompatibility with `Extend` and `Pretty Constructors` #57

Closed franciscocardoso closed 5 years ago

franciscocardoso commented 8 years ago

Entend

The Assert class can be extended by using theExtend feature. Besides adding custom asserts this allows us to override any of the asserts available in the module. Let's say that someone needs to tweak the behaviour of an existing Assert, Required per instance.

You can achieve this by doing something like this:

const BaseAssert = require('validator.js').Assert;

// Load your customer asserts, including your tweaked Required.
const customAsserts = require('path-to-custom-asserts');

module.exports = {
  Assert: BaseAssert.extend(customerAsserts)
};

And you are done. At this point you can use your own Required assert.

new Assert().Required();

Pretty constructors

With the introduction of this feature we can write this with less code, which is awesome. You can simply write:

is.required();

Problem

The behaviour will be different for each of the calls.

new Assert().Required(); // This will result in using the overridden assert.
is.required(); // This will result in using the original assert.

This is due to the cache mechanism added to the _prettify method which prevents any existing Assert from being overridden.

Solutions

I believe that the solution will depend on the desired behaviour.

How should we approach this issue?

franciscocardoso commented 8 years ago

WTYD @guillaumepotier ?

guillaumepotier commented 8 years ago

Hi, thanks for this quite long and explaining issue.

I think this is related somehow with #29 as the original Required were kinda bloated no? I mean, user should not have a need to override an existing Assert as they should be atomic (not the case for Require) and "built-in" right?

We should allow user to define its own Assert, and that should be done in the pretty way too like the others built-in, but not being able to override them does not seem a serious matter to me.

franciscocardoso commented 8 years ago

Hi,

Indeed, one thing leads to another. I'll create a PR disallowing override asserts when extending.

Thanks!