-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat: Add Standard Schema validator to qwik-city #7518
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 3ab676c The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Interesting! If it doesn't increase build size when not in use, that sounds great. Can you run npx syncpack-fix-mismatches? And you also have to run pnpm api.update |
09cd3fe
to
6d47bfe
Compare
built with Refined Cloudflare Pages Action⚡ Cloudflare Pages Deployment
|
commit: |
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.
Looking at the build sizes, qwik-city grows with 100 bytes or so and the qwik docs with a few thousand but it seems negligeable. LGTM!
I'd like @fabian-hiller to have a quick look at this before merging. |
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.
@adrian-budau actually, I forgot that Fabian already worked on this in #7281, with specific attention to the types of error returns, which are not handled correctly here.
Do you think you could continue that PR? Fabian has no time right now...
Well I started the work based on that PR so sure, but I'd like a little more context on what exactly needs to be changed about the error returns? |
Yes, the problem is with the dot-path error type. It would be great if you could finish it and get it merged! |
If I remember correctly, my PR missed testing and updating the docs. Also, there was an idea to include the types directly into the repo to avoid adding a dependency. Feel free to copy the code from my PR. |
What is it?
Description
Based on #7281
Adds a
schema$
validator accepting any Standard Schema confirming validation libraryChecklist
pnpm change