Skip to content

Accept hostname lookup family option when creating WebSockets#962

Merged
lpinca merged 1 commit intowebsockets:masterfrom
hansonw:master
Jan 11, 2017
Merged

Accept hostname lookup family option when creating WebSockets#962
lpinca merged 1 commit intowebsockets:masterfrom
hansonw:master

Conversation

@hansonw
Copy link
Copy Markdown
Contributor

@hansonw hansonw commented Jan 11, 2017

http.request accepts a family option to specify the IP address family to use when resolving the server hostname. Seems pretty reasonable to accept this from the WebSocket side and forward it over to the http request call.

Specifically, this is useful for Facebook since we have IPv6 only development servers with IPv4 proxies (in which case we need websockets to connect to the IPv6 address)

Comment thread test/WebSocket.test.js Outdated

it('should accept the family option', function (done) {
const wss = new WebSocketServer({ host: '::1', port: ++port }, () => {
const ws = new WebSocket(`ws://localhost:${port}`, {family: 6});
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add spaces inside the options object? { family: 6 }.

Comment thread test/WebSocket.test.js
it('should accept the family option', function (done) {
const wss = new WebSocketServer({ host: '::1', port: ++port }, () => {
const ws = new WebSocket(`ws://localhost:${port}`, {family: 6});
});
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add a blank line after this?

@hansonw
Copy link
Copy Markdown
Contributor Author

hansonw commented Jan 11, 2017

Done! I think there's an eslint rule for the spaces-in-objects, btw :)

@lpinca
Copy link
Copy Markdown
Member

lpinca commented Jan 11, 2017

@hansonw yeah :) we are using semistandard and it doesn't enforce that.
We can extend it with our own rules but it kinda defeats the point of using a popular and premade config.

@lpinca lpinca merged commit fd910f1 into websockets:master Jan 11, 2017
@lpinca
Copy link
Copy Markdown
Member

lpinca commented Jan 11, 2017

Thanks!

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

Successfully merging this pull request may close these issues.

2 participants