Since I have been thinking about similar things working on SimpleSchema v2, here are my thoughts:

• In general I like your syntax proposals
• The feedback I get seems to be about 50/50 for required default vs. optional default. I decided to please almost everybody by keeping required default but having an optionalByDefault setting for the whole schema. Pretty sure this will make it into the final release.
• I sometimes wish I had not gone down the road of allowing functions. The schema would then be reliably serializable, allowing sending schemas over DDP or HTTP and storing in the DB. (Which is still possible if you don't use functions, but not reliably possible.) So if you force all custom logic into a serializable (EJSON) custom validation object, which would just need its logic in both client and server code, then you could support that.
• As @stubailo said, we should all use mdg:validation-error so we can be compatible. Whether you use it directly or wrap it with Astro.ValidationError would be up to you.

• The idea with the config option is nice. I think I will use it too. Thanks :)
• You know, using function is sometimes the only way to achieve more complicated validation rules. Of course I could assume that validators are only for checking simple rules like text length, regexp etc. And move complex rules to application logic. In such situation developer would be responsible for performing additional checks in some custom meteor methods. However there may be situations where calling Doc.save() or Collection.update() should also execute these complex validation rules. I would definitely want to avoid functions and make it as simple as possible. There was also request for support for JSON schema and with validation function such support would not be possible.
• Hmm mdg:validation-error has SimpleSchema as a dependency which is pointless when using Astronomy. Of course, I'm for the compatibility and I can follow the ValidationError error schema but what benefits do we have from such compatibility? It's still up to developer how is he/she gonna display errors. Under the hood it's still Meteor.Error

It's still up to developer how is he/she gonna display errors.

I'm hoping that someone can build a form package that will be compatible with all kinds of different schema/model packages. One necessary thing to do is to standardize how errors are reported.

Also, this means that I can write a forms section for the Meteor Guide that is compatible with Astronomy.

@stubailo If we want to have forms package that would be compatible with all kinds of schemas packages then MDG would have to provide some base that we should build upon. Right now I have this problem that I don't know where is Meteor heading. It's changing too fast its core principles to keep up with packages updates.

However, if it goes about ValidationError class. I won't use it from the mdg:validation-error package because it's has to many dependencies. It's fairly simple class and such class should be in the core of Meteor.

Would it not have been just awesome if astronomy and simple-schema/collection2 shared some common definitions, for example a schema format (JSON schema?) and a common extensible custom validator registration API (which would then allow @aldeed to get rid of functions)

Astronomy and simple schema / collection2 would then depend on that common base to improve in their own opinionated directions.

then MDG would have to provide some base that we should build upon

ValidationError is the first of those. By not putting it inside the Meteor repository (which we will do with more and more core technology in the future), we are actually making it easier to iterate and make improvements. We're going to remove the SimpleSchema dependency.

What else do you need? Perhaps we can make it happen. Since you and @aldeed are the two most prominent schema-related package authors right now, I can't think of anyone else who is in a better position to agree on a schema definition language going forward.

Also, to clear up what I can do here, @justinsb and I are the newly formed Data Team here at MDG, so we're going to be focused full-time on making Meteor's data system better than ever before. It's great already I think, but it hasn't seen significant improvements in 2 years or so, and we want to get on top of that.

@stubailo Ok great!

So if we are in the subject of schemas/models. Is MDG planing any GraphQL integration? Should be consider supporting it?

What are your plans if it goes about improvements to data system?

We're hoping to announce our near-term plans in 1-2 weeks, I'm sure we can cross that bridge when we get there. If you guys can agree on a single schema format, I'd guess it will be the main one for Meteor for at least 6 months to a year, and will probably still be relevant in a lot of places even when something related to GraphQL comes about. I don't know if you've seen GraphQL definitions, but they are quite verbose for simple things.

Ok, can't wait what are you planning.
Yes, we can try to agree on schema format.
I'm quite new to GraphQL, I don't understand all principles yet. I'm just curious if Meteor will introduce another layer for model.

I'm just curious if Meteor will introduce another layer for model.

