deepkit / deepkit-framework

A new full-featured and high-performance TypeScript framework
https://deepkit.io/
MIT License
3.15k stars 118 forks source link

(deepkit/type) Few questions/ideas about async validation #110

Closed theodorDiaconu closed 3 years ago

theodorDiaconu commented 3 years ago

I am exploring using deepkit/type for validation in my models, because it has not only validation, but all the goodies one could ask for.

I have the following use-cases:

  1. I need async validation, for example, I may want to add my own custom validator which checks the database for something. Currently all validators I've seen seem in-sync, is there support for that? Would that affect JIT in any way?

  2. Currently, my validators are "container" aware, so when performing validation, we would need to pass a sort of context object (which for example has the container), so in my validators I can then get the db driver and perform validation.

The specific use-case in mind, is to be able to do something like:

t.uniqueField({ collection, field: "x" });
theodorDiaconu commented 3 years ago

I can have a try at creating a PR for this, as it's something I might need.

  1. Would you prefer moving validate fully asynchronous, or expose a .validateAsync() method, to avoid potential BC?

  2. I'm looking at this code: https://github.com/deepkit/deepkit-framework/blob/master/packages/type/src/jit-validation.ts#L247, I'm guessing you want to keep this isomorphic, I'm not sure whether Promise() might work here, maybe you can tune in?

  3. Speed implications, would you have any concerns regarding that? Clearly the cost of promise creation might have impact, therefore we might need a way to only do async if necessary.

  4. Would we need separate api for this? t.async.x

Thanks!

marcj commented 3 years ago

Mh, I don't think that asynchronous validators with access to a DI are the right place in this library with regards to separation of concern. While validating its own integrity is something the model can easily do, the validating of a business rule where the model is only partially part of on the other side is something I don't consider the job of the model itself but an external abstraction. Usually there are transactions and other data-sources involved. Something the model should definitely not be aware of. Hence, async validators that access database shouldn't be attached to the model directly, but at the right abstraction level in your business abstraction.

Unfortunately, we don't accept PRs at this point (we can't disable them unfortunately), since we have no legal framework yet for IP related concerns.

theodorDiaconu commented 3 years ago

I understand your point of view. You treat this as effectively type validation only, not necessarily if the data is valid on a more abstract business-level level. It makes sense for this package, I will find another way

marcj commented 3 years ago

There are three types of validation usually: type, content validation, and business rule validation. First two are covered by deepkit/type, the latter should always be handled in another abstraction, namely your business abstraction.