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

How can I set a custom header? #66

Closed
binarykitchen opened this issue Mar 8, 2015 · 8 comments
Closed

How can I set a custom header? #66

binarykitchen opened this issue Mar 8, 2015 · 8 comments

Comments

@binarykitchen
Copy link
Contributor

On the client side, in the websocket() constructor, how can I set a custom header there? I need this for an API ...

@max-mapper
Copy link
Owner

you can pass in your own WebSocket instance as the first argument, so just use the browsers builtin new WebSocket and do whatever you want https://github.com/maxogden/websocket-stream/blob/master/stream.js#L12

@binarykitchen
Copy link
Contributor Author

Yes, I could do that. But the disadvantage is that I have to add ws as a new dependency into my project. I want to avoid this.

Why not have a third parameters headers in the constructor and pass this? See here
https://github.com/theturtle32/WebSocket-Node/pull/58/files

@binarykitchen
Copy link
Contributor Author

@maxogden Also you'll risk that versions of WS do not match when you add ws as an external dependency. Not so good I think.

@binarykitchen
Copy link
Contributor Author

@maxogden @mcollina Would you accept a PR?

@mcollina
Copy link
Collaborator

mcollina commented Mar 8, 2015

It seems it might be just a options argument away, see https://github.com/websockets/ws/blob/master/doc/ws.md#new-wswebsocketaddress-protocols-options. In this line we pass no options https://github.com/maxogden/websocket-stream/blob/master/stream.js#L17.

I am ok for passing through the options, but nothing more. @mafintosh @maxogden what do you think?

@max-mapper
Copy link
Owner

yea that sounds good to me

@mafintosh
Copy link
Collaborator

👍

@binarykitchen
Copy link
Contributor Author

Ok, now that you have agreed on this, here a small PR #67

@mcollina If you dig deeper, it turns out that WebSocket.js already treats the protocols argument as options under some circumstances, see https://github.com/websockets/ws/blob/master/lib/WebSocket.js#L49

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