Async validation - Promises et al

[jsobell]

It seems odd that given the powerful and extensive use of Promises throughout Aurelia that they are not used in the validation module. It seems to me that all validation rules should be promise based. Differentiating between the two (as Validate.js does) is something of a pain, and in grofit's treacherous library he's wrapped every rule as a Promise, most of which return Promise.resolve, but this allows very easy creation of slow-running validation rules. One issue he had was with waiting for all rules to validate, and knowing when there were outstanding Promises. Obviously we can use Promise.all, but that has a couple of issues; it can't be 'read' by other async tasks, and any error exits without waiting for the remaining Promises to resolve (or fail). I wrote a bit of a hacked but valid example of implementing the Promise based validation process in the following gist: https://gist.run/?id=044f37f5e4211717f3834bef29aba618 It doesn't perform the validation (that's left for validate.js or similar) but it does show a tidy method of counting outstanding validation rules, and enabling validation methods to wait for all outstanding validation rules to resolve, and has the advantage of introducing a tri-state to enable the UI to reflect the fact that validation is still in progress. My thought is that this can transform the current synchronous process into an async one, which allowing easy creation of custom rules, both sync and async.

[jdanyow]

Thanks for opening an issue for async validation. Several community members have raised this issue in blog post comments or as part of discussions in other issues. The subject deserves it's own issue for sure.

Before getting into API changes I would really like to get educated on the use-cases for async validation- some real examples of rules, how they'll be triggered and how they'll be surfaced in the UI would be extremely helpful.

The way I've been looking at this (which is based on my own experience and could of course be completely wrong) is validation rules typically fall into two categories:

1. Non-async:
   • cheap to execute,
   • do not require a connection to the server,
   • typically triggered on "blur" or sometimes as changes occur
   • examples: "foo is required", "start date must be prior to end date", "baz is not in the correct format"
2. Async
   • expensive to execute
   • require a connection to the server
   • typically triggered on form submission
   • typically executed only when the non-async rules are passing
   • examples: "jdanyow@bmail.com is already on file", "this delivery for 125 units cannot be saved because it causes the contract max of 50,000 units to be exceeded by 36 units".

Again, this is the way I've been looking at it, which is based on my own experience and not reflective of everyone's requirements. It would be great to gather more information on the use cases in this issue so we can come up with a design that meets as many needs as possible.

[jsobell]

In addition to these, real-world examples include:

• "email is required if property 'emailme' is true"
• "one of A,B or C must be checked"
• "one of A,B or C must be have a string value"

Re async rules;

• These may have the same priority as synch rules
• As only the dev knows which are (or could be) expensive, each rule needs to allow marking for validation only at 'submission' time or dynamically during data entry
• Some rules are non-deterministic, such as a database uniqueness check immediately prior to submission. As such, it should be re-evaluated, even if the local value has not been modified

As I mentioned, bringing in async (which is essential) also brings in other considerations;

• every check has to be Promise based, as the caller has no idea if 'slow' validations exist or not
• if the data is being validated, progress feedback may be required

Re triggering rules, most systems support dirty-checking, as dependencies in rules based on multiple fields don't generally have a single trigger method. The A,B,C examples above are representative of this situation, and even if we create a custom property in the VM to reflect that rule, we still end up with the question of how a single rule validation result is displayed on-screen. This is also why (in my experience) it's important to separate the rules from the properties. In 80-90% of cases there is a one-to-one relationship, but in almost every solution I've developed there have been complex cross-property dependencies that have to appear in a summary and possibly against a 'group' of elements, but don't relate to a single property.

[y2k4life]

Validate a Zip Code. Almost anything that requires server side validation, because that would go through HTTPClient which is async. Is email already in use, is user name already in use. These are three real examples that are needed for my in my real website going live soon.

[jdanyow]

@y2k4life not just the rule, but the full use case- rule+trigger+user experience. What triggers this validation? Submit? Property change?

What about multiple async validation rules on the same form/entity? Are people making multiple requests to the server to validate each rule separately (eg validateZipCodeAsync() + validateEmailUniqueAsync())?

Or are they submitting the entity and the if the server deems the entity to be invalid, they need a way to render the errors returned by the server in the UI (eg saveMyEntity().then(result => { if (result.errors) { ...render... } })?

Both?

[jods4]

We certainly need async validation for all the use cases mentionned here and others.

But I have to disagree with @jsobell on the fact that everything should be based on Promise. This was the old design and it was not pretty.

Async comes with drawbacks: rules are verbose to write, perf is worse and most importantly: the async part in your submit() method requires you to take care that nothing happens between your validate() call and what comes next. Most newcomers will just await validate() (if they are lucky to use ES2017) and that's it, which is not robust.

I personally think that the validatejs approach provides the best of both world: rules are trivial to write, your submit code 90% of the time is the intuitive if (validate()) send(). Only when you write an async rule do you have to bother with a Promise and only when you use it do you have to implement complicated async submit().

I would push for a validateAsync() method to the validation controller interface. Details need to be discussed, like: how's that going to work with blur, etc. People who don't want a sync version can simply use this method exclusively ;)

[y2k4life]

https://my.jewelersmutual.com/jewelry-insurance-quote-apply The link above has a similar experience of what I'm trying to achieve if you enter a zip you get a spinner, while validating, then you get a check mark if o.k. or an x if not. It is also a validation if coverage is available in a zip code.

I like both blur/property change along with submit. For a unified UX I would like blur, it would look odd that all other fields are triggered by blur but one would not. Also instant feedback is nice I fill in one field and know I don't need to go any further without filling multiple field only to find out there is a an issue. Here is my live example http://www.minicocollectiblesdirect.com/app/index.html when you leave the Zip Code I would like to validate that it is correct. Also I like that if it is not an error, but it meet other requirement to show more information. If you enter 11234 as a zip you get a dialog box stating flood zone. If you enter 99999 I would like the invalid zip code as a inline error message.

The validator is doing the validation as an async validator or a Non-async validator if not. Then _validateBinding would test how the validator handles calls. I was also thinking this could maybe possibly be handle by the rule, but the upstream of the call would need to konw, which is the validate controller. Again this goes back to multiple validators, one for mundane non-async, one for complex and yet non-async function validation, and maybe a third to handle async.

He is my idea. I know it is incomplete and will not work. Things like the return errors at the end of the method, etc..

    for(let i=0; i < this.validators.length; i++) {

      if(this.validators[i].isAsync) {
        this.validators[i].validatePropertyAsync(object, property, rules)
          .then(ne => {
              this._updateErrors(errors, ne, target);
          })
      }
    }
    this._updateErrors(errors, newErrors, target);

[jsobell]

@jods4, re disadvantages of async validation throughout, I'm interested to balance out the pros and cons before we discard that as an option. The advantages I see are:

• Aurelia is already Promise driven, so it's a familiar user usage scenario
• Performance is very good for Promises; not as fast as a simple return statement, but Promise.resolve(value > N) is almost identical to a return statement
• Having an identical usage structure between sync and async would simplify usage significantly, and avoid 'special cases'

The disadvantages are:

• Developers perceive a synchronous task as simpler and faster
• Syntax is simpler; if (validate()) {blah}, (but is hugely complicated as soon as there is a single async rule)
• Creating a custom rule is marginally simpler, e.g. return (value > N); as opposed to Promise.resolve(value > N)

I think that the core issue here is that without a Promise base for all rules, those '10%' of async rules are going to double the codebase and introduce a significantly more complex API for minimal benefit. Separating the two puts the onus on the developer to 'know' whether or not any rule against a property is async, making it impossible to dynamically apply rules to properties or objects.

your submit() method requires you to take care that nothing happens between your validate() call and what comes next

This is a critical consideration. In a sync solution this is irrelevant, because async rules are impossible and the point is moot. In an async based solution this is very real, and was the reason I started this issue. What happens when an async validation (i.e. is the username unique?) takes 2 seconds to resolve, and the user has already typed in a new value? Firstly, if the developer is only validating on submission then it's their responsibility to block editing if that would alter the model. We already have the same issue with 'save' operations. Secondly, if we are validating 'on the fly' (on blur or input) then we need to know that validation is outstanding, and that our 'save' button should be disabled until all outstanding validations are completed, even if there are multiples for the same field where only the last one is relevant.

This is a real-world, common scenario, and it's why I wrote the original Gist. Synchronous validation ignores these issues, but we can't. We can't prevent the developer screwing things up, by ignoring Promise results, forgetting the return statement, over-calling async functions on 'input' events etc, but I think we can create a superset of functionality that addresses all of these issues while adding very little complexity.

I spoke with @jdanyow last night about the use of validatejs, and I'm actually wondering what it's providing us? Sure, we get a set of predefined rules, but the message system is bizzare with it's %{value} syntax, the async handling is a bit of an afterthought. The ability to load rules from json is nice (although probably not a common usage scenario - has anyone ever used that?), so I'm simply adding this to ensure we're not adding unnecessary complexity incorporating 10% of an external dependency :)

Let's keep this discussion alive. One of my company's mantras is "if you can't justify it, it's probably wrong", so let's drill into all this stuff in detail and find ways to shoot it down. Once it's bulletproof @jdanyow can spend 3 hours coding it in because his javascript is infinitely better than mine :)

[jods4]

There are many points raised here. For a bit of background on Promises, I opened this ticket on the old validation code, which was admittedly not as good as it could have been: aurelia/validation#110

I think that the core issue here is that without a Promise base for all rules, those '10%' of async rules are going to double the codebase and introduce a significantly more complex API for minimal benefit.

I strongly believe the opposite. Making everything async makes all the code more complicated. Notably all validators, even custom validators written by users. And especially the common save() method, as you noted. If I look at validatejs source code, which supports both sync and async rules, the async version is a measly 30 lines of code on top of the rest, which is written in synchronous fashion. And it can be simplified a bit more if you ask me. So no, evidence shows that having both a sync and an async surface will not "double the codebase".

Even less so since aurelia-validation will not implement anything... The controller validate() and possibly validateAsync() will delegate to a real validation library through Validator... So if this is done well, it will go directly to the already existing validatejs code. No additional effort required.

Now the difficult part is how the bindings will work. Yes, this is going to be hard but it has to be done even without sync APIs. I suggest the following strategy: for this part (which is not a public API) call into the async API always. It doesn't matter for UI if the red border is really immediate or quasi-immediate. And unlike the form submission there is nothing to block -- but concurrency, out-of-order completion, or re-entrant validation requests by save() all need to be taken into account.

Obviously if we want to display spinners and stuff like @y2k4life suggests, the API of the renderer needs to be extended as well. Instead of having a binary OK/error status, renderer should optionally support a "Validating..." status.

[jsobell]

Hmm, well given that Promise.resolve is essentially an immediate return, I doubt there is likely to be any performance issue here, but if you think so, write a test and let's see if it's significant enough to be a concern. I'm still seeing an argument here to split validation into async and sync, which doesn't address the fact that the developer has to know in advance whether there are or might ever be any async validation rules.

I'm not pro or con any suggested solution, but I'm really not seeing any realistic argument to counter the benefits of using Promises here. Can you explain where you feel the 'complexity' of a Promise based rule definition comes in? Here's a snippet from treacherous:

    public validate(value, regexPattern: RegExp): Promise<boolean>
    {
        if (value === undefined || value === null || value.length == 0)
            { return Promise.resolve(true); }
        return Promise.resolve(value.toString().match(regexPattern) !== null);
    }

I assume we'd simply remove the word Promise.resolve to make this sync.

Re the save method, it simply is complicated. It's a fundamentally complicated problem to resolve, given the combination of concurrent access and asynchronous functionality. There's obviously a lot more to proper validation than 'run it once and come back with a boolean', and the field-level real-time validation that most developers want is complicated further when async is involved.

If you can think of a way to address this simply, either knock up a quick Gist so we can try it out, or just write some pseudo code so we can see the process.

We might be able to find a merge of the two that works nicely, but I'm not yet convinced that replacing a polymorphic validation rule system with two distinct methods is every going to be intuitive or simple. Convince me with some code :)

[jods4]

Hmm, well given that Promise.resolve is essentially an immediate return, I doubt there is likely to be any performance issue here

You don't look at the correct place. The hit is not in Promise.resolve(), it's in the callback .then().

which doesn't address the fact that the developer has to know in advance whether there are or might ever be any async validation rules

Of course. If you don't know what you're doing (well...) then you should use the async version. You can use it all the time if you want. Note that validatejs will throw an error if you call the sync api and there's an async validator. So if you test your code it's not like you won't notice your mistake.

Re the save method, it simply is complicated.

You are dodging the bullet as that's exactly the point. Having written applications with the old aurelia-validation library I can assure you (a) it is complicated and (b) I could have gotten away with the sync version 99% of the time. And it is trivial to use. I have wished several times that there was a sync option to keep things simple, as they should be.

If you can think of a way to address this simply

Yes, I can. Provide a synchronous validation API. Don't impose the burden of a robust async implementation to people who don't need it.

functions save() {
  if (!validator.validate()) return false;
  server.save(); 
  return true;
}

Compared to what you need to do for a robust async solution, this is crazy simple.

Convince me with some code :)

Look at validatejs source code, it does it already. And with very little code. As I said, the async layer is just 30 lines on top of the rest of the library...

Let me return you the question: why don't you want the benefits of a mixed solution with a public validate() api? So far I have the impression that your only argument is "aurelia code will be too complex", which I oppose with:

• aurelia is written once. Consumers code will be massively simpler, many times.
• done well, it's not even going to be that much more complex that the async-only version will be. validatejs is an example of such.

[jsobell]

I'm sensing a strong preference to not have Promises for reasons unrelated to the ones I mention, but that's OK. Performance is simply not an issue. I thought I'd better check in case I'd missed something, and the overhead is ~3ns per then on Chrome and Firefox, so on a seriously complex form with 50 validated fields we'll be wasting 0.15ms, so we can forget about that side of things. Usage, I would see as being the same as every other feature we call in aurelia;

function save() {
  validator.validate().then(isvalid => { if (isvalid) save(); });
}

My preference is to return the validation state as a parameter, while some prefer a catch(). I believe catch should only be used for errors, rather than indicating that one of n items has failed a rule. It's bedtime for me now, but we can discuss further during the week

[EisenbergEffect]

I'd like to avoid using exceptions. That caused us some issues with the last version of the library. I'm ok with the call to validate being async. I think the real usability concern comes in the writing of the rules themselves. If someone has a bunch of simple sync rules, I'd rather they not have to worry about dealing with promises at that level.

I think there's a bit of an open question as to the internal complexity that would be caused by having either all or part async. We just might need to do some more experimentation on this front before we can feel confident about a given solution. I'm sure @jdanyow has more thoughts on this.

[jods4]

~3ns per then on Chrome and Firefox

This is wrong for like 3 orders of magnitudes when using polyfills, e.g. in IE which is a supported target of Aurelia. More like 3ms. The future is bright but not everybody can choose the latest greenest browser. Anyway, perf is not the main issue here. Let's say it's OK, because it mostly is, if the code is not written poorly.

function save() {
  validator.validate().then(isvalid => { if (isvalid) save(); });
}

This is not robust. You don't protect yourself from changes between the validator.validate() call and the save() call. For production code, I would ask you to block the UI somehow while validation is in progress. That's a very non-trivial addition if you don't need it.

@EisenbergEffect I think everyone agrees that the sync version returns a bool, so it seems logical that the async Promise returns a bool as well (or a validation result) rather than rejecting.

There is internal complexity undeniably, but RE: "caused by having either all or part async" I am not sure we can avoid it. Not having async validation at all doesn't seem reasonable. (Well, I never use it but I can see why many people wants it.)

[jdanyow]

One thing that's in the works is addError/removeError APIs for the controller: https://github.com/aurelia/validation/issues/261

Assuming those are in place, what if the hypothetical view-model's save method was implemented like this:

save() {
  // synchronously:
  // - clear the errors, 
  // - validates rules, 
  // - notifies renderers and
  // - returns the error array
  const errors = this.validationController.validate(); 

  // did preliminary validation pass?
  if (errors.length !== 0) {
    return; // no! get out of here....
  }

  // todo: consider disabling UI because we're about to do some async stuff...

  // attempt to save.
  this.service.save()
    .then(result => {
      if (result.errors) {
        // server-side validation failed... tell the controller...
        this.validationController.addErrors(result.errors);
      }
    });
}

To me this seems like it would cover 99% of the async validation cases, with the added benefit of not making API calls on blur, etc. It also avoids getting into things like rule prioritization and cancellation: https://github.com/aurelia/validatejs/issues/83

One might argue how does this help with real-time async validation?

Consider the twitter "choose a username" form:

This is the simplest example I could think of. Doesn't involve multiple properties, etc. Do we even want to try and add further support for scenarios like this in aurelia-validation? Wouldn't things be more flexible if the guidance was to implement this sort of UI as a custom component that calls the controller's addError and removeError APIs?

• No matter what, validation will occur at the server and the submission of the form could potentially return an error that would need to be rendered using addError
• If we were to try and support more complex UI patterns like this in aurelia-validate we would probably need more granular options around when to validate fields. Currently it's specified at the controller level but this twitter form I used as an example validates name on blur, email on blur and the username after debouncing the input so we'd need to support a mix. I don't think we'd ever make everyone happy, no matter how many options we added.

Thoughts?

