Fluent configuration needs explicit validate() call

[MaximBalaganskiy]

As opposed to the previous validation implementation, the new one needs an explicit validate() call to trigger reporter callbacks. It would be very inconvenient to validate manually. One could argue that we should always have a proper class with decorators, but this is very limiting and violates the 'aurelia' way of unobtrusiveness

[plwalters]

Definitely what do you think is the best way for the developer to indicate that they want to watch the properties? @jdanyow any thoughts on this?

this.validator = validator.ensure//...
this.validator.watchProperties();

or by default it reports or some other way? Any thoughts @MaximBalaganskiy ?

[MaximBalaganskiy]

since decorators watch properties by default wouldn't it make sense to watch them by default in the fluent setup? Can't really think of a case when I didn't want to watch automatically... to support it though I could call this.validator.watchProperties(false) or introduce a configuration in ensure. Or may be even this.validator = validator.ensure(this.model, "Name").watch(false|true) to scope it to a particular field.

[EisenbergEffect]

I think that's a reasonable assumption. I do think we should revisit this and see if we can make it a bit cleaner.

[jdanyow]

I wonder if we should separate the observation from the rule definition. Instead of making the decorators create a getter/setter for the object, maybe it should just create validate-js constraints we push into a validationRules property on the prototype.

Likewise, instead of the fluent API taking the model as a parameter, maybe it should just setup a rules/constraints array.

Specifically, this:

import {Validator} from 'aurelia-validatejs';

export class Something {  
  static inject = [Validator];

  constructor(validator) {
    this.model = new AnotherModel();
    this.validator = validator
      .ensure(this.model, 'firstName')
        .required()
        .length({minimum: 3, maximum: 10})
      .ensure(this.model, 'lastName')
        .required();
  }
}

would change to something like this:

import {Constraints} from 'aurelia-validatejs';

export class Something {
  constructor() {
    this.validationRules = Constraints
      .ensure('firstName')
        .required()
        .length({minimum: 3, maximum: 10})
      .ensure('lastName')
        .required();
  }
}

This would separate the rules from the property observation characteristics. Then we could create some sort of observer that would need a model, some rules, and the validator implementation and would talk to the reporter?

Does any of this make sense? I was thinking this would work better especially where you have a bunch of pojos that need to be validates against a single rule-set. Maybe this is more of a server-side scenario?

[MaximBalaganskiy]

@jdanyow It makes perfect sense! This would allow creating rules and switching a model without the need to rerun validation configuration. Although, I would still need to call a method to hook rules to whatever object instance I have

[EisenbergEffect]

@jdanyow That's a really interesting idea. The trick is that we want some core infrastructure that can work regardless of what validation library is used. So, in all of this, keep in mind not just validate-js but also breeze. That's to very different apis that we'd like to tie into the same system with the validate behavior, reporters and renderers.

[EisenbergEffect]

Also, keep in mind that validation should work independent of the view as well, for server-side scenarios.

[EisenbergEffect]

But maybe it works something like this:

• Validation Library configures the model (could be just metadata)
• Validate behavior observes whatever property it's set on. When that changes it locates a validation engine.
• The Validate behavior tells the validation engine to validate the object (passing the prop that changed).
• The validation engine publishes the error info result to a reporter.
• Renderers subscribe to the reporter and display validation info.

[EisenbergEffect]

In the above case, the Validate behavior, validation reporter and error info are all core pieces. Any number of renderers could be implemented. The main thing we need is a validation engine locator that finds the validation-library-specific mechanism for validating things when a property changes. So, everyting else is shared. We just need the validation engine implemented for each validation library. It would register itself with a locator, kind of the way that the binding engine allows for binding adapters.

[EisenbergEffect]

Just thinking out loud here.

[MaximBalaganskiy]

Making binding behavior watch a binding and call validation sounds like a good solution.

[jdanyow]

how about something like this:

1. Rule Definition

   using decorators:

   export class Person {
    @required lastName;
   }

   now instances of Person have a validationRules property

   using fluent API:

   import {Constraints} from 'aurelia-validatejs';
   
   export class Person {
    constructor() {
      this.validationRules = Constraints
        .ensure('lastName')
          .required();
    }
   }

   now instances of Person have a validationRules property

   using the fluent API without a class:

   import {Constraints} from 'aurelia-validatejs';
   
   const validationRules = Constraints
    .ensure('lastName')
      .required();

2. Validating an object (no UI involved):

   class instance with validationRules property (created via decorators or fluent API):

   import {ValidationEngine} from 'aurelia-validatejs';
   import {Person} from './person';
   
   @inject(ValidationEngine)
   export class Something {
    constructor(validationEngine) {
      let person = new Person();
      let validationErrors = validationEngine.validate(person);
      // or
      let validationErrors = validationEngine.validateProperty(person, 'lastName');
   }

   validating pojo

   import {ValidationEngine, Constraints} from 'aurelia-validatejs';
   
   @inject(ValidationEngine)
   export class Something {
    constructor(validationEngine) {
      let person = { lastName '' };
      const rules = Constraints
        .ensure('lastName')
          .required();
   
      let validationErrors = validationEngine.validate(person, rules);
      // or
      let validationErrors = validationEngine.validateProperty(person, 'lastName', rules);
    }
   }

3. Composing a user interface

   class instance with validationRules property (created via decorators or fluent API):

   import {ValidationEngine} from 'aurelia-validatejs';
   import {NewInstance} from 'aurelia-dependency-injection';
   import {Person} from './person';
   
   @inject(NewInstance.of(ValidationReporter))  // or use @inject(ValidationReporter) to retrieve reporter higher up in the UI hierarchy
   export class MyViewModel {   
    constructor(reporter) {
      this.consumer = new Person();
      this.emergencyContact = new Person();
      this.primaryPhysician = new Person();
      reporter.observe(this.consumer);
      reporter.observe(this.emergencyContact);
      reporter.observe(this.primaryPhysician);
      // overload for pojos:
      reporter.observe(myPojo, myValidationRules);
   
      // optional:  keep reference to reporter's validationErrors array (eg to create a validation summary panel):
     this.reporter = reporter;
   }

   <template>
    <require from="bootstrap-validation/form-group-validation-behavior" as="validate"></require>
   
    <ul>
      <li repeat.for="e of validationErrors">${e.message}</li> 
    </ul>
    <input value.bind="consumer.lastName & validate">
    <input value.bind="emergencyContact.lastName & validate:{ helpText: true }">
    <input value.bind="primaryPhysician.lastName & validate">
   </template>

   //  bootstrap-validation plugin:  form-group-validation-behavior.js
   import {ValidationBindingBehaviorBase} from 'aurelia-validation';
   import {FormGroupRenderer} from './form-group-renderer';
   
   @inject(FormGroupRenderer)
   export class FormGroupValidationBindingBehavior extends ValidationBindingBehaviorBase {
    constructor(renderer) {
      this.renderer = renderer;
    }  
   }

   //  bootstrap-validation plugin:  form-group-validation-renderer.js
   
   export class FormGroupRenderer {
    render(validationError, element, options) {
      .....
      .....
      if (options && options.helpText) {
        ....
      }
      ....
    }
   
    unrender(validationError, element, options) {
      .....
      .....
      ....
    }
   }

   another binding-behavior/renderer for use with bootstrap tables

   //  bootstrap-validation plugin:  form-group-validation-behavior.js
   import {ValidationBindingBehaviorBase} from 'aurelia-validation';
   import {TableRenderer} from './table-renderer';
   
   @inject(TableRenderer)
   export class TableValidationBindingBehavior extends ValidationBindingBehaviorBase {
    constructor(renderer) {
      this.renderer = renderer;
    }  
   }

   //  bootstrap-validation plugin:  table-validation-renderer.js
   
   export class TableRenderer {
    render(validationError, element, options) {
      switch(options.mode) {
        case 'cell':
          // ...
          break;
        case 'row':
          // ...
          break;
        case 'column':
          // ...
          break;
      }
    }
   
    unrender(validationError, element, options) {
      .....
      .....
      ....
    }
   }

Recap

• rules are defined using decorators or fluent API
• objects/entities are given to the reporter via reporter.observe(... objects...)
• reporter observes prop changes and raises validationErrorsChanged events (breeze version much simpler because the breeze EntityManager does most of this stuff)
• NewInstance.of(ValidationReporter) will guarantee a pristine instance of a validation reporter is created using the container and registered. Effectively "scoping" a reporter in the UI hierarchy. If NewInstance.of is not used, the "ambient" or "parent" reporter is effectively injected which enables sharing a renderer instance in UI composition scenarios
• ValidationBindingBehaviorBase has logic to find the property name and object, as well as use the container to get the nearest validation reporter instance in the hierarchy (this requires the templating changes we discussed to make the container available to view instances). Armed with these items it can subscribe to the reporter's validation error events, and call the this.renderer.render(...) or this.render.unrender(...) methods
• Developers can write simple ValidationRenderer interface implementations. These can be used in a stand-alone fashion but will most likely be used with a corresponding binding-behavior to simplify associating a DOM element with an object/property/validation
• any parameters passed to the binding behavior will be forwarded to the renderer

[MaximBalaganskiy]

So reporter.observe is the thing which will watch properties change?

[jdanyow]

right- I'm proposing that reporter.observe(object) or reporter.observe(pojo, rules) would have logic to observe the object's property changes and evaluate the ruleset accordingly. We could even have options for the reporter that turn of property observation, allowing developers to call reporter.validate() if they wanted to validate manually (eg only when the user presses submit or something)

[MaximBalaganskiy]

Seems like it will work for me :)

[EisenbergEffect]

@jdanyow I like everything expect I've got one nitpick. I'd like to avoid having to create a new binding behavior for each renderer if possible. I think we should think about how we could have a single validate behavior that somehow accessed a renderer. Maybe we have a custom element (noView) for a renderer that pushed itself into the overrideContext with a special key and then the valiate behavior looks for that. I don't know. Would be cool if there was some way to separate these a bit more. Everything else looks awesome.

[plwalters]

@jdanyow You've proposed a handful of major changes to the way that things work so just going to respond one change at a time to make sure they were all intended -

1. person.validationRules - we mentioned a few times that we wanted to keep from polluting the object with properties by instead having the ability to retrieve a ValidationConfig by the object from metadata instead of relying on each object to have it's own validationRules properties. For this reason we probably want to keep the rules there.
2. Right now we have two classes - ValidationEngine and Validator. We've talked that there is some overlap there but last time was that ValidationEngine should implement a createValidator static method to return the validator so there is only one class (ValidationEngine) we are asking developers to import in to their files.
3. The validate method on the config object already takes an optional key to only return results of an object that are pertinent to the caller - https://github.com/aurelia/validatejs/blob/master/src/validation-config.js#L6 - this may actually end up being brittle because it could potentially report out issues that are only for a property to all subscribers, though I'd have to try it out to be sure. This is from moving everything to be a part of the ValidationEngine like you show above.
4. Right now the reporter is also retrievable independent of the instance currently so it's not required for the developer to tell it to observe the various objects.
5. On the binding behavior / renderer piece - do you think it would be more simple to observe the changes by default unless the developer does something like this -

this.validator = validator(this.person).dontObserve().ensure('firstName').required()

This does include a breaking change to the validator usage as described here - https://github.com/aurelia/validatejs/issues/17

[EisenbergEffect]

Taking @PWKad 's feedback and combining it with the ideas from @jdanyow along with my own twist:

1. Rule Definition

   using decorators:

   export class Person {
    @required lastName;
   }

   using fluent API (non-polluting):

   import {ValidationRules} from 'aurelia-validatejs';
   
   export class Person {
    constructor() {
      ValidationRules.for(this)
        .ensure('lastName')
          .required();
    }
   }

   using the fluent API without a class (capture or pollute):

   import {ValidationRules} from 'aurelia-validatejs';
   
   const validationRules = new ValidationRules()
    .ensure('lastName')
      .required();

2. Validating an object (no UI involved):

   class instance with hidden validation metadata

   import {ValidationRules} from 'aurelia-validatejs';
   import {Person} from './person';
   
   export class Something {
    constructor() {
      let person = new Person();
      let validationErrors = ValidationRules.validate(person);
      // or
      let validationErrors = ValidationRules.validateProperty(person, 'lastName');
   }

   validating pojo

   import {ValidationRules} from 'aurelia-validatejs';
   
   export class Something {
    constructor() {
      let person = { lastName '' };
      const rules = new ValidationRules()
        .ensure('lastName')
          .required();
   
      let validationErrors = rules.validate(person, rules);
      // or
      let validationErrors = rules.validateProperty(person, 'lastName', rules);
    }
   }

For composing the user interface, I'd like to see if we can have a single ValidateBindingBehavior that was used, but could gain access to user-specified renderers. I'm not 100% clear on the best way to do that for all scenarios yet.

[EisenbergEffect]

So, in this case, I think each validation library would provide its own reporter. The general api needs to be able to find a reporter for any given object.

[plwalters]

Right now adding validation to class, pojo, property of a class via current validation (both decorators and via fluent API) -

import {required, Validator} from 'aurelia-validatejs';

export class MyViewModel {
  @required viewModelName = '';
  description = '';
  person = { firstName: 'Billy' };
  personTwo = new Person();
  static inject = [Validator];
  constructor(validator) {
    this.validator = validator
      .ensure(this, 'description')
        .date();
    this.secondValidator = new Validator()
      .ensure(this.person, 'firstName')
        .required();
  }
}

class Person {
  @required firstName = 'Jeff';
}

Right now the main thing that is missing in the reported issue is a way for the fluent API to check for changes when a property changes instead of having to call validate manually. If you want to watch for changes in any bindingBehavior, view-model, etc... all you should need to do is ask the ValidationEngine for the correct reporter -

import {ValidationEngine} from 'aurelia-validatejs';

export class MyViewModel {
  constructor() {
    this.reporter = ValidationEngine.getReporter(this);
    this.reporter.subscribe(result => {
      // handle results
    });
  }
}

This is really intuitive at the moment because, to me, the user can continue to use Aurelia's binding system in any manner in which it chooses, the validation library simply won't know about the need to re-validate until the user actually sets the property, either manually or from HTML using binding behaviors, signals, or whatever else they want.

To get over the hump we can extract out property observation and by convention we observe the property and re-evaluate the object on change no matter if it was set up through decorators, through the fluent API, or however else. Then we would simply only need a way to configure plugin wide or on a per-validator basis to not observe for changes.

Second point on renderers

I think after going back you've nailed a nice easy way to implement custom renderers and tie them to a binding behavior @jdanyow . By extending the base renderer where the functionality is and simply using an abstraction of that form-group-binding-behavior that sets the renderer then it doesn't even force someone to use DI to set the renderer, they could create their own binding behavior that resolved the renderer from wherever they wanted. Much more extensible, right? If that sounds good I'll probably copy the relevant conversation pieces over here and implement the property observation stuff for now over in this issue

Abstracting out the validation rules

I don't see what the gain would be over current to switching to a ValidationRules object with static methods that returns results directly instead of simply listening to the reporter as it does now (which is super clean and has async support) but perhaps we can move the high-level discussion to a new issue so we can close this out to commonize the two behaviors?

[EisenbergEffect]

Ok...but maybe we can find a way to do this without inheritance ;) Maybe using a decorator like custom elements do.

[EisenbergEffect]

and a convention

[plwalters]

Sounds good, I'll wait for a thumbs up from @jdanyow as well.

Sorry another point I forgot - perhaps we can also augment the reporter by allowing the user to create or use an existing reporter and listen to additional objects to help accomplish the scenario where a developer may want a single reporter to watch for validation errors for multiple objects, but again, I think we can open a separate issue for that. May need a bit of tweaking as well because currently the reporter is polluting the object so maybe it's something like reporter.observe(this.otherObject) like mentioned above which actually adds a new reporter to __validationReporters__ (instead of __validationReporter__ currently) so you can listen for changes to multiple objects with a single reporter. Again, I can push this over to a new issue to continue discussion on that point.

[EisenbergEffect]

@PWKad I think what Jeremy is saying is that you don't need to have multiple behaviors, just to get the renderer...because the behavior itself can now get access to the view.container property and use DI to resolve the view scoped renderer. So, the end user can declare that globally for the app...or could override that for their view model and the behavior would access whatever the "closest" render was. So, I think it's even simpler.

However, if someone wanted to, they could always inherit and do something custom.

[plwalters]

I talked with @jdanyow this morning and I think the next logical step here is as follows -

So if we remove property-observer.js from being used in the decorators the decorators and fluent API are now only in charge of adding rules to the config. We can hook property-observer.js back in directly to the bindingBehavior to have the behavior continue working again but from the bindingBehavior instead of from the installation of the rules by decorator or fluent API. Once that is complete the only way for the view-model to trigger validation again is by calling validatorInstance.validate();

The binding behavior for now will be in charge of property observation and disposing of that subscription. This should fix this issue and I think set us up to get the renderer from DI and reporter from the context. I'm timid to break them out in to a completely separate library yet because it's already a pain to develop on two libraries (validation and validatejs) so perhaps just creating a mono-repo for now that holds both aurelia-validatejs and aurelia-validatejs-templating and shares tooling would be best to encourage others to continue contributing.

Any qualms with this approach?

[EisenbergEffect]

I say, go for it! 😄

[mbroadst]

Do it to it man! This looks fantastic

[EisenbergEffect]

It might be worth considering turning the validation repo into the mono repo that holds all three parts.

[ajayvikas]

This is fantastic discussion. I do have one suggestions though. Validation rules for the class need not be defined at the constructor level.

I propose Instead of this

import {ValidationRules} from 'aurelia-validatejs';

export class Person {
  constructor() {
    ValidationRules.for(this)
      .ensure('lastName')
        .required();
  }
}

Allow this

import {ValidationRules} from 'aurelia-validatejs';

export class Person {
}
function(){
    ValidationRules.for(Person)
      .ensure('lastName')
        .required();
  }
}();

The advantage of this approach is that the validation rules are being defined at the metadata level (possibly reuse the config metadata used for decorators) and are only needed to be executed once, instead of executing at every instantiation. Also it nicely decouples the validation rules from the class core logic, as well as provides ability to define the validation rules at a different place (e.g. at startup).

[jdanyow]

@ppn2 that makes sense- the decorator version of rule definition works like that, probably makes sense to make the fluent version do the same.

@PWKad thoughts?

[jdanyow]

this is implemented in the alpha: http://blog.durandal.io/2016/06/14/new-validation-alpha-is-here/