feat(validate): support async validation within defineValidatedHandler#1491
Conversation
`defineValidatedHandler`'s query and headers
📝 WalkthroughWalkthroughAsynchronous schema validation is supported for request headers and URL queries. Validated handlers now await request and URL validation, and handler tests cover successful validation plus invalid body, header, and query cases. ChangesAsync validation support
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/utils/internal/validate.ts (1)
91-97: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFix variable naming for consistency.
Consider renaming
validatedheaderstovalidatedHeadersto follow camelCase conventions.♻️ Proposed refactor
- const validatedheaders = await validateSource( + const validatedHeaders = await validateSource( "headers", Object.fromEntries(req.headers.entries()), validate.headers as StandardSchemaV1<Record<string, string>>, validate.onError, ); - for (const [key, value] of Object.entries(validatedheaders)) { + for (const [key, value] of Object.entries(validatedHeaders)) { req.headers.set(key, value); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/internal/validate.ts` around lines 91 - 97, Rename the local variable validatedheaders to validatedHeaders in the validation flow, including its declaration and the Object.entries iteration, while preserving the existing behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/utils/internal/validate.ts`:
- Around line 91-97: Rename the local variable validatedheaders to
validatedHeaders in the validation flow, including its declaration and the
Object.entries iteration, while preserving the existing behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c3e97fa2-2738-487c-bc2f-4f258e81b917
📒 Files selected for processing (3)
src/handler.tssrc/utils/internal/validate.tstest/handler.test.ts
|
Only can you double check it is all compliant with standard-schema? |
yes, I've added some new zod tests that do have a async refines, which forces EDIT: the upstream standard-schema example for sync-only usage is for libs that might not support async operations, like some sort of hooks, but we have no such limitation |
| onError?: OnValidateError, | ||
| ): T { | ||
| const result = fn["~standard"].validate(data); | ||
| if (result instanceof Promise) { |
There was a problem hiding this comment.
We could do later but Promise chaining should be prefered over forced async keyword (it forces code to resume in next tick even if not needed to!) part of why H3Core is fast is that we avoid async/await
pi0
changed the title
defineValidatedHandlerdefineValidatedHandler
2 participants
Discussed in #1437
This is the first PR of hopefully a few related to improving validation strictness and related type support within h3's toolset.
In this change I solely focus on adding async support for query and header validaiton within
defineValidatedHandlerthat were previously sync-only.Summary by CodeRabbit
New Features
Bug Fixes