feat: add valibot$ validator and fix types of zod$ implementation

[fabian-hiller]

Overview

Adds Valibot support with a valibot$ validator similarly to how Zod is supported with zod$.

Related #4998, Credits @brandonpittman

What is it?

• Feature / enhancement
• Bug
• Docs / tests / types / typos

Description

The current implementation of zod$ used type casting. I have fixed the types and solved the problems that led to the current type-casting solution. I also added support for my library Valibot with a valibot$ validator that works similarly to the zod$ validator.

Checklist:

• My code follows the developer guidelines of this project
• I have performed a self-review of my own code
• I have made corresponding changes to the documentation
• Added new tests to cover the fix / functionality

[changeset-bot]

🦋 Changeset detected

Latest commit: 47f7d60

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages

Name	Type
@builder.io/qwik-city	Minor
eslint-plugin-qwik	Minor
@builder.io/qwik	Minor
create-qwik	Minor

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

[fabian-hiller]

This is a draft PR because it is missing docs and probably a few other things. I prefer to get a review and confirmation that this will be merged before finalizing this PR.

[wmertens]

Can you run pnpm api.update so we can see what the changes are?

[wmertens]

LGTM - we could add it as an alpha preview feature in v1.8 IMHO, saying that we're looking for feedback

[fabian-hiller]

Thank you for your review!

Can you run pnpm api.update so we can see what the changes are?

Will do this tomorrow.

we could add it as an alpha preview feature in v1.8 IMHO, saying that we're looking for feedback

Not sure if this is necessary but we can do it.

[pkg-pr-new]

Open in Stackblitz

npm i https://pkg.pr.new/@builder.io/qwik@6752

npm i https://pkg.pr.new/@builder.io/qwik-city@6752

npm i https://pkg.pr.new/eslint-plugin-qwik@6752

npm i https://pkg.pr.new/create-qwik@6752

commit: 47f7d60

[fabian-hiller]

How quickly can this be merged? Should I continue with the documentation?

[wmertens]

I think we can merge this as a new feature provided we're sure that the error output will not change later.

Then we'd have to have some tests that verify the error shape.

[wmertens]

I think it straightforward enough to merge asap, it could be part of 1.8.0. Documentation would be great, please add some tests though, see https://github.com/QwikDev/qwik/blob/main/packages/qwik/src/core/render/jsx/types/jsx-types.unit.tsx for examples on how to test types, as well as real tests for the error shape.
Could you also pnpm change add to add a changeset?

[fabian-hiller]

Will do. Is it true that the Zod implementation is not tested, or am I just not finding the tests?

[wmertens]

Will do. Is it true that the Zod implementation is not tested, or am I just not finding the tests?

indeed no tests currently, but added in #6720

[fabian-hiller]

@wmertens does my last pnpm change add commit looks good to you or is something missing?

[wmertens]

@wmertens does my last pnpm change add commit looks good to you or is something missing?

Could you write it as a report of things changed instead? So not "I changed X to Y" but instead "X is now Y".

[tzdesign]

@wmertens @fabian-hiller is there something I can help with?

I need this isAny check in the types to upgrade qwik. We still hang on 1.5.6 with this neat JSX security issue.

[fabian-hiller]

Feel free to take a look at the currently unresolved error shape implementation and share your thoughts on how to proceed: #6752 (review)

I am also interested in merging this PR next week.

[octet-stream]

Looks like this PR fixes type inference for zod validator, and if so, then it will solve this issue: #5784

[tzdesign]

@fabian-hiller can you fix the key thing, I would than check it out in our large code base again.

[fabian-hiller]

I will try to go over everything in the next few days and share my thoughts on how to proceed.

[fabian-hiller]

@wmertens I have thought about this PR and think it is best if we mark the new valibot$ adapter as "alpha" and merge it as is. The main reason is that this PR improves many types and fixes some problems with the current zod$ implementation without introducing breaking changes. If we were to start rewriting everything to support the default error format of the validator being used, this PR would probably never get merged as I have less time at the moment. After merging, someone else could change the error format and figure out how to rewrite the types.

[github-actions]

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name	Status	Preview	Last Commit
qwik-docs	✅ Ready (View Log)	Visit Preview	47f7d60

[fabian-hiller]

@wmertens I am not sure what to do with the tests. This is because the type ValidatorErrorType does not match the implementation of flattenZodIssues in edge cases if I understand the code correctly. Before investing a day of work in fixing the types and writing meaningful tests, I would rather not write tests, merge this PR as is and create a new PR for Qwik City v2 that fixes and changes the implementation of zod$ and valibot$ by using the native error format of the validation library. Otherwise, we will be forced to write the tests twice.

[fabian-hiller]

My current recommendation is to review and merge this PR as is. I plan to create another PR for Qwik City v2 that will change the error formatting to the native output, further improve the types, and add unit and type tests.

[wmertens]

👍 🤠 👍

[brandonpittman]

Happy to report this is working as I had hoped it would last year. Thank you @fabian-hiller for seeing it over the finish line! 🥳