Skip to content
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

Open
wants to merge 47 commits into
base: next
Choose a base branch
from

Conversation

fallenfreq
Copy link

@fallenfreq fallenfreq commented Apr 30, 2023

Closes #4280 /
Closes T-26

🎯 Changes

What changes are made in this PR? Is it a feature or a bug fix?

  1. 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

  2. The middleware method that you pass to an adapter will now pass the correct types through to the "req" and "res" arguments.

  3. Added the necessary tests to test the new features.

✅ Checklist

  • ✅ I have followed the steps listed in the Contributing guide.
  • ✅ If necessary, I have added documentation related to the changes made.
  • ✅ I have added or updated the tests related to the changes made.

@vercel
Copy link

vercel bot commented Apr 30, 2023

@fallenfreq is attempting to deploy a commit to the trpc Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link

vercel bot commented Apr 30, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
trpc-next-app-dir ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 2, 2024 6:44pm
www ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 2, 2024 6:44pm

Copy link
Member

@KATT KATT left a 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

KATT
KATT previously requested changes May 24, 2023
Copy link
Member

@KATT KATT left a 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

@KATT
Copy link
Member

KATT commented Feb 29, 2024

cc @fallenfreq could you get this to point to next instead? we're not really doing new features on main anymore

sorry for the delay on this!

@fallenfreq
Copy link
Author

cc @fallenfreq could you get this to point to next instead? we're not really doing new features on main anymore

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.

@fallenfreq fallenfreq marked this pull request as draft May 13, 2024 11:48
@fallenfreq fallenfreq changed the base branch from main to next May 13, 2024 11:49
@fallenfreq
Copy link
Author

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.

Copy link
Member

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

Copy link
Author

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.

Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: trpc is assuming the type of the req.query property on all adapters
5 participants