RFC: Joi plugins/extensions/custom functions

[Marsup]

See https://gist.github.com/Marsup/14597d0c8eaa10c4addb for latest version of the RFC.

[totherik]

This looks like a good start, but one major issue is that it's using what is effectively a global in an unsafe way, potentially leading to naming collisions and unexpected behavior.

For example, imagine if module A and module B both depend on the same version of Joi and both declare a type Foo. The module will be deduped and they'll both reference the same instance of the Joi module, meaning they now compete for the same name, Foo, in the same namespace. This isn't a terribly unrealistic scenario and the collision would be very difficult to debug.

[Marsup]

As written, any attempt to override a type will result in an error, I don't think this should be a problem to debug.

[totherik]

Ah, ok, I missed that part.

If I depend on A and B and an error is thrown, I have no recourse or ability to remedy the situation as I don't own those dependencies and those authors could not have anticipated being I the same dependency graph as each other.

This would require module authors to define globally unique (as in unique from every other package in npm) types.

Also, as a module author I can say that having to come up with and define globally unique names via trial and error is undesirable .

[Marsup]

I'm not sure it's preferable to locally load all extensions. Uniqueness will be partially guaranteed by npm naming, for other cases I don't see how we can solve that.

[totherik]

Uniqueness will be partially guaranteed by npm naming

Can you clarify?

Also, I understand I'm the worst kind of commenter. I identify a problem but, unfortunately I don't have a solution. (Need to think on it more.) Sorry about that.

I can say that at my company where we would need to define types for validation in things like service wrappers, this mechanism would not work and we would not be able to use it due to the high likelihood of name reuse.

[Marsup]

If people publish types as modules named joi-type, that should avoid many conflicts, but that doesn't fix the internal ones. So you'd rather require all extensions each time in all your files?

[AdriVanHoudt]

1. I really like the spec and how it opens up Joi for custom stuff
2. Maybe add some docs for the handlebars in language where you reference parameters?
3. Will Joi itself now use this system? Because you can basically rebuild Joi with this right?

[Marsup]

I don't believe the templating language is relevant for this proposal but yes it'll have to be documented. Joi need to maintain a bunch of core features because of parameters validation.

[rlidwka]

1. Maybe rename Joi.addType to Joi.extend for consistency reasons?
2. What the difference between convert function Joi.addType and validate function in Joi.<type>.extend? As far as I see, both can change value, and both can throw on error.
3. I don't see a way to access arguments passed to a type. In your example, Joi.user(options), options seem to be inaccessible.

[Marsup]

1. Yeah why not, based on the options given, it should work. The Joi object is an Any currently though, you'd be extending it, I'll have to see if it's feasible.
2. The convert has a chance to do something before anything else does, this is a post-base check if you will. I've thought of this because of an issue about mongodb's ObjectId: if you need to coerce your value to a specific type (in this case new ObjectId(value)) for any other extension method to work, you have no place to put it.
3. There are no options passed to a type, what do you mean ? The options used to build a type or a method are not the concern of the user, hence not exposed, should it be ?

[myndzi]

Really excited about this, and bonus: asynchronicity!

I think maybe you misunderstand the namespace problem brought up by totherik. He's not talking about uniqueness as an NPM module, he's talking about uniqueness in the Joi namespace. I agree, this is a problem. It can be solved fairly readily, however, by allowing Joi to be instantiated rather than using it as a singleton, e.g. var Joi = require('joi')(options?);

It would then be up to the developer to share the Joi instance around, but this is trivial in Node.js since you can just create a simple wrapper that also defines your extensions:

var Joi = require('joi')();
Joi.extend(etc...)
module.exports = Joi;

You can drop the requirement for the () by making it instead a method of Joi itself, like Joi.giveMeANewDefaultInstance() (can't think of a good name!)

Joi has long been my favored validation library, but I've wound up using it almost nowhere because of the two things this proposal will address. Happy happy joy joy!

[myndzi]

Re: the question about cloning/freezing or trusting the developer, I'm in favor of the latter. Solid enforcement appeals to me, but one of the most useful use cases for Joi is in things like HTTP requests, where the performance difference can be solidly beneficial. A suitable compromise is to allow the user to specify an option about whether to freeze the objects or not, and this option could be used when running tests or development, for example, but not in production.

[totherik]

Thanks for clarifying my comments @myndzi. (Weekend away with the fam so trying to stay offline :) )