Currently we aren't thinking about models, it's mostly about the data loading mechanics. If we start thinking about models, we'll make sure to reach out to the relevant people, yourself included.

Oh I see. That's nice :) Thanks!

@aldeed so maybe let's arrange Skype/Hangout meeting, so we can talk about what our plans for SS2.0 and A2.0 are. Maybe we will be able to come up with similar way of defining schema and validators. Even if not, it's always nice to share experience :)

@jagi Sounds good, I will email you.

@serkandurusoy Functions and custom validators are probably the one thing that can't be reconciled between packages because SimpleSchema needs to provide information about and have logic for validating modifiers. The extensibility of the schema options and packages like autoform which read them for alternative purposes add to the mess. We may be able to get the rest of the schema syntax pretty close, but to me JSONSchema is too verbose, and I think type: String vs. type: "string" might be a point of contention, too. I'm leaning more in favor of recommending and helping to maintain conversion packages, so you could pick your favorite schema syntax but wrap your object like myFavoriteToSimpleSchema({ ... }), or convert to JSONSchema, etc. Having similar syntax to begin with will make that easier.

I get the complexities and pain points involved. But then, conversion packages may be even more dangerous because there is no bidirectional and one-to-one mapping, furthermore a they may fall behind as the contenders move along.

Even add-ons have such problems. For example, I'm an avid user of autoform and have tried out semantic and materialize templates only to find out as autoform progressed that I need to either maintain the template add ons myself or revert back to the autoform default bootstrap because that's what is maintained in line with the core by the core developer.

The point is, if we want astronomy and ss/c2 codebases to communicate well the only realistically viable option would be for them to actually share common code. I fear other strategies would only render futile as they progress in different directions.

Furthermore, I also believe that such common core should be provided by a core meteor api so that noone can easily choose to ignore it.

@aldeed Ok I will reply to your email and we will arrange a meeting.
@aldeed @serkandurusoy Right now I'm getting deeper into GraphQL and I'm quite impressed. And I would like to support it in 2.x versions, and that would mean that your way of defining field's type will be better suitable for that.

fields: {
  firstName: String,
  tags: [String],
  address: Address
}

However it does not mean that we will have 1:1 schema definition. I think it will be on my side to create conversion package. Something like Class.toSimpleSchema(). That's because there are more third party packages for SS than for Astronomy. I think we can agree on many things.

Hmm, I think it could be a good idea to borrow some schema definition ideas from GraphQL. Like Geoff said in his forum post recently, we are investigating ways to use GraphQL to improve Meteor data loading, and it will be nice if you can just write your schema once and get a GraphQL endpoint out of it as well!

I think it could be something like Class.toGraphQL(). I like how queries are done in GraphQL, however I don't like the way how schema is defined. However, making the changes in fields types that I was talking about will allow for easier integration with GraphQL. So yes it will be possible to write schema once and get a GraphQL endpoint out of it.

Sounds awesome.

I think having an option to run certain validators on certain type of operation would also make sense. Like if we are able to declare a validator for 'insert' only or 'update' only. By default they would run on all type of operations.

Have you considered something like this for custom validation functions? Having access to other fields is something I needed sometimes in my current SimpleSchemas.

validators: function(field, fields) {
  // field.value; field.isSet
  // fields.fieldName.value; field.fieldName.isSet
}

@zimt28 and how would it work? Does it does something that is not possible with the way I've provided?

@jagi I just didn't see a way to access other fields. Or would they be available via this? SimpleSchema has some more properties we can use, but I'm not sure they're needed. However, the suggested syntax would allow to implement these things later.

Tell me if the following is far away from how you'd actually use Astronomy, but the code would look like this:

{
  password: {
    type: 'string',
    optional: true
  },
  passwordConfirmation: {
    validators: function (field, fields) {
      var password = fields.password;
      if (password.isSet && field.value !== password.value) {
        throw new Astro.ValidationError({ validator: 'passwordMismatch' });
      } 
    }
  }
}

I agree with @zimt28. Being able to use other fields in validation of a field can come in handy. I think you can still manage this by using a hook like BeforeSave, but it would be messy and pertains to validation so should be a part of the validation function signature.

@zimt28 @talha-asad yes I will probably provide access to the document using context (this). So you will be able to access any field in a doc. Custom validation function will probably also receive more arguments. I don't know what yet as I'm going to work on it this week.

@jagi Sounds good 👍

If we could have something like `.isSet(fieldName)`` on the document level it would make sense. Sometimes you want to run a validation only if the document has something explicitly set and not a default value.

@jagi @talha-asad What I like about the syntax I posted above is that it doesn't use the context (this). I like APIs that are consistent and predictable. Using function arguments to get some information, data context to get some other and then functions for something else is looks ufgly, imo.

The examples at the very top use both this.field and then a fieldValue argument. Access to other fields would require usage of the data context and probably some functions. Having the current field as the first, and all fields accessible as the second parameters everywhere would be a clean solution; no function calls required.

@talha-asad for that purpose there will be a method doc.isModified(fieldName) - it will check if a given field was modified also if it differs from the default value.

@zimt28 I can't agree on that. Let's imagine a situation when we have arguments (field, fields). It's ok for now, but what if we want to access some other data in a document, let's say behavior. We need access to entire document. So we have to add another arguments function(field, fields, doc). Putting more arguments on the list is not a good idea. In fact, there is a patter for putting it in the object: function(data) and data object would contain all required information (fieldName, fieldValue, doc etc.).

I agree that using context is not always the best option and maybe I will just provide the data object. We will se when I'm going to work on it today.

@jagi I see. I'm currently using SimpleSchema and planning to make the switch with Astronomy 2, so all my thoughts are based on that background. You're right, having too many arguments doesn't improve the syntax. function(field, fields, doc) would still be fine in my opinion, as you'll use arguments on the left more often and just use them as needed; plus it would allow all the required features. But it shouldn't get more than that three arguments.

However, try it and let us know what you come up with :) My point is just that I'd like a consistent way to access fields 👍

Yes I agree. In events system in Astronomy 2.0 I'm using the event object passed to the event handler. And when you want to access document you do e.target or e.currentTarget the same way as it's done in events in browser. The similar principle can be used among entire Astronomy. So in validators we would have function(data) and to access document data.document or data.doc. I will notify you today about progress.

I like the approach but i don't understand what would e.target and e.currentTarget will ever be? Since most events might not be emitted through an element. Or are you just keeping the interface for the events consistent?

e.target and e.currentTarget is quite easy to understand. You just have to read documentation from i.e. Mozilla. target is an element (document) on which original event was triggered. currentTarget is an element (document) on which event is being call right now. To better understand it in the context of Astronomy. Let's say we have the User class and user has the Address nested field. When you execute user.save() it will trigger the beforeSave event. At first it will be invoked on the User so both target and currentTarget will point to the User. Next, it will propagate to the nested fields and when it reached Address, the target will still be User but the currentTarget will be Address. That's it.

I think I will use similar approach if it goes about validation to allow access to the parent document from the validator of the nested document.

I'm in the middle of work with validation. I haven't been able to finish all things that I wanted. So just give me one more day. It's much more complicated than I thought it will be.

No rush jagi. Take your time. Good things take time. 👍

Thanks for the explaining the events, though i was thinking what will happen if a field is double nested inside a document. Will you still be able to access the root document as it won't be possible using e.target and e.currentTarget as they will both point to a nested field. We'll need this to point on the root document in this case. Is my understanding correct?

e.target will always point to the root document as it's where an event was originally triggered. And e.currentTarget will point to document being currently handled. I was also thinking about allowing something like nestedDoc.getParent() but I think it would complicate things to much.

Couple thoughts for posterity:

• @jagi It's not at all necessary for the schema definitions to be identical. The only real advantage would be to Astronomy in that it could be compatible with other SS dependent packages.
• To jagi's point regarding GraphQL, a simple connector package that layers on this functionality is better than building it into Astronomy core. Astronomy deals with mongo documents, and until that changes they'll just be documents on both the client and the server regardless of how the data is being requested and fulfilled.
• @stubailo A common validation error pattern could be cool but is also not necessary. Forms packages are themselves complicated and must differ between UI frameworks. What is useful here is for each library to have a well-defined api that includes hooks in the appropriate places (both on the schema and form lib side) so that connecting packages can be made.
• The difference between type: 'string' and type: String is pretty irrelevant except for the ease at which custom type definitions (like nested astronomy models) can be serialized in JSON/EJSON. But the inflation of the vanilla JS object into an astronomy model instance happens in an execution space where serialization doesn't matter... we're just talking about serialization of the schema... so... not quite sure why that needs to happen.
• Regarding defining custom validators: as someone mentioned, you can easily make the validations definition just json objects with no functions and still support custom validators. All this requires is a way to define custom named validators, so that when I do

fieldOne: { 
   type: String
   validators: {
       isTheSameAs: {
           field: 'fieldTwo'
       }
   }

my function

function isTheSameAs(value, fieldName, doc, options){
     return value === doc[options.field];
}

will be run (again just using some kind of validator function registration api. Options should be able to be arbitrarily complex, and since I can define methods on an Astronomy model, if fieldTwo happens to be a computed value, that should still work fine with a slight change in syntax.

• Which begs the question of how to pass information to validators... as brought up by @zimt28 . If I can define a custom function as a validator, I should be able to bind it's context if I wish, this and closures are the only way I would be able to get any extra info I need into the validation function without relying on pseudo-global information/state. You need a few things: the value or the name of the field you are validating, the entire document as it is available in whatever environment you are in (e.g. on the server you might have all fields while on the client you might not, which is fine) and the options to your validation function that were specified. These can be passed in a number of ways, but the function context this is definitely not my first choice.

I would agree with @ianserlin last point. Using this will cause problem if the validator function is being bind.

@ianserlin @talha-asad right now context of the validation function is set to the default one provided with the function. All necessary parameters are passed to the function as arguments. It's only a matter of choosing if it should receive some data object or each individual argument. I definitely agree that this should not be changed.

@ianserlin If it goes about custom validators, in deed, it can solve a problem of passing function to validation. However, I don't care about that. We can pass functions. It's especially necessary in the if validator. However, I think for more complex validation developers would still create their own validator :)

I wonder if it's possible to add more custom error message support. Right now I think for most of the built-in validators the error message are developer-centric. But I feel it will be nicer to show more understandable error message directly to the end user. The best thing I can think of is to have "field-level", "validator-level" and "document-level" custom error message support. Taking the sample code of v2.0 that you wrote above as example, I think it'll be nice if we could do:

validator-level custom errror message

  firstName: {
    type: 'string',
    validators: {
      minLength: {
        args: [3],
        error: 'The minimum length is 3'
      }
      maxLength: 20
    },
  }

// Or
  firstName: {
    type: 'string',
    validators: {
      minLength: 3,
      maxLength: 20
    },
    error: {
      minLength: 'Firstname should contain at least 3 characters'
    },
  }

field-level custom errror message

  firstName: {
    type: 'string',
    validators: {
      minLength: 3,
      maxLength: 20
    },
    error: 'Please enter a valid firstname'
  }

document-level custom errror message

{
  fields: {
    firstName: {
      type: 'string',
      validators: {
        minLength: 3,
        maxLength: 20
      },
    }
  },
  error: 'The validation fails'
}

I know it's a lot of work..., but hopefully it will make the validators more flexible :)

It would definitely make code much more complicated. Of course it would be much more flexible. In Astronomy 1.0 there is the validationerror event where you can compose your own validation error for each validation error. It will also be available in Astronomy 2.0.

The problem with your syntax is that it would be almost impossible to distinguish if an object passed to the validator is the validator's definition or the validator's param. I will explain it in more details: There are validators where you have to pass object as a param.

firstName: {
  if: { // it's an object param passed to the validator
    condition: function() {},
    true: /* ... */,
    false: /* ... */
  }
}

and if you would want to pass object with the error message, then it would make hard to distinguish if it's a param or validator's definition.

firstName: {
  if: {
    param: { condition: /* ... */, true: /* ... */, false: /* ... */ },
    message: 'Error message'
  }
}

I don't want to make to many checks of arguments. But I will think about a better more flexible approach.

Thanks for your explanation! I was unaware of the validationError event before, and just checked the documentation. Looks like it could do all three levels custom error.

However, I believe that syntax could cause unclearness when you have lots of fields and want to lots of custom error. As inspired by your last comment and the existing validationError event, I wonder if the sample code below works:

  firstName: {
    type: 'string',
    validators: {
      minLength: 3,
      maxLength: 20
    },
    error(e) {
      // 'e' is the validation error event where e.data.fieldName = 'firstName'
      switch (e.data.validator.name) {
        case 'minLength':
          return 'the minimum length ...';
        case 'maxLength':
          return 'the max length ...';
        default:
          return 'the default error';
      }
    }
  }

It works in consistent with the existing validationError event, and exists on the field-level. When we do event propagation, we could go from field-level to the document-level. So we could stop propagation at the field-level. I feel in this way the error messages are more easily customizable 😄

The most obvious way of generating or just passing error message would of course be your first proposal about passing object a few properties instead just only param. However, as I said it may cause some problems but I will investigate this issue further.

The idea, you've proposed in the last comment seems to be quite nice however you still need some switch-case of ifs. It would definitely be nice to have ability to just provide string error to each individual validator. In fact, it was possible in 1.0. The validator function was taking error message as the last argument. However, that approach was a lot more complicated from the code level and required a lot more of code to be written to define validators.

Yeah I totally agree with you. And the ideas of passing the error message as the last argument also came across my mind before. And right, it is applicable for validators defined in the format of v1.0. For the last comment I made, I feel it's a good tradeoff. Of course you must have done much more research than I do, and I'm sure you will come up with a great choice.

I also like the idea about having consistent schema definition and/or error handling from the discussion above. Really hope you guys could reach a consent on that topic.

Ok validators are working, not everything is implemented yet but there are several things that needs discussion. First I will present you what ideal syntax for defining validators would be which will be very hard to implement. And the second example is a syntax that is easy to implement, does not require a lot of checks but requires a little bit more code to write.

Ideal syntax

fields: {
  firstName: {
    type: String,
    validators: { // Single validator with the param
      minLength: 5
    }
  },
  lastName: {
    type: String,
    validators: { // Multiple validators with params and custom error message
      minLength: {
        param: 2,
        message: 'Too short'
      },
      maxLength: {
        param: 10,
        message: 'Too long'
      }
    }
  },
  birthDate: {
    type: Date,
    validators: { // Validator with the function that resolves to the param
      lte: function() {
        let date = new Date();
        return date.setFullYear(date.getFullYear() - 18);
      }
    }
  }
}

Easy syntax but more code to write

fields: {
  firstName: {
    type: String,
    validators: [{ // Single validator with the param
      type: 'minLength',
      param: 5
    }]
  },
  lastName: {
    type: String,
    validators: [{ // Multiple validators with params and custom error message
      type: 'minLength',
      param: 2,
      message: 'Too short'
    }, {
      type: 'maxLength',
      param: 10,
      message: 'Too long'
    }]
  },
  birthDate: {
    type: Date,
    validators: [{ // Validator with the function that resolves to the param
      type: 'lte',
      resolveParam: function() {
        let date = new Date();
        return date.setFullYear(date.getFullYear() - 18);
      }
    }]
  }
}

I've added comment for every validators section to show what is important to notice. So let's describe it in more details.

Single validator with the param

The validators list has to be an array of objects instead of just an object. So, we have to provide validator type and param properties explicitly.

Multiple validators with params and custom error message

Here more important is custom error message. Because as we discussed above, we just provide array of validators so multiple validators is nothing special here. However, if it goes about custom error message, we only have to provide the message property. In the ideal syntax, we would have to distinguish with what situation we are dealing (more code to write for me and it would be more error prone).

Validator with the function that resolves to the param

Another very important thing when taking about validators is ability to resolve parameter during validation instead of class creation. Thanks to that we can for example validate the birthDate field to only accept users that are 18 years old and older.

SUMMARY

So the question is: Do you like the second syntax? I know it's not ideal but will cover more situation you can deal with and more over it will be less error prone. Just would like to here your opinion.

Yeah it looks like the second one has more consistent syntax and easier to understand, but more verbose too. I am okay with both syntax and I think they almost make equal sense to me, but after reviewing both proposals, I tend to like most of the last proposal I made about using field-level error event.

I think one criterion to decide which syntax to choose is what makes helps productivity most in the real world scenario. I'm taking my work flow as an example (and I assume most people work in a similar manner).

Initially, in the development phase, I want to define the model quickly, concisely and accurately. Then your original proposal for validator 2.0 will shine here:

firstName: {
  type: 'string', // or String
  validators: {
    minLength: 3,
    maxLength: 20
  }
}

This code works perfectly for my need. In this phase I won't care about what how beautiful the error message is as long as it helps me identify the error. So I won't bother writing custom errors.

Then after I have tested the model enough times, and decide to go for production, then I will start to care about UI/UX thing, and want to customize the error messages. And I want to make sure the core validation logic to remain unchanged. So having the ability to just that one extra attribute error(e) {} in the field definition works perfectly for my need.

firstName: {
  type: 'string', // or String
  validators: {
    minLength: 3,
    maxLength: 20
  },
  error(e) {
    switch (e.data.validator.name) {
      case 'minLength':
        return 'The minimum length is 3';
      case 'maxLength':
        return 'The maximum length is 20';
      default:
        return 'Some error happened';
    }
  }
}

Also, in fact what you just proposed is more of a validator-level custom error, and it should work in harmony with the field-level error event. So I guess probably we could have both in case people have different preferences?

Generating validation error is another thing. Of course it can be just string message or function that gets resolved to the string message. I could introduce two properties error and resolveError, the same as with param: param and resolveParam. Thanks to that you only have one way of accessing given functionality and there will be less mistakes and questions about "how it works?".

The thing that I've mostly wanted to focus in my previous comment was also ability to provide resolveParam function. It was very hard to deal with in Astronomy 1.0. That time, I've chosen approach that was similar to the first one from my previous comment. And I think that was mistake. It was causing errors and limited validators' functionality. So that's why I'm more for the second approach. I just would like to here if it's readable and easy to write and if all of you would like it. I think it's still much better than in Astronomy 1.0

I am happy with the second approach. It's not like i would be writing models all the time that i would really be willing to save up on coding bytes. However if we could have the first approach i would be all for it, as long as it doesn't cause other problems.

Ok validators are working. There are two more little things to work. You can test it out in RC.1. I'm closing this issue

Hi, the astronomy package has program "validator" methods, however, what does the user define with the schema? It is a declarative definition.

1. Users "require" certain conditions 2) requiring something to be null is strange.
   Possibly more declarative wording:

{
  email: {
    type: 'string',
    require: {         /* instead of validators */ 
      email: true,            /* instead of null or undefined */
      maxLength: 50
    }
  }
}

Every field is required by default so you don't require anything. If you want to make field optional you do:

fields: {
  email: {
    type: String,
    optional: true
  }
}

You do not require something to be null. However, you can create more complex validation rules where field value can be null in some situations depending on another field value.

Could you confirm that this kind of complex validation is not yet implemented into the v2 ?

anotherField: {
    type: 'string',
    validators: {
      if: {
        condition: function() {
          return this.field === 'abc';
        },
        true: { /* some validators */ },
        false: { /* some validators */ }
      }
    }
  }

There is no if validator and there won't be. You have to implement such validators yourself.

When validation depends on a field's value of the object instance being evaluated how do I get the document instance in the customer validation creation?

Validator.create({
  name: 'maxLength',
  parseParam(param) {
    if (!Match.test(param, Number)) {
      throw new TypeError(
        `Parameter for the "maxLength" validator has to be a number`
      );
    }
  },
  isValid({ value, param }) {
    if (!_.has(value, 'length')) {
      return false;
    }
    return value.length <= param;
  },
  resolveError({ name, param }) {
    return `Length of "${name}" has to be at most ${param}`;
  }
});

I found it later. I'm sorry for it.

No problem