-
-
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
feat(server
): agnostic http request handler
#4285
base: next
Are you sure you want to change the base?
Conversation
@fallenfreq is attempting to deploy a commit to the trpc Team on Vercel. A member of the Team first needs to authorize it. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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 unfortunately fails all tests
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 unfortunately fails all tests
cc @fallenfreq could you get this to point to sorry for the delay on this! |
Yeah, I'll be working on a project that uses trpc again very soon so i'll do it then. |
Hey @KATT, This is ready for review again. The original disputed property name is no longer required with trpc v11. I've updated the changes and tests to work with v11 so hopefully this is good to be merged into next now. |
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.
Please just delete this, we have plenty of tests that test this path
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 do, this was originally testing the now redundant forceTrpcQueryFromUrl property as well but it's still testing that query parameters that don't match the express type don't throw and that the handler middleware has the types passed correctly. The other tests are just testing the implementation of the test itself.
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.
Hi @KATT, This was done 3 weeks ago, I know it said there were two extra failing tasks but I think that was just due to a token generation limit being reached for some of the tests. I've just updated it and those failing tests have passed.
Closes #4280 /
Closes T-26
🎯 Changes
What changes are made in this PR? Is it a feature or a bug fix?
You can now add custom servers without them having to have the same req.query type as express.
ParsedQs interface has been removed for this because its only use becomes redundant if you can't guarantee that it can be passed to URLSearchParams
The middleware method that you pass to an adapter will now pass the correct types through to the "req" and "res" arguments.
Added the necessary tests to test the new features.
✅ Checklist