Skip to content

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.

Copy link
Member

@gioboa gioboa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this PR still valid?

@gioboa gioboa added the WAITING FOR: user Further information is requested from the issue / pr opener label Jul 29, 2025
@fabian-hiller
Copy link
Contributor

It would be great if we could get this or my PR merged at some pint.

@gioboa gioboa removed the WAITING FOR: user Further information is requested from the issue / pr opener label Jul 30, 2025
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.

4 participants