feathersjs-ecosystem / socketio

[MOVED] The Feathers Socket.io websocket transport plugin
https://github.com/feathersjs/feathers
MIT License
37 stars 14 forks source link

Attach socket.io server to separate port #66

Closed kostia-official closed 7 years ago

kostia-official commented 7 years ago

Hello, I need to start socket.io server on another port. Not on Node.js server port. I have a microservice that hasn't public access, but I must emit some events to UI. If I open only access to socket.io port it can be possible.

Socket.io has ability to listen port or httpServer. Let's add the same option to this package.


server.listen(httpServer[, options])
Synonym of server.attach(httpServer[, options]).

server.listen(port[, options])
Synonym of server.attach(port[, options]).```
marshallswain commented 7 years ago

You can actually do this directly with socket.io. See this issue: https://github.com/feathersjs/feathers/issues/540

daffl commented 7 years ago

I think this is actually different as in you can pass a port instead of a server (see https://socket.io/docs/server-api/#server-attach-port-options).

Although this might already be possible this way:

const http = require('http');

http.createServer(app).listen(80);

app.setup(9090);
kostia-official commented 7 years ago

Thanks for your help! Can we make this more friendly? Maybe we can add this in docs? Or specify some options for socket io or server setup? Because hidden workarounds are not cool. They don't readable and you have to spend a lot of your time and time of other developers to solve an issue.

marshallswain commented 7 years ago

@kozzztya I think we have room for improvement to make this easier. This actually came up for the first time just this week (well, 8 days ago), and again today. That's why I had that other issue fresh on my mind. I've created this issue to make sure it makes it into the docs: https://github.com/feathersjs/feathers-docs/issues/461 I will leave it up to @daffl to decide if this PR is still something that we want to merge.

daffl commented 7 years ago

I didn't mean that we should not use this PR. But without documentation and tests the functionality is also rather hidden 😉

My only issue is that options used to be just the standard Socket.io options and now we added an additional property. I think the nicer API would be to allow

app.configure(socketio(port, options, callback));
kostia-official commented 7 years ago
app.configure(socketio(port, options, callback));

Looks good to me. I'll write tests and implement arguments in this way.

daffl commented 7 years ago

That would be great, thank you. Alternatively we can just document the workaround as @marshallswain tracked in https://github.com/feathersjs/feathers-docs/issues/461

daffl commented 7 years ago

Superseded by #69