Skip to content
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

WebSocketCtor in WebSocketSubject must allow more params in new instance #2933

Closed
akanass opened this issue Oct 10, 2017 · 10 comments
Closed

Comments

@akanass
Copy link

akanass commented Oct 10, 2017

RxJS version: v5.4.3

Code to reproduce: Not a bug but an ask for enhancement

Expected behavior:

WebSocketCtor must allow all options that we can have in WebSocketClient

Missing options can be passed inside WebSocketSubjectConfig like url and protocol

Actual behavior:

Only two options are allowed in WebSocketCtor

@benlesh
Copy link
Member

benlesh commented Oct 11, 2017

Hi, @njl07, both options expected by WebSocket can be passed via the configuration object

I'm not sure what WebSocketClient is, it looks like some third party API, which we wouldn't support.

@benlesh benlesh closed this as completed Oct 11, 2017
@akanass
Copy link
Author

akanass commented Oct 11, 2017

Hy @benlesh but WebsocketClient is from nodejs-websocket that you explain to use for WebsocketCtor if we don’t want to use browser instance.

If we want to pass custom headers by example we need to use other connector because browser doesn’t have this option.

I think so it’ll be nice to have the faculty to pass more options and if it’s root.websocket don’t use their but if it’s custom connector insert their in new instance.

It can be very helpful for many people especially if we use rxjs in node environment that not have root.websocket and must have custom connector

Thanks to think about it

@benlesh
Copy link
Member

benlesh commented Oct 11, 2017

WebsocketClient is from nodejs-websocket that you explain to use for WebsocketCtor if we don’t want to use browser instance.

I see. Hmmm... nodejs-websocket was more somethign I was suggesting as a polyfill for Node that doesn't have WebSocket.

Honestly, in the near future I'd like to take stuff like this and the AJAX stuff and move it to a DOM related package instead of in core.

I'm not sure how I feel about updating this library to support a non-standardized API from a third party. So I'm going to say my answer is "no" for now, but I'll reopen the issue and allow others, like @kwonoj to comment.

Thank you for the suggestion!

@benlesh benlesh reopened this Oct 11, 2017
@akanass
Copy link
Author

akanass commented Oct 11, 2017

First, thanks to considering this issue.

Second, I think standard is not just the browser because websocket is in server side too.

If we want to communicate between microservices in websocket we can do it.

And send custom headers in websocket for authentication by example from browser to server is very interesting.

All libraries like nodejs-socket or socket.io allow their options and I think something is missing in browser instance and not something wrong in other libraries.

My friend and I have created a new nodejs framework based on rxjs and we like your job but for websocket we are blocking with their missing options.

@benlesh
Copy link
Member

benlesh commented Oct 11, 2017

Perhaps some factory function that produces web socket instance and accepts all of the options might solve this cleanly? It would be a breaking change, of course.

cc @kwonoj

@akanass
Copy link
Author

akanass commented Oct 11, 2017

Or maybe an operator to pass options on websocket connection | instance ?

@benlesh
Copy link
Member

benlesh commented Oct 11, 2017

@njl07 an operator isn't really doable, as operators will return a new Observable without altering the source. This is more something we'd need to be able to handle at the observable creation method.

@akanass
Copy link
Author

akanass commented May 14, 2018

@benlesh do you think you'll treat this one ? Thanks

@fubhy
Copy link

fubhy commented Sep 24, 2019

@benlesh The WebSocketCtor could simply be a customizable function that returns an instance of a WebSocket client instead of a constructor. That way we would be able to completely take control of what options are passed. Currently it is for instance not possible to set custom headers for the handshake, etc.

@markmcdowell
Copy link

@benlesh since v7 is coming up, could we make this breaking change?

WebSocketCtor?: { new (url: string, protocols?: string | string[]): WebSocket };

Would be replaced by:

webSocketFunc?: (url: string, protocols?: string | string[]) => WebSocket;

I'm happy to open a PR for it, I already use this in my code.

@akanass akanass closed this as completed Dec 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants