Sockpress Router Instances

[stevenkissack]

Hi,

I got some time today to start working on an Express style Router instance for sockpress, I wanted your initial feedback as to your thoughts on merging this in to the main repo once you're happy.

Inspired by: https://github.com/expressjs/express/tree/master/lib/router

The main part I am unsure of is whether Router.js should work using a collection of Route.js instances the same as Express? I am not sure if this is the best pattern for sockets?

Things I still need to do:

• More tests (There are no tests for .use() currently)
• Remove the need for the new keyword on creating an instance
• Clean comments

Express Example:

var router = express.Router()

router.use(function(req, res, next) {
  // .. some logic here .. like any other middleware
  next();
});

router.get('/events', function(req, res, next) {
  // ..
});

// only requests to /calendar/* will be sent to our "router"
app.use('/calendar', router);

Sockpress Sockets Example:

var router = new app.io.Router()

// Optionally set route, or just call router.on() and router.event()

router.route('/')
.on('connection', function(socket) {
  // All functions on the router will return an instance of router, 
  //not a route instance as with Express Router
})
.event('ping router_namespace', function(socket, data) {
  socket.emit('pong router_namespace', data);
})
.use(function (socket, next) {
  next()
})

app.io.route('/namespace', router);

All code can be viewed pulled from my fork of your repo.

[Lesterpig]

Hi!

This seems to be a pretty good idea, and I would be really happy to merge this feature into Sockpress :)

Please just ensure that:

• Current behavior is preserved
• Current tests are still executed with success with the future route system
• New tests are added to check that everything works

The main part I am unsure of is whether Router.js should work using a collection of Route.js instances the same as Express? I am not sure if this is the best pattern for sockets?

I'm a little bit confused by the term "Route" when we are dealing with sockets. How do you differentiate two events with the same name, but not the same route? Is it linked with namespaces?

Thank you for you work!

[stevenkissack]

All original tests are still working, I just did a reshuffle of the code base to allow for the Router class. I added more namespace tests using the new Router too and that is all working, I just have to add some to check the .use() functionality.

You're correct, it is using namespaces to emulate the routes, we're using them on our company project so we can assign routes for each plugin within our modular express app and we don't need to worry about event naming conflict.

With Express it has a Router class that you can then call router.route('/someRoute'), this then returns a Route.js class instance and all methods you attach only affect that route.. With my router there isn't any concept of multiple routes within a router, only a single route and many events that will register to that route.

I guess my issues are not knowing enough about the best practices of sockets with complex namespaces & sub-namespaces but I will do some reading tomorrow as I have at least 1 more day to work on this.

Expect an update tomorrow and possibly some more commits.

If you have any suggestions I will be happy to implement them, check out the current tests to have a play with it if you like.

[stevenkissack]

Closing this now as I have opened a pull request with the functionality implemented