search

fastify / fastify-websocket

basic websocket support for fastify
MIT License
400 stars 74 forks source link

Enhance code re-usability #208

Open bahamut657 opened 2 years ago

bahamut657 commented 2 years ago

Prerequisites

🚀 Feature Proposal

It seems that it could not be possible to manage the upgrade event in custom way, without re-writing the entire logic. The cause seems to be the Symbols used to store headers and socketRef into the rawReq object (kWs, kWsHead)

Motivation

Code re-usability. In my case I have to add custom headers on HTTP Upgrade response #204 (ws library under the hood, supports it).

Example

Replace Symbol usage with simple string keys or expose some properties to manage the key name attribute.

mcollina commented 2 years ago

In my case I have to add custom headers on HTTP Upgrade response https://github.com/fastify/fastify-websocket/issues/204 (ws library under the hood, supports it).

I would actually prefer to have a PR that added support for this.

airhorns commented 2 years ago

^ same, but, we could switch to Symbol.for instead of Symbol to allow folks who want to enter the "i know what i am doing and mucking around with the internals" zone to do so, instead of making it impossible?

mcollina commented 2 years ago

Using Symbol.for would make them part of the public API anyway. I don't think it's a good idea as we would have to support many more use cases than what we are currently testing.

bahamut657 commented 2 years ago

Ok. Do you think that permitting to override the Symbols in options could be the correct way? (An example here: https://github.com/bahamut657/fastify-websocket/blob/master/index.js)

Let me know and I will make a PR.

mcollina commented 2 years ago

I would actually prefer a PR that adds support for whatever use case you think it's missing rather than exposing internals.