peerigon / alamid-schema Goto Github PK

Consider the following example:

var PandaSchema = new Schema({
    name: {
        type: String,
        required: true
    },
    age: {
        type: Number,
        min: 9,
        max: 99
    },
    mood: {
        type: String,
        enum: ["happy", "sleepy"]
    }
});

var panda = {
    name: "Hugo"
};

The attributes age and mood are not required, but other validators will evaluate anyway. Is this wanted behaviour? Imo this should not be the case.

As we like mongoose, this is was mongoose says:

Validators are not run on undefined values. The only exception is the required validator.

I think setter & getter should work out of the box. What do you think @jhnns

How am I to save this schema" Mongoose/Mongo won't do it; you do not show how.

Lousy documentation.

Wouldn't it be much nicer to use a module for merge.js instead of self-maintain this functionality?

A module would allow to drop merge.js + tests -> less maintenance.

My suggestion for an api:

schema.fields("name", "age", "location").validate(..);

Internally the fields()-method just creates a new object that inherits from the schema and overrides the key-field:

Schema.prototype.fields = function () {
    var subset = Object.create(this);

    subset.keys = Array.prototype.slice.call(arguments);

    return subset;
};

What do you think @meaku ?

@flootr can you check if min works with strings?

It would be nice to have a match validator which is applicable to String types and evaluates regular expressions.

For example:

var panda = new Schema("Panda", {
  name: {
    type: String,
    match: /^A/,
    required: true
  }
});

The (custom) validator:

function match(regexp) {
    return function match() {
        return regexp.test(arguments[0]) || "regexp-mismatch"
    }
};

var panda = new Schema("Panda", {
  name: {
    type: String,
    validate: match(/^A/),
    required: true
});

What do you think @jhnns and @meaku ?

All methods and fields related to writeable and readable should be part of a separate plugin with a name yet to be found. @flootr maybe you can think of a good name and refactor?

I think the schema should only have one dedicated key for validators. I'd just use "validate" and kick the support for "validators". It makes always things more complicated if you can either use this or another style.

It would be quite handy if it is possible to pass a key and the according value as arguments to validate().

Like this:

myCoolSchema.validate(key, value, function (validation) {
  if (validation.result) {
     console.log("I knew it!")
  }

  // ...
});

I dont know if this an issue for alamid-schema but how can we handle an update request to delete an optional key. Alamid-schema doesnt know about requests, but maybe there should be a method to apply a diff. Or maybe this should be done by the model?

How do we handle extra keys? Are they allowed? Does it depend on the schema?

Also related to normalization #9 #10

Mongoose has different implementations of most validators like required to check for the certain type. I think we should do it the same way.

I think it's bad practice that a lib brings its own shim. alamid-schema should just use the Promise constructor. It's up to the user to add this shim to the environment.

Implementation vs. README.md

Should defaults be a plugin or work out of the box?

Validate should return a promise if no callback given

The current tests only check if the builtin validators have been applied. But it should also be checked if they work correctly.

I think it would be a more comfortable API for users of this module if plugins could be loaded like this

const Schema = require('alamid-schema');
Schema.use('validation');

rather than

const Schema = require('alamid-schema');
Schema.use(require("alamid-schema/plugins/validation"));

Imho this is more readable. The required steps to load this plugin can easily be done by alamid-schema.

It seems like mongoose cannot handle the normalized schema definition (for example type: "Array" will produce an error like "string is not a function"). Currently the schema definition needs to be copied on the application's side before passing it to alamid-schema.

I know this seems like "stupid" discussion, but I think naming is important and short and descriptive names are always a sign of a good api.

Imho in the validation context using errors is sufficient. But this is somehow related to our discussion regarding whole model errors. If there are these kinds of errors errors would not be descriptive enough.

So we should discuss whole model errors first and then decide here.