So with the pursuit of types-as-npm-modules, is it reasonable to expect that a single application/module may need to depend on incompatible versions of the same npm module type definition? If so, npm can't be used to manage those dependencies. (I'm thinking major revisions of types that aren't required to be backward compatible according to semver.)

I don't know all use-cases, but I have to imagine this could be an issue.

[Marsup]

I understood what he meant, npm is indeed a "trick", not a solution, but it might avoid some basic conflicts.
Having multiple versions of your models is indeed a problem (joi or not), but isn't it better to have an error thrown to tell you about this mistake rather than having 2 separate parts of your code supposedly manipulating the same thing and have different versions of it ?
If it is an npm module, it's probably wiser to have a single version in your dependency tree, and if it's part of your code, maybe you'll have to make this explicit, like rename type user into v1user and create a v2user if you create a new version of your API. What do you think ?

I kind of agree on the freezing/cloning/vanilla problem, if you really want to mess up your schemas, that's your responsibility, can't protect users from everything.

[myndzi]

I don't see how the name of the NPM module affects the name of the thing extending Joi in any way, really. Validation is useful for lots of things, and the same name can mean different things in different contexts. Namespacing is a troublesome solution, but extending separate instances seems like a clean way around it...

I'm not sure you do understand it, since throwing an error is out of the question. It's not a mistake so much as a circumstance that can arise as a result of two separate modules using Joi for validation. It's not about describing a model and manipulating it from two angles, it's about the fact that node.js modules are inherently singletons, so you run into namespace problems when you use multiple distinct modules that utilize joi for validation...

[myndzi]

Let's take your mongo example as a base. It extends Joi with a type called 'user'. Say you add in something like Passport, which has also utilized Joi for validation and has a 'user' type of its own with a different meaning. You, as the developer, can do nothing about the fact that you're using two modules that want the 'user' namespace, but only one is allowed to have it, unless you do evil things with the path structure which leads into undefined behavior. It's not an error that the mongo module is defining 'user', and it's not an error that passport is defining 'user'. Throwing doesn't solve it, and the developer has no tools to solve it either. Allowing the modules to utilize their own individual instance of Joi, however, cleans this kind of scenario up nicely :)

[AdriVanHoudt]

@myndzi This can easily be solved by the authors of the modules by choosing more unique names like passport-user but it is indeed a concern that you may run into duplicates which leads to not being able to combine certain modules if the authors don't want to change their names

[Marsup]

I perfectly understood the problem thanks. I'm trying to find alternatives to that, because what you're proposing doesn't make sense to me. If you inspect your API, you will find several validations with the same type but not the same underlying concept, that bothers me.

I have not made this proposal just to offer my own version of custom functions, which would be much simpler if you look at previous PRs about it. Joi is also built for documentation through code, I intend to keep it that way.

Now we could introduce an optional namespace property that would force you to do Joi.myModule.myType but I'm not even sure a single level will please everyone. I can't think of any way to have what I want without some kind of uniqueness.

[AdriVanHoudt]

Any thoughts on alias support? Joi.alias(<type>, <otherType>) e.g. Joi.alias('valid', 'only')

[Marsup]

What do you think if addType just returned a new type and let you attach it to Joi if you really want it ?
Like Joi.user = Joi.createType({ ... }) (createType makes more sense considering the behavior).

[AdriVanHoudt]

So a module would create a Joi type and export it, so you can add specific types to your custom Joi?

Something like this?

var mongoUser = require('joi-mongo-user');
var passportUser = require('joi-passport').user;
var Joi = require('joi');

// either this
Joi.user = mongoUser;
Joi.passportUser = passportUser;

// or this
Joi.user = passportUser;
Joi.mongoUser = mongoUser;

module.exports = Joi;

joi-mongo-user

module.exports = Joi.createType({...});

joi-passport

module.exports = {
    user: Joi.createType({...}),
    otherKey: Joi.createType({...})
}

I like it.

Although I still like the idea of Joi.object().isUser() since you can clearly see the base type.

[Marsup]

I've updated the proposal, see the diff in https://gist.github.com/Marsup/14597d0c8eaa10c4addb/revisions.
Does it address your concerns @myndzi and @totherik ?

[AdriVanHoudt]

