-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add output response parser #1699
Conversation
This pull request is being automatically deployed with Vercel (learn more). next-prisma-starter – ./examples/next-prisma-starter🔍 Inspect: https://vercel.com/trpc/next-prisma-starter/CGiMjxC95wNXtWaHpQUFCM9PV4go www – ./www🔍 Inspect: https://vercel.com/trpc/www/8g6uQx3qyFSMrKkvbJjj95pbrDXN |
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.
Not able to look it through until tomorrow at the earliest but it is great! I would love for optional output validation
I think if there is an output validation, it should always run, we can add the optionality in prod as a feature later! I would use this myself to guarantee that I don't leak anything sensitive. |
Also setup github sponsors so I can send you 💷💷! |
Let me know if there's any way I can help! I have some experience in this area with |
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.
This all looks really sound to me. GOOD JOB!! 🙏 🙏
Some suggestions:
- Make the output parser stuff into its own test file
- Don't allow bad outputs regardless of the environment
- Some smaller comments inline
You might've accidentally removed the #1182 feature BTW ☺ |
Rolled it back - will make sure that any transforms in the output zod schemas are also handled! |
Seems like an edge case to have transformers in the output but why not! |
d242226
to
a14495b
Compare
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.
Will merge this tomorrow and do a release.
I removed the TRPC_SKIP_OUTPUT_VALIDATION
because I don't want magic runtime variables - this should either be explicitly implemented per procedure or on the app router similarily to transformers/error formatters.
This pull request has been locked because it had no new activity for 30 days. If you think, this PR is still necessary, create a new one with the same branch. Thank you. |
🚧 Work in progress
Added output validation to router queries & mutations
I have a goal to try and auto-generate OpenAPI docs for a TRPC router (cc. #755). The feature in this PR will enable us to generate JSON schemas for request and response payloads. This output parser is opt-in only and should not introduce any breaking changes.
API
output
property.output
is provided, theresolve
function will expect a return type inferred from theoutput
parser.output
parser, unlessNODE_ENV
isproduction
.TRPCError INTERNAL_SERVER_ERROR
error will be thrown.Todo