p.s. one thing the twitter UI made me realize is the ValidationController tells the renderers when to add and remove errors but it never tells renderers when add or remove "success". In today's implementation we wouldn't be able to support the blue checkmark that appears when a field is validated successfully (see gif above or image below).

[EisenbergEffect]

@jdanyow This is typically the way I have handled this in the past. It seems that minimally we should have this basic API because that enables all scenarios. Then we can discuss if we need something more "high level".

[jods4]

@jdanyow that's how I have always done it. That's why I'm pushing towards keeping a sync api and not making everything async.

For the "engagement" scenario with direct feedback (your twitter example basically), I am not sure whether it needs to be supported built-in or if a bit of custom code is in order -- because I never use it.

[jsobell]

@EisenbergEffect I agree re exceptions. The treacherous project had lots of people arguing for them, but it breaks things like Promise.all, and as I said, it indicates an error has occurred, and invalid data in an input filed is not an error. Re internal complexity; if the choice is between all or part async, there is no question that implementing both is more complex than one, but the overriding question is how will users perceive it. If we have an API where they have to 'know' that some rules are async, and change their validation calls, that's ugly. This is why I asked for code samples of how you might use a system that operated in that fashion, and if validate.js's solution is the showcase, then I'd like to see it avoided.

@jdanyow The addErrors is always a requirement. We have no idea how or why errors may be added to a validation block, and this is needed to do things like apply errors for things like combined properties such as 'only select two of A,B,C,D'. These are for fringe cases and server-side validation failures when data is submitted. However, server-side saving failures are not validation calls, they are simply something we have to do because we can't trust the client. We can perform all of our unique checks on save only, but that gives no entry-time feedback, and is very poor UX so end users don't want that. The Twitter example is a prime example encompassing the standard features you would expect to see available on any field.

As Rob says, "It seems that minimally we should have this basic API because that enables all scenarios". Absolutely, and this is provided by exposing the internal methods to add and remove errors. We do however have to make sure we can uniquely identify the property to which the error is allocated.

There is internal complexity undeniably, but RE: "caused by having either all or part async" I am not sure we can avoid it. Not having async validation at all doesn't seem reasonable. (Well, I never use it but I can see why many people wants it.)

@jods4 That last comment is what concerns me about pushing for sync rules with async as an afterthought/custom addon. In about half of our solution forms we use async validation, for example;

• when adding a new survey, the user can give it a name that must be unique within their account. They expect feedback at data entry, so it's async validation on blur.
• when configuring a survey, the URL they enter as the base address must be unique across the system, so it's validated async
• new users, username must be unique, email not already used, async rules
• adding images, buttons, templates,support files in the admin screen, name must be unique
• ...

We need more people's input into this. I've asked about validation in Gitter, and every response has been that async is a key requirements, whether we like it or not.

Async is not a second-class requirement for most people, it's first class if they want or need to provide data entry time feedback. Re some of the other points: Let's move on from the timing rubbish. If you leave IE's 3-4ms Promise in place it takes 22seconds to load Aurelia, so validation time is the least of our worries

You don't protect yourself from changes between the validator.validate() call and the save() call... For production code, I would ask you to block the UI somehow while validation is in progress. That's a very non-trivial addition if you don't need it.

That's right. This is something we have to code into every solution, but the same problem exists irrespective of whether your validations run in 4ns or 4seconds, and refusing to acknowledge that in the solution doesn't make it go away. The moment we have anything async happening we hit this problem. If the async was done on save, we can't let the user edit the data until we get the response, otherwise we will have DB save errors displayed against changed fields. When the user clicks 'save' on your form and the save is occurring, do you allow them to edit the fields on-screen?

For an actual example of a working validation solution using Promises, look at https://github.com/grofit/treacherous/tree/promise-decoupling That version has the same code as my Gist for completion tracking (which is very important and not well implemented in validate.js). I would point out that I am biased towards this solution, because after spending days trawling the web and experimenting with different ones, I found this one simply works in almost all situations. It's relatively efficient, and one of the only things missing is the really nice UI abstraction ideas @jdanyow came up with, which is why I believe a meld of the two would provide an awesome validation solution and we should adopt an already-viable solution that's already used in live systems instead of starting again from scratch

[Vheissu]

Without parroting too much of what has been said here already, I am in the camp of async validation. I think we definitely need to support it. In most situations where I have implemented validation outside of Aurelia, async has definitely made things cleaner and nicer to work with.

[MaximBalaganskiy]

I vote for making validate call async. The most obvious scenario is on the fly unique name checking - which is a must nowadays.

[jods4]

@jsobell

Async is not a second-class requirement for most people, it's first class if they want or need to provide data entry time feedback.

You made up the fact that it would be second-class. I only said I would like to keep a sync validate() next to the async one. Notice that a lot of Aurelia already works that way: all the lifecycle events can either complete synchronously or return a Promise.

In about half of our solution forms we use async validation, for example: [...]

OK, I think motivations have been given several times in this thread already. Note that you'll have to handle the validation errors that come back from your save method anyway, because someone might insert another "unique" name between the time you fill the form and the time you submit it. We acknowledged that some people may want to provide immediate feedback on top of that. And this is the part where async validators are required, yes.

Let's move on from the timing rubbish. If you leave IE's 3-4ms Promise in place it takes 22seconds to load Aurelia, so validation time is the least of our worries

Please don't disregard other people's use cases. The IT company where I work makes the vast majority of its business with clients who use IE11. And you know what? Aurelia loads in 1-2sec (in case you ask: we bundle everything; we have projects that use requireJS and other that use Systemjs as loaders). If it takes 22sec on your computer you're doing it wrong.

For an actual example of a working validation solution using Promises, look at

I have looked at it in details before even looking at the rewritten aurelia-validation. So what?

