-
-
Notifications
You must be signed in to change notification settings - Fork 128
docs: add guide for disabling output validation #1163
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds documentation and a sidebar entry for "Disabling Output Validation" and a test that verifies output validation can be disabled by setting Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Procedure
participant Validator as Output Validator
rect rgb(200,230,255)
Note over Procedure,Validator: Validation enabled (normal flow)
Client->>Procedure: invoke()
Procedure->>Validator: validate(output)
Validator-->>Procedure: pass/fail
Procedure-->>Client: return result
end
rect rgb(240,230,200)
Note over Procedure: Validation disabled (initialOutputValidationIndex = NaN)
Client->>Procedure: invoke()
Procedure->>Procedure: skip validation
Procedure-->>Client: return raw result
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
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 |
Summary of ChangesHello @unnoq, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the documentation by providing a new guide on how to disable output validation in oRPC procedures. This is particularly useful for scenarios where output schemas are primarily used for OpenAPI specification generation, allowing for performance improvements by skipping runtime validation. The changes include adding the new documentation page, updating the navigation, and introducing a corresponding test case to confirm the feature's behavior. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
Code Review
This pull request adds documentation for disabling output validation. The documentation is clear and correctly highlights the limitations. The added test confirms the feature works. I've suggested an improvement to the test file to also cover the default behavior (when validation is enabled), which will make the test suite more robust and self-contained. Overall, a good addition to the project.
| import z from 'zod' | ||
| import { os } from '../src' | ||
|
|
||
| it('support disable output validation by setting initialOutputValidationIndex to NaN', async () => { | ||
| // docs: apps/content/docs/openapi/advanced/disabling-output-validation.md | ||
|
|
||
| const procedure = os.$config({ | ||
| initialOutputValidationIndex: Number.NaN, | ||
| }) | ||
| .use(({ next }) => next()) | ||
| .output(z.number()) | ||
| .use(({ next }) => next()) | ||
| // @ts-expect-error invalid output type | ||
| .handler(() => { | ||
| return 'invalid' | ||
| }) | ||
| .callable() | ||
|
|
||
| await expect(procedure()).resolves.toBe('invalid') | ||
| }) |
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.
While the added test correctly verifies that output validation can be disabled, it would be beneficial to also include a test case that confirms the default behavior (i.e., that validation is enabled and throws an error for invalid output). This would make the test suite for this feature more complete and robust. I'd suggest wrapping both tests in a describe block for better organization.
import z from 'zod'
import { os } from '../src'
describe('output validation', () => {
it('is enabled by default and throws on invalid output', async () => {
const procedure = os
.output(z.number())
// @ts-expect-error invalid output type
.handler(() => {
return 'invalid'
})
.callable()
await expect(procedure()).rejects.toThrow(z.ZodError)
})
it('can be disabled by setting initialOutputValidationIndex to NaN', async () => {
// docs: apps/content/docs/openapi/advanced/disabling-output-validation.md
const procedure = os.$config({
initialOutputValidationIndex: Number.NaN,
})
.use(({ next }) => next())
.output(z.number())
.use(({ next }) => next())
// @ts-expect-error invalid output type
.handler(() => {
return 'invalid'
})
.callable()
await expect(procedure()).resolves.toBe('invalid')
})
})
More templates
@orpc/ai-sdk
@orpc/arktype
@orpc/client
@orpc/contract
@orpc/experimental-durable-iterator
@orpc/hey-api
@orpc/interop
@orpc/json-schema
@orpc/nest
@orpc/openapi
@orpc/openapi-client
@orpc/otel
@orpc/experimental-publisher
@orpc/react
@orpc/react-query
@orpc/experimental-react-swr
@orpc/server
@orpc/shared
@orpc/solid-query
@orpc/standard-server
@orpc/standard-server-aws-lambda
@orpc/standard-server-fastify
@orpc/standard-server-fetch
@orpc/standard-server-node
@orpc/standard-server-peer
@orpc/svelte-query
@orpc/tanstack-query
@orpc/trpc
@orpc/valibot
@orpc/vue-colada
@orpc/vue-query
@orpc/zod
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/content/docs/openapi/advanced/disabling-output-validation.md (1)
25-48: Consider expanding the limitation section.The transform example is excellent, but the limitation section could be more comprehensive. When validation is disabled, other schema features are also skipped, including:
- Refinements (
.refine(),.superRefine())- Default values (
.default())- Coercions and preprocessing
- Any other schema-level data modifications
Consider adding a note that all validation and transformation logic is bypassed, not just explicit
.transform()calls.Apply this diff to add a more comprehensive note:
## Limitation -This approach will not work correctly if your schema transforms the data into a different type during validation. +This approach will not work correctly if your schema performs any transformations or modifications during validation, including: + +- Transforms (`.transform()`) +- Refinements (`.refine()`, `.superRefine()`) +- Default values (`.default()`) +- Coercions and preprocessing ```ts twoslashpackages/server/tests/advanced.test.ts (1)
4-20: LGTM! Test correctly verifies the feature.The test properly validates that setting
initialOutputValidationIndextoNaNdisables output validation, allowing the string'invalid'to pass through despite thez.number()output schema.Consider adding a test case for the transform limitation mentioned in the documentation:
it('demonstrates transform limitation when output validation is disabled', async () => { const procedure = os.$config({ initialOutputValidationIndex: Number.NaN, }) .output(z.object({ value: z.number().transform(val => String(val)) })) .handler(() => { return { value: 123 } }) .callable() const result = await procedure() // With validation disabled, transform is skipped // Client expects string but receives number expect(result.value).toBe(123) // number, not "123" string expect(typeof result.value).toBe('number') })This would help verify the documented limitation and ensure the behavior is well-understood.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/content/.vitepress/config.ts(1 hunks)apps/content/docs/openapi/advanced/disabling-output-validation.md(1 hunks)packages/server/tests/advanced.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/server/tests/advanced.test.ts (1)
packages/server/src/builder.ts (1)
os(336-352)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: publish-commit
- GitHub Check: test
- GitHub Check: lint
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
apps/content/.vitepress/config.ts (1)
287-287: LGTM! Sidebar entry correctly configured.The navigation link is properly placed in the Advanced section and matches the new documentation file path.
apps/content/docs/openapi/advanced/disabling-output-validation.md (2)
1-8: LGTM! Clear and concise introduction.The frontmatter and introduction effectively explain the feature's purpose and when to use it.
10-23: LGTM! Configuration example is clear.The example demonstrates the correct usage of
initialOutputValidationIndex: Number.NaNand explains that all procedures built from the base will inherit this behavior.
Closes: #602
Summary by CodeRabbit
Documentation
Tests