Skip to content

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

adrian-budau
Copy link

What is it?

  • Feature / enhancement

Description

Based on #7281

Adds a schema$ validator accepting any Standard Schema confirming validation library

Checklist

  • My code follows the developer guidelines of this project
  • I performed a self-review of my own code
  • I added a changeset with pnpm change
  • I made corresponding changes to the Qwik docs
  • I added new tests to cover the fix / functionality

@adrian-budau adrian-budau requested review from a team as code owners April 15, 2025 10:28
Copy link

changeset-bot bot commented Apr 15, 2025

🦋 Changeset detected

Latest commit: 3ab676c

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

@wmertens
Copy link
Member

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

@adrian-budau adrian-budau force-pushed the main branch 2 times, most recently from 09cd3fe to 6d47bfe Compare April 15, 2025 14:01
Copy link
Contributor

github-actions bot commented Apr 15, 2025

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name Status Preview Last Commit
qwik-docs ✅ Ready (View Log) Visit Preview 69d9253

Copy link

pkg-pr-new bot commented Apr 15, 2025

Open in StackBlitz

npm i https://pkg.pr.new/@builder.io/qwik@7518
npm i https://pkg.pr.new/@builder.io/qwik-city@7518
npm i https://pkg.pr.new/eslint-plugin-qwik@7518
npm i https://pkg.pr.new/create-qwik@7518

commit: 69d9253

Copy link
Member

@wmertens wmertens left a 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!

@wmertens
Copy link
Member

I'd like @fabian-hiller to have a quick look at this before merging.

@wmertens wmertens moved this from Backlog to Waiting For Review in Qwik Development Apr 15, 2025
Copy link
Member

@wmertens wmertens left a 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...

@adrian-budau
Copy link
Author

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?

@fabian-hiller
Copy link
Contributor

Yes, the problem is with the dot-path error type. It would be great if you could finish it and get it merged!

@fabian-hiller
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Waiting For Review
Development

Successfully merging this pull request may close these issues.

3 participants