aurelia / validatejs

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

Duplicate ValidationEngine #13

Closed EisenbergEffect closed 8 years ago

EisenbergEffect commented 8 years ago

The ValidationEngine class is duplicated in both validatejs and validation. We should probably put that in the core validation library and remove it from validatejs.

plwalters commented 8 years ago

Since each of these classes is an interface that the validation library should 'override' wouldn't it be best to leave it in both places and simply have the validatejs implement the methods as it needs to?

That is how everything is set up currently but I can fix whatever needs to be done.

I think if anything we can remove the dependency on Validation from the Validatejs repo since it really doesn't depend on it. Thoughts?

EisenbergEffect commented 8 years ago

I think we have some things in the wrong place.

It seems that ValidationEngine, ValidationRenderer, ValidationNotifier and ValidationError should all be independent of validate.js. These are the pieces I would put inside the validation library.

Then I would have an implementation of ValidationRenderer inside the templating-validation library. Just one simple implementation so it works. Other renderers I would put in their own library so people can choose based on the css framework they are using. We can let the community create these.

I would also have the validate binding behavior inside the templating-validation library. It gets the notifier (using the engine), registers, maps bindings to elements and passes that information back to the renderer.

ValidationEngine should provide the standard way to get the notifier, regardless of framework. There should be a way to plug into that so that a validation library can provide a location strategy, just like in the binding system. That way breeze can find or create a renderer as needed, but there should be a default strategy using the metadata system probably. Consumers of notifiers shouldn't have to know about the different ways the notifier is found or created, but should just use the engine api to get one.

ValidationNotifier can just be an interface defined in the validation repo. The concrete implementation can be provided in validatejs and by any other validation system. The main thing is that we need a cross-library way to locate the notifier, which is what the ValidationEngine should give us. However, there could be a simple implementation of this in validation as well, that just enables the basic pub/sub interface. I think we'll want to add other things to this as well such as "isDirty", "isClean", "reset()", etc. Since a validation library just needs to publish to the notifier, this might really end up being a single implementation shared across libraries in the end. The Breeze adapter would just "get" this same notifier type but would wire up its events to publish to it and return the created/stored object.

ValidationError should store all information so that the renders have consistent data to work with and the validate behavior can reliably locate elements. That should be a concrete class in the validation repo.