aurelia / validatejs

Enables expressive validation using decorators and/or a fluent API.
MIT License
22 stars 23 forks source link

Use DI to provide Validator #47

Closed jsobell closed 8 years ago

jsobell commented 8 years ago

According to Rob the current implementation expects an instance of Validate:

constructor() {  
  this.validator = new Validator(this.model)
    .ensure('firstName')
      .required();
}

This is reported as being because the instance returned from the DI is a singleton, and a unique instance is required for each usage. Can we have the plugin initialize process add the Validator to the DI as transient, so that the standard injection feature still applies? Given that this is the only core Aurelia feature that requires 'new'ing, it's bound to be asked about repeatedly in the support areas. The added advantage is that this also means the documentation is still correct!

apawsey commented 8 years ago

Alternatively, the singleton validator needs to start collecting all the rules for all objects it comes across. I don't know if there's any significant downside to that, but if this was marked as transient, it would also surely be a deviation from the aurelia norm, as pretty much everything else is a singleton (event aggregator, router, httpclient, etc).

EisenbergEffect commented 8 years ago

I think there is an issue with the design preventing it from being used with DI actually. That fact that the Validator requires the model in its constructor causes this problem. I would personally still much rather enable the Validator to build up a set of rules and then "Apply" them to an object. This would allow DI to be used, with a transient registration. It would also allow the creation of rulesets which could be re-used across multiple objects.

michaelmalonenz commented 8 years ago

That sounds a lot like the:

constructor(validtion) {
  this.validation = validation.on(this);
}

pattern :stuck_out_tongue:

EisenbergEffect commented 8 years ago

I'm going to close this issue because there a more comprehensive discussion elsehwere now which will resolve the final api for this.