So Joi.extend() will not extend the singleton Joi but return a 'new version' of it right? Whereas the Joi.type.extend() can do both(taken from the examples)? As for the rest I like it!

[Marsup]

If anyone is still interested it's published on tag 9-beta, still waiting for feedbacks...

[MarkHerhold]

I'll look at it next week! :)

On Thu, May 5, 2016, 4:26 PM Nicolas Morel notifications@github.com wrote:

If anyone is still interested it's published on tag 9-beta, still waiting
for feedbacks...

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#577 (comment)

[kamronbatman]

I'll be ready to test it in a week or two. Sorry for the delay.

[svestka]

From documentation I would expect, that when pre method cast value, validation would be run against this casted value. Imho this code should NOT throw an error:

let joi = require('joi')
let joiCustom = joi.extend({
  base: joi.boolean(),
  name: 'alwaysTrue',
  pre(value, state, options) {
    return true
  }
})
joiCustom.alwaysTrue().validate('whatever')

[Marsup]

@peterslivka have any real use case ? It feels like your base should be Any.

[svestka]

Yes, with Any base it works, but you loose boolean validation. Real use case is may be localization, e.g.:

let joi = require('joi')
let joiCustom = joi.extend({
  base: joi.boolean(),
  name: 'booleanLocalized',
  pre(value, state, options) {
    if(value==='yes' || value==='ja' || value==='ano') return true;
    if(value==='no' || value==='nein' || value==='nie') return false;
    return null;
  }
})
joiCustom.booleanLocalized().validate('ano') // 'ano' should be true, but it fails validation

[Marsup]

So you don't trust your own code to return the type you want ? That doesn't seem like a valid use case to me. I feel like this is the kind of rule that should be checked in your tests, not in joi each time a validation runs.

[svestka]

Current boolean validator also covert "yes" or "on" string to true value. For me this is the very same use case. My point is not to build full-feature validator covered by tests, but to extend existing. This is not possible because if value is not valid by base validator, it will never reach preprocess function.

[MarkHerhold]

I really like this but I don't see a way to extend everything in one go. Maybe this was by design?

If I extend Joi.any(), I would like to be able to do:

• customJoi.number().myCustomRule()
• customJoi.string().myCustomRule()
• customJoi.any().myCustomRule(), etc.

without defining multiple custom Joi objects for each type via extend().

[Marsup]

@peterslivka ok, I was trying to minimize the number of hooks but you might have a point.
@MarkHerhold I don't yet have an idea about how to do it properly without making you recreate all the types, the problem is any is already the base of all joi types, so extending it doesn't impact others, which makes sense as far as inheritance goes, but it's not what I would want either.

[nfcampos]

@Marsup one possibility would be cloning any on any call to extend, irrespective of which base the extensions have, and then recreate all the internal joi types inheriting from the cloned any. That would be a completely independent clone of the original Joi object with all types inheriting from this new clone of any. I believe this way an extension with base any would correctly appear on all other types. Obvious downside is this takes up more memory. (edit: on re-reading your comment this is probably what you meant by 'making you re-create all the types')

[Marsup]

@peterslivka another beta out (9.0.0-4) with a new coerce hook.

[MarkHerhold]

@Marsup The coerce hook is very useful (and necessary!). Great addition.

[Marsup]

Just broke the previous contract in 9.0.0-8 because it seems people do weird things with Joi, you now always have to return the value in extension functions.

[MarkHerhold]

I converted my plugin/extension to use the values instead of null and everything still works great. 👍 Easy change.

[arthurvi]

Are there any plans to make async validation possible in the validate function? I saw some older discussions about this subject in the repo but don't know what the plans are for the near future?
cc @Marsup

[Marsup]

It's planned. Can't promise it's "near" future.

[lukrizal]

Is there any possible approach for async?

[kamronbatman]

I vote for Bluebird.

[rafaelcorreiapoli]

Any updates on this ?

[bodinsamuel]

Hi everyone, any news ? :)

[mehulmpt]

Is there any update on adding async validation?

[bodinsamuel]

This issue seems abandoned, so I made a little side project to handle async validation with a syntax pretty much similar to Joi, if you are interested: https://github.com/bodinsamuel/altheia

[Marsup]

It's not abandoned, you're just all commenting on an issue that's been closed forever and everyone moved on to another, namely #1194.