-
Notifications
You must be signed in to change notification settings - Fork 738
fix: handle websocket connections using experimental deno:request event #3446
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
base: main
Are you sure you want to change the base?
fix: handle websocket connections using experimental deno:request event #3446
Conversation
| if ( | ||
| headers.get("upgrade") !== "websocket" || | ||
| // Vite uses websockets with protocol set to vite-hmr, vite-ping | ||
| headers.get("sec-websocket-protocol")?.startsWith("vite-") | ||
| ) { |
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.
I wish every non-vite request was going through this route, as the current implementation looks too fragile
It is however only required and used for websocket for now.
|
@marvinhagemeister
What would be your approach if intercepting the request is not the correct way. |
|
@DontMash Create a Fresh cli that calls vite as a middleware. That's the proper solution. This PR can't be merged because it depends on another PR being merged into Deno itself. The PR for Deno is very unlikely to be merged. |
Should As far as I know, the vite middleware can only be accessed when creating a server or configuring a plugin. |
You don't need to do that with requests being Deno requests, which is default in Fresh The problem is that vite starts a node.js http server instead, and you can't properly convert node requests back to deno requests, because node.js handles websockets differently It is possible however to create a vite dev middleware for fresh (I.e |
I know, but you still need to handle the upgrade yourself. The question was also closer related to meaning of cli in this context
I also tried to convert Nodejs websocket requests to Deno myself.
This would mean that just the dev_server would not be part of the plugin anymore, but rather be implemented as a middleware for fresh? |

Fixes: #3350
Requires deno with this PR: denoland/deno#30746
Context: