-
Notifications
You must be signed in to change notification settings - Fork 202
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: use zod for transformation and validation #1777
base: main
Are you sure you want to change the base?
Conversation
9b0b629
to
2f6d627
Compare
Nice stuff @berendsliedrecht!! I'd really like this getting in. It should simplify quite some stuff, but will have a very big impact on existing users of the framework I think... Maybe we can aim to get it in for 0.6 A few questions:
As you say, it'd be good to add some good primitives in core (and maybe even |
Yes, that would be the goal!
Yeah that is a good question. Maybe we should look at a pattern where the constructor is not used and we have a
Ah that is something I did not think of, yet. If the current and intended behavior is to keep the unknown fields of the JSON then that should stay for sure. IIRC
Yes, I think classes are the way to go here. This way we also still have the properties of like the didcomm message url, etc. If we would just use objects it would still be possible, but I think using it will become more annoying. If we want to move away from class instances for things like this, I think we should first go through a larger part of the code base as it would feel a bit inconsistent if we only do this for the messages. Also, creating custom modules, etc. is easier if we can just extend objects and use decorators like |
Signed-off-by: Berend Sliedrecht <[email protected]>
2f6d627
to
1adc7d8
Compare
closes #1489
PoC pull request to display, and discuss, the implications of using
zod
for class transformation and validation.Right now, only did the action menu package, it is quite the easy transition and I really do prefer the API over
class-transformer
andclass-validator
. The decorators are quite nice, but in general it is quite limited and annoying to work with.I compared it to
arktype
, but mainly based on the download count (7.1m vs 22.6k) weekly I chose forzod
to make an initial PoC with. I think both have some great advantages, butzod
seems more mature and well-established.We will likely have to do the entire conversion in one PR as the core uses class-transformer for all the records which may be defined in a package.
Another thing is that
zod
can provide quite expanded errors and this is something we can rely on very heavily to inform the user.One thing that I noticed as well is that the
zod
schemas work really well when extending eachother so we can likely reuse a lot of schemas internally. Maybe we can expose some schemas under avalidation
object publically so other packages may use it.Curious to hear if there are any opinions on this, whether we should move forward and possibly if someone would like to help with some of the classes. I'd be happy to setup a base which we can reuse, but it will be quite quite quite some lines.