-
Notifications
You must be signed in to change notification settings - Fork 20
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
fix: do not automatically return Sec-WebSocket-Protocol
header
#142
base: main
Are you sure you want to change the base?
fix: do not automatically return Sec-WebSocket-Protocol
header
#142
Conversation
Sec-WebSocket-Protocol
headersSec-WebSocket-Protocol
header
"packageManager": "[email protected]" | ||
"packageManager": "[email protected]", | ||
"pnpm": { | ||
"onlyBuiltDependencies": [ |
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 allows the installed deps to execute their build scripts after installing. Seems like it's necessary to add this list to avoid having to approve the builds every time we install packages.
https://pnpm.io/package_json#pnpmonlybuiltdependencies
@@ -1,5 +1,4 @@ | |||
import type { AdapterOptions, AdapterInstance, Adapter } from "../adapter.ts"; | |||
import type { WebSocket } from "../../types/web.ts"; |
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 import wasn't being used so I removed it. The one that's being used is the import alias WebSocket as WebSocketT,
fixes #141
This PR includes a breaking change that modifies the behaviour of the Node and Deno adapters to no longer automatically accept the first WebSocket protocol the client sends. This makes the behaviour the same across all adapters and prevents errors for Node and Deno when trying to return the
Sec-WebSocket-Protocol
header from theupgrade
hook.