-
Notifications
You must be signed in to change notification settings - Fork 218
chore(dev-deps): upgrade to hono v4 #1092
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
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1092 +/- ##
==========================================
+ Coverage 79.48% 79.52% +0.04%
==========================================
Files 77 77
Lines 2276 2276
Branches 574 574
==========================================
+ Hits 1809 1810 +1
+ Misses 391 390 -1
Partials 76 76
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
packages/trpc-server/src/index.ts
Outdated
@@ -44,7 +44,7 @@ export const trpcServer = ({ | |||
return Reflect.get(t, p, t) | |||
}, | |||
}), | |||
}).then((res) => c.body(res.body, res)) | |||
}).then((res) => c.newResponse(res.body, res)) |
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.
The types for c.body
don't allow the combination of..
(data: Data | null, init?: ResponseOrInit): Response;
..but c.newResponse
does, both end up calling #newResponse
anyway!
https://github.com/honojs/hono/blob/0bb821a4a10053f82a6afac09a1243fd2da62ad4/src/context.ts#L707
Unsure if this needs a changeset 🤔
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.
Both c.body
and c.newResponse
are okay, but c.body
is better since c.newResponse
is not often used as a public API. So how about adding @ts-expected-error
like this (I think it's not a big problem adding @ts-expect-error
or @ts-ignore
internal implementation)?
// @ts-expect-error c.body accepts both ReadableStream and null but is not typed well
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.
Great! Done 👍🏻
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.
LGTM!
Perfect! Let's go with this. Thank you! |
* chore(dev-deps): upgrade to hono v4 * chore(zod-openapi): build workspace dependencies * chore(trpc-server): ignore null body type
Updates the monorepo to use the root
package.json
to installhono
. This ensures a single version is installed, in this casev4.x
I've been able to take what I've learned in other PRs and apply that here 🎉