BTW I am going to step out of the discussions here. We are going to write our own thin aurelia validation adapter. Nothing against aurelia-validation, it's just that (1) the timing with my current project is not good, clearly aurelia-validation is going to change a lot; and (2) there seems to be so many opinions on so many aspects of validation (not just the async part) that we might as well do something that fits our needs. The great thing about Aurelia is that it is modular, validation is just a library and we can plug in our own. Kudos for that @EisenbergEffect. :tada:

[jdanyow]

@jods4, @jsobell, @MaximBalaganskiy, @Vheissu thanks for the feedback. Let me get the initial API changes that we have consensus on implemented and then look at evolving things further in terms of async.

I'm wondering if overhauling aurelia-validatejs needs to happen before additional async work- we've gotten the following feedback items:

• some people don't like the second dependency (not a major issue imho)
• creating custom rules is too complicated
• validatejs's syntax for creating messages is different than aurelia's syntax and does not enable reuse of converters, i18n, etc. (major issue that I previously hadn't considered).

Wondering if we should merge the decorators, fluent api, etc into aurelia-validation and add our own implementation that uses standard aurelia binding syntax for messages. While we're at it we would plan for improved custom rule definition and async workflows.

thoughts?

p.s. please don't tune out here- I totally understand the frustration on a number of levels: the timing of all this refactoring, multitude of view-points and use cases being voiced, etc. I'm confident that we can put together something that will serve everyone well as long as we continue to have the community guidance. A lot of the problem is just with me getting up to speed with everyone's requirements. Please bear with me.

[y2k4life]

If I understand correctly aurelia-validatejs is a proof of what one could do if they built their own validator plugin. Or built an interface to an existing validation framework. The question to me is what part of this is core aurelia without dependencies, which I believe to be aurelia-validation and what is a plugin which to me is aurelia-validatejs. A Validator plugin is more of an interface to logic that validates something, or a wrapper around something that already exist that does a pass fail test. There needs to be a way to allow the framework (aurelia-validation) with the features requested (custom rules, messaging, i18n, async, etc.) and a proof of concept Validtor which responds with pass or fail. To me the confusing part was that aurelia-validatejs calls itself Validator which makes it sound tightly coupled to the validation framework instead of ValidatorJS or something, along with ValidationRules instead of ValidationRulesJS. I think there is a separation of responsibility missing, what it the responsibility of the validation framework, the heavy lifting and what is the responsibility of the validator, passing or failing. Using Validator and ValidationRules interface between those two. Those need to be an interface to which a concrete plugin would implement in order to be used by aurelia-validation. The interface should require ways to validate an object asyn or none-async, setup what error are to be, add or remove rules, etc... Okay my thoughts are running out and I rambling. I think you get my point(s).

[gbegerow]

IMHO we need both sync and async variants of the standard rules. The synchronous version should be default. If you need an asynchronous version of them, import aurelia-validationRulesAsync or something like that. Than you can get a uniform handling of all rules, if you don't know which of them are used at design time.

There is no consensus when to call a validation. There was a related article the other day Inline Validation in Forms

Custom rules need to be able to control when async calls are made. E.g. in one form I will validate a lot of cross property rules on every save but only when marked as ready for approval, I will call a SAP Service to validate the whole thing. Which might take regularly between 1s and 15s. Yes, seconds, not ms. When SAP done its thing I will get either an ok or a bunch of server side errors.

In my opinion special cases like that must be possible but the 90% majority of validations should be simple.

P.S. and I18n for error messages is a must have, not optional in any way for me.

[jsobell]

@gbegerow Yes, it's important that it be kept as simple as possible, but one thing I've still not had feedback on is what exactly people find non-simple. Is it:

• the coding of custom validation rules (sync and async)
• the assigning of existing validation rules
• calling validation function (getting back a Promise vs a boolean)
• handling the rendering of field states to reflect statuses
• the frequency or triggers for validation to occur on a field

Perhaps this is a semantic thing, but when someone says "we need sync validation", what are they referring to? Is it because in their VM they'd like to be able to say if (validate('firstname') { username=this.firstname.toUpperCase(); }?

It is frustrating to get bogged down in something so minor sounding, but this is absolutely critical to resolve properly now, as it forms the basis for both the implementation and the interface projects.

As was previously suggested, we could allowing the user to return boolean or a Promise in the same way as activate() does, but what we don't see in our code is that the framework lifecycle code calling attached() has to look at the result and handle it differently depending whether it's a Promise or not, and in fact if it's not then it returns it as Promise.resolve() anyway.

var result = valthing.validate();
 if (typeof result !== 'function') {
    if (result.valid)
       submitmymodel(this);
    else
       rendererrors(result.errors);
  }
else
{
  result.then(isvalid => 
    if (result.valid)
       submitmymodel(this);
    else
       rendererrors(result.errors);
}

How about if we create a different core library for Async? If we make one single validation call to our SAP service, we have to import the aurelia-validationRulesAsync library for the solution. If we develop a project that we assume never needs a slow validation rule, then the customer requests a unique check on a field, we will have to replace the sync one and rework all of our validation checks to use Promises.

Let's just add a validateAsync() to the sync version. That would be simpler, yes? OK, so now we have two types of rule classes to define, sync and async, and when we perform an async validation we wrap the sync ones in Promises and return a Promise anyway. Hence the initial question re what is 'simpler'

I would like to point out that I'm not doggedly pushing this as a point of principal or for ego reasons, it's because I genuinely cannot see a reasonable argument for having two classes of validation rule, and I want to see this final(?) version work

[jdanyow]

Definitely reasonable. This feedback and @gbegerow's links are very helpful. Let me get caught up on implementing the stuff we have consensus on, digest some more of this info, and put together a proposal for everyone's consideration.

[jods4]

@jsobell

Let's just add a validateAsync() to the sync version. That would be simpler, yes? OK, so now we have two types of rule classes to define, sync and async, and when we perform an async validation we wrap the sync ones in Promises and return a Promise anyway. Hence the initial question re what is 'simpler'

Have a look at how validatejs handles it.

When you write a validator: you can return sync or return a promise. Whatever you want. Simple.

When you validate your model: call either validate() or validate.async(). In both cases all rules are evaluated and validatejs takes care of the details. Async always works and returns a Promise. Sync throws if you have used async rules, otherwise it works and returns the result sync.

How is that not simple? And it's not even like the validatejs implementation is complex, look at the source.

[RWOverdijk]

I think it's weird to have different methods for this. So if I ever add an async rule my code will break? What's wrong with always returning a promise? Or returning a promise when there is one, and a primitive when there isn't? If you have async validation rules, you should know that you'll get back a promise. This way the method being used never changes and it becomes more predictable.

I see two options that I think are best:

Option 1: Use 1 method: validate() which returns either a promise, or the validation results.

Option 2: Use 1 method: validate(), and always return a promise to make the api predictable. The .catch() for errors also gives back decent logic imo.

[jsobell]

@RWOverdijk I also think it's illogical, which is why I keep pushing for option 2, and Option 1 requires the code I posted which seems unnecessarily complicated. There's just something wrong with calling a function with a collection of rules, and not knowing if it will generate an error because I don't know which rules are sync or async. Is rule["checkUser"] a sync or an async rule? What if I'm dynamically attaching rules; will my app start crashing at runtime with a "no async rules allowed" error because it turns out that "checkUser" was async and missed in our UI tests due to a complex run-time logic condition? I know validate.js does this, and I think it's awful, and I would suggest we do not copy it.

[RWOverdijk]

@jsobell Let's join forces on this then. I'll make the picket, you come up with a catchy yell, like "va-li-date, not okay, easier for you when using option 2!"

It's also a consistent, familiar api which works and separates failures from success for you. My preference goes in that direction, too.

[jsobell]

Wesley, you idiot! :)

[EisenbergEffect]

What if we simply switched the convention and made validate always return a promise but added a validateSync method. In this way, async validation would be the standard way of doing things. This would also match the NodeJS approach.

[RWOverdijk]

@EisenbergEffect That's even better. Then the convention is there, but developers like @jods4, who prefer two methods, can still use an additional method. And indeed, it's the node.js way of doing things too. Nice middle ground :) Pretty sure @jsobell will agree to that.

[jsobell]

@EisenbergEffect That may be a placatory solution rather than a practical one. Differentiatiating between sync and async rules means we either need two interfaces, or one that can support both scenarios. The logical validation interface for a rule is something like validate(value, options?: any): boolean; (yes, we could return errors instead of booleans, but let's keep it simple) Now the rule for an async has to be validate(value, options?: any): Promise<boolean>; So if by default our master validation calls each rule in a collection, are we saying that it has to check each return type, and if it's sync it can convert it using Promise.resolve(retval)?

We'll also have to provide metadata to allow clients to identify which rules are sync or async.

Perhaps this is an option, as we then don't have to check return types, but can ask the rule for it's behavior and requirements. In future we may also need additional knowledge of an rules to know if they are deterministic and avoid unnecessary calls to revalidate, or state only to validate on a pre-submission call rather than on field changes.

At least with this additional flag we could throw an immediate exception if someone tries to call validateSync() and anything async, but it still seems messy to have a solution where the VM has to have knowledge of each rules internal implementation simply to avoid using .then on calls. The introduction of async/await will make this significantly less relevant too.

I've done a quick search without success, but can someone show me an example use case where a sync call is required? I don't mean a "but I like it this way" case, but a situation where using a callback/Promise/async wouldn't work? I genuinely would like to know if this exists, and if I'm somehow missing something obvious?

[RWOverdijk]

@jsobell I don't think it's about "not working". I think it's about 95% of all forms having sync validation. Defining the length, setting as required, check email format... Then I can understand that for some developers Promises feel unnecessarily complex. For checks like username availability and banking checks promises do make sense.

I like using just validate() and always returning a promise. I just think that if there are developers that wish to not use them, then something like validateSync() makes sense. It gives them what they need, follows a format (even though it's not really the same, because it doesn't make async sync, which could cause confusion) and gives us something that provides a solution for both use cases.

[jsobell]

@RWOverdijk I understand that 95% have sync validation, but most responses to my question about async validation claim that they use at least one async check on around 50% of the forms they create. We've been over this soooo many times now, but we're giving people a framework based on Promises, with services, page lifecycles, routers, interceptors, etc. all based on Promises, then we're saying "Hang on, some people might not like Promises, so lets make the rules more complex by trying to accommodate both". Yes, there are developers that wish not to use them, but most probably don't use Aurelia :) Let's get real; at the end of the day people will implement whatever is offered as long as examples are provided, the solution is simple, and it just plain works. If someone can figure out a way to allow sync calls without splitting the rules into two groups, great, let's get some ideas posted! Plus, bear in mind that we can add sync-only handling as an 'optimization' at any time.

[RWOverdijk]

@jsobell I've indicated (multiple times, just like you have) that I want to use validate() and always return a promise. I think that's best. But if this blocks progress on the plugin because 50% of the devs don't agree with me, adding a sync method is something that wouldn't affect me in any way and means we can move forward. They just have to realize that if it contains a promise, and they use that method, it'll fail.

[apawsey]

I've been avoiding this thread, just because of the sheer length of it, but I finally took the time to read the thing...

Most of my development career has been on the more strict languages, with typings and stuff. This one of the few areas where javascript really makes things messy, just because it leaves these options open. If you go and read any book on c# (or most similar languages I expect), it very quickly starts trying to teach you things like SOLID , design rules that help you formalise the components you build and their connections.

This just wouldn't be a discussion in that world, and it's one of the few unfortunate things about javascript that it allows it. We need to support async validation, therefore everything needs to be based on promises, it's that simple. The idea that a method returns just whatever it feels like and I have to test to work with it is (politely) insane. This should really apply all the way through as well, with custom validators always returning a promise, because the interface 'in' should be as structured and constant as the interface 'out'.

The only discussion then is regarding the performance of promises, as this is the part which is 'unnecessary overhead' for the x% of sync rules. And really? We're in a web browser based world where your performance is affected by the network, the browser, the server, the wifi, the guy next to you who is filling up the network with his youtube traffic, and the developer at google/opera/microsoft/mozilla who forgot a type check or something in his latest javascript engine bug fix and has actually made it 6ns slower if you return a date after July 2004! Point is the performance is absolutely negligible compared to all the other factors affecting your application.

Therefore, let's please just stick to the same approach as 95% of aurelia, and return promises - make the development simple for @jdanyow, and get a fully working, trustable, beta build out, so aurelia developers can move forward. This is causing a lot of hold up in the outside world. (Case in point, I had to launch v1 of a my first public aurelia site last night, without validation, because there are bugs still to be resolved.) 100% with @jsobell on this.

On other topics, I'm distinctly not in agreement with the addError and removeError apis. Validation is performed with validator rules, and my understanding would be you just need to create a new custom rule, rather than use add/removeError. Otherwise the entire validation library has become a complicated way for me to add red text to the ui. Developers will just start using add/remove error rather than coding a validation rule properly.

I would also argue that the use case where a username has been used in between an async validation check, and actually signing up with the server, is also not a case for add/removeError because that's a server returned error, and needs explaining to the user. The user has filled out the form, everything's ticked, and they click submit. After waiting 1/2/3/20 seconds, suddenly the check mark next to the username goes red and it says it's used...?? "But it said it was fine just now! WTF happened? I really wanted the username BigBoy!" That needs explaining, and probably resetting the username field, which is a better use case for Aurelia Notification (quick plug for @RWOverdijk :wink:).

I know javascript let's us do these crazy things, but all that flexibilty shouldn't extend to our interfaces. Validation is a rules based approach to checking the state of an entity (and by association usually a ui), before we push it on to the next stage of the process. That's the approach we should stick with, and dev's will whinge and moan that they need this and that method, but when you give them a nice clean framework, with understandable, documented, consistent interfaces, they will thank you so much in the long run.

(p.s. I shall be submitting this post to the Pulitzer people in the best factual novel category, please vote for me :smile:)

[jsobell]

Validation is performed with validator rules, and my understanding would be you just need to create a new custom rule, rather than use add/removeError. Otherwise the entire validation library has become a complicated way for me to add red text to the ui. Developers will just start using add/remove error rather than coding a validation rule properly.

They are, but only at the client end. We have two situations;

1. The user has async 'name' field validation, clicks 'save', and there is a failure. This could be on many fields, not just the 'name' field, so we have to know how we inject server-side errors keyed by property back into the error collection
2. The user does no async validation and has no client side rules. Again, save errors occur, and we have to somehow link them to the fields on-screen (or, more to the point, in the model)

The situation re unique names applies anyway, because the database or server-api may reject a call because their email fails some referential DB constraint, so even though it was valid, it's now invalid. But... we can't re-use the email validation rule, because that's not the one that's failing, and we can't sensibly pre-create every possible server validation failure, so all we can really do is allow the server to return a failed rule and let the client decide how to display it.

I think that so long as we can use a convention to allow the return of errors, we should be safe. The developer has to decide on the mapping of server-side errors to client-side keys (we can't say 'properties', because some rules may exist that don't belong to a single property), but there's no way around that.

Re the abuse of the addError function, I agree that some people probably will abuse it, but I think the built-in rules will be easier to use in most cases, and I just don't see an alternative way of mapping received errors back into the validation error list. I reckon that as long as we accept errors in the same format that rules return them, we should be safe

(p.s. You've got my vote)

[masterpoi]

+1 for @apawsey FWIW with the old API i channeled my server side validation errors into custom ValidationRules on the form, calling revalidate and have the errors show up in the UI. As long as you can do the same with the new API, create a custom rule that checks if you got a server error back in the last response of form submission, i don't see any reason for an addError function to exist.

[jsobell]

@masterpoi How did you do that? Are you saying you have a generic rule for all server-side errors, and the results populated the displayed data, or did you add a custom rule for every field that might ever receive a server-side error? I'm not quite clear as to how you link the two, and how calling revalidate fits in. Can you provide an example?

[masterpoi]

@jsobell Yes i've added a custom rule for every property. (Which i happen to know in advance, because my whole form is being generated by Swagger metadata, but i guess you just could enumerate the properties of any object in javascript)

It is something like

export class ValidatorDecorator {
    constructor(public validation: ValidationController, public i18n : I18N) { }

    static everythingIsValidServerValidationResult = {};

    serverValidationResult: any = ValidatorDecorator.everythingIsValidServerValidationResult;

    resetServerValidationResult() {
        this.revalidate(ValidatorDecorator.everythingIsValidServerValidationResult);
    }
    revalidate(serverSideResult: any) {
        this.serverValidationResult = serverSideResult;
        return this.validation.validate();
    }

    public getValidationMessage(key: string): string {
        key = key.replace(/\s+/g, "_");
        return this.i18n.tr(key);
    }
}

export class CustomValidationRule extends ValidationRule {
    constructor(ruleGroupName: string, context: ValidatorDecorator) {
        super(
            {
                ruleGroupName: ruleGroupName,
                context: context
            },
            (newValue, threshold) => {
                return !threshold.context.serverValidationResult[threshold.ruleGroupName];
            },
            (newValue, threshold) => {
                return context.getValidationMessage( <string>threshold.context.serverValidationResult[threshold.ruleGroupName][0].errorMessage);
            }
            );
    }
}

Then further on, pseudocode:

const validator = this.validation.on(thing);
var context = new ValidatorDecorator(validator, this.i18n);
for(let prop in thingsproperties) {
  validator.ensure(prop).passesRule(new CustomValidationRule(prop, context))
}

When the server call comes back with an errror:

 context.revalidate(serverErrors);

[apawsey]

I've been thinking about @jsobell's argument regarding...

and we can't sensibly pre-create every possible server validation failure, so all we can really do is allow the server to return a failed rule and let the client decide how to display it.

and then...

The developer has to decide on the mapping of server-side errors to client-side keys

How is the developer going to decide on the mapping of the server-side error if he doesn't know what the error to be returned is...? I would argue strongly that unknown errors returned by the server are not "validation" errors, and as such should not be handled through the same framework.

As for the 'known' validation errors, in the use case where it is impossible to validate the form without posting it to the server, I'm not claiming to have worked out the whole plan in my head, but it seems something similar to @masterpoi's approach above makes more sense. If you are expecting possible validation errors in your result, then passing that result into something (a server validation rule type thingy), which can then behave like yet another validation rule, would be heading in the right approach. In fact a starter/generic server validation rule can be added where it will take standardised json with a fieldname(s) and error and pipe that into the validation 'results'.

The next argument will be "but why do all that, when addError is much easier?"... because addError conveys no purpose or structure, and it's not that it might be abused, it will be abused! In 6 months, it will underpin the majority of validation code in live aurelia apps! And everyone will have a mess of validation code, some async, some sync, some before the form is posted, some after, and all of it mashed together in a method from hell.

[jsobell]

@masterpoi OK, that looks neat. The main issues I see from a generic viewpoint are:

1. You must create validation rules for every property, which is significant task in itself given that we either have to decorate every single property that the server might have an issue with
2. If we added all of these, how do we cope with entity related errors when we don't have a specific property to allocate them to. Business rules (which are intrinsically server-side) will often report an issue related to several fields
3. Unless I'm missing something, this requires that the server return the set of errors in the correct format that the SPA expects, so context.revalidate(serverErrors); is injecting the errors directly into the error result

I can imagine this type of approach being adopted in most SPAs, except that is most cases people will want to map the errors to their SPA validation results, as it makes little sense to enforce your SPA validation structure on the server API. We often have little or no control over the server generated responses (pre-existing APIs, or separate teams), so we need to allow users to map server-side issues to the validation rule they apply to (which, as I already mentioned, may or may not be a property, or generic to the operation as a whole, or to a subset of the model such as the address fields only)

[jsobell]

How is the developer going to decide on the mapping of the server-side error if he doesn't know what the error to be returned is...? I would argue strongly that unknown errors returned by the server are not "validation" errors, and as such should not be handled through the same framework.

Good point. When an error comes back, we may not be able to ascertain what that error applies to, and we may not know what the cause was. If the designer of the server-api simply returns [{"field":"name", "err": "invalid"}], then the only thing we can derive is that field called name returned a string "invalid". This is not an 'unknown error', and it's up to the SPA developer to decide whether to simply display ${err}, map it to a lookup for a more meaningful string, or perhaps map it against a list of predefined multi-lingual error messages because what we assumed was an unhelpful string "invalid" was actually a unique code meaning "contains invalid character". I believe the crux of the matter is simply that we just don't know how validation will be handled, and I can guarantee you that the first ten people to use a validation solution will all have different usage scenarios, different server-side API features and limitations, and different ways to map the resulting failures back to the UI. We're not providing and E2E validation framework here, we're providing a client-side only one. All because someone can write an appalling server-API doesn't mean we can refuse to let the consuming client use validation. We can provide 'best-practices' options using solutions similar to @masterpoi's, but every 'helper' wrapper created will need to access these low-level functions anyway so we can't exclude them.

[masterpoi]

@jsobell I completely agree. The point i wanted to make is that i don't think an addError method is a good "low-level" function since you have to make assumptions about it that do not hold for a lot of people, since if the api doesn't do what you want it to do you will HAVE to abuse it.

You could have addError(fieldName:string, errorMessage:string) ==> what if my server result returns an errorMessage for multiple fields? addError(fieldNames:string[], errorMessage:string) ==> what if my server result does not include field names? I only want to show it in the validation summary. addError(errorMessage:string) ==> what if i have a custom result that needs custom processing? ...

As i see it, you should be able to completely customize a) what the server validation result model looks like, b) how that is translated to the client side validation model and c) how this model is rendered in the ui. I think having custom validation rules that can have custom renderers can get this job done.

1. You must create validation rules for every property, which is significant task in itself given that we either have to decorate every single property that the server might have an issue with

You could decorate the object. So there would be validation rules that are unbound to any property, but bound to the object you are validating. i.e.

ValidationRules.ensureInvariant((obj : any) => Promise<boolean>).on(obj);

1. If we added all of these, how do we cope with entity related errors when we don't have a specific property to allocate them to. Business rules (which are intrinsically server-side) will often report an issue related to several fields

a) show them in the ValidationSummary only b) show them next to every field related c) custom

1. Unless I'm missing something, this requires that the server return the set of errors in the correct format that the SPA expects, so context.revalidate(serverErrors); is injecting the errors directly into the error result

In my case i just transform the server side valiation result model to the aurelia client side one. I did not model my server validation results to Aurelia's. You could, if the aurelia client is the only "user" of the server side API, but in my case it is not.

[jsobell]

yep, this all makes sense. The only variation on this to consider is that 'fieldName' is assuming that we're always identify a failed state with a model property (or the validation attempt as a whole), but that may not be the case. It may be that we allocate each rule-link (which links together a model, defined rule, and options/message metadata) with a simple named identifier, which by default would be the fully qualified model property (e.g. "address.postcode" or even "user[3].address.postcode"), but in defining none-single-property related rules or generic rules we can assign a user-defined name or 'id' for each rule-link. We also have to consider the common use case of having to display a particular validation state in a particular location of the DOM. E.g. someone might want a rule on a group of properties displayed below that group, rather than in the summary. The 'id' approach solves this nicely.

[jdanyow]

implemented!