-
-
Notifications
You must be signed in to change notification settings - Fork 670
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(ws): allow to use upgradeWebSocket in handler #3942
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3942 +/- ##
==========================================
- Coverage 91.32% 91.30% -0.03%
==========================================
Files 168 168
Lines 10688 10786 +98
Branches 3070 3103 +33
==========================================
+ Hits 9761 9848 +87
- Misses 926 937 +11
Partials 1 1 ☔ View full report in Codecov by Sentry. |
Hey @nakasyou This is a good idea! I like this approach. Please check the comment I left. |
Hi @yusukebe, I fixed the code from your review. Could you check it? |
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!
Thank you! Looks good to me. This is a new feature, so I'm wondering whether it's being released in a patch version or a minor version. For now, I don't merge into the main immediately. I'll do it later. |
Closes #3005, and related to #3202 and #3845.
By the pull request, you would use upgradeWebSocket after
return
in handler like that:A thing what I will be glad is
c.req.param
will be type-safe.Also bindings and variables are supported.
The author should do the following, if applicable
bun run format:fix && bun run lint:fix
to format the code