-
-
Notifications
You must be signed in to change notification settings - Fork 163
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
migrate arktype resolver to v2 #686
Conversation
@@ -233,7 +233,7 @@ | |||
"@vitejs/plugin-react": "^4.0.4", | |||
"ajv": "^8.12.0", | |||
"ajv-errors": "^3.0.0", | |||
"arktype": "1.0.19-alpha", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wow, we are using an alpha
, that's not great, and now switching to dev
. let's wait for the full release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bluebill1049 The integration isn't very deep, I can guarantee these parts of the API will be the same when a stable 2.0 is released.
In the meantime, giving users access to a resolver that is compatible with the latest version seems like a big win. Any reason not to merge this now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bluebill1049 Users have already asked to use 2.0 with react-hook-form
. There will be stabilization period before there are no tags on the release that will not involve any of the APIs used in this PR.
Is there anything else I can do to get this merged?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bluebill1049 Sorry to both you on this, just looking to understand if there is anything I can do to move this along.
🎉 This PR is included in version 3.5.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
@bluebill1049 Thanks so much 🫶 |
Thank you & welcome @ssalbdivad 🙏 |
The
latest
version ofarktype
is now2.0.0-dev.14
, which is consistent with an upcoming major release.I've created a new major version of the
@hookform/resolvers/arktype
to match this.I did have a question as to whether it was okay to resuse the existing
errors.byPath
property with atype
alias forcode
asFieldErrors
. It would have many additional properties users could introspect, so long as nowhere in hookform is there an assumption that there are no extra properties on that object (as can be seen by the snapshots with many additional properties).If extra properties are problematic, I can adjust the code to just include
message
andtype
as they did before.