[WIP] Add app.subroute option for method chaining middleware for routes, as well as automatic OPTIONS responses and 405 Method Not Allowed error codes

[jonathanong]

app.subroute('/foo')
  .use(allMiddleware)
  .get(getMiddleware)
  .post(postMiddleware)
  .all(allMiddleware2)
  .end()

Will be equivalent to:

app.all('/foo', allMiddleware)
app.get('/foo', getMiddleware)
app.post('/foo', postMiddleware)
app.all('/foo', allMiddleware2)
app.options('/foo', function(req, res, next){
  res.setHeader('Allow', 'GET,HEAD,POST,OPTIONS')
  res.end()
})
app.all('/foo', function(req, res, next){
  var err = new Error()
  err.status = 405
  next(err)
})

.end() adds the app.options() and the last app.all() middleware and is completely optional.
There is also a app.subroute().allow() and disallow() method to add/remove methods from app.subroute().options().

This kind of implements @guille's #1501 alternative route syntax.
This also alleviates #1499 405 Method Not Allowed routing.

There are, however, issues with this code. If you're interested in merging this, I can work on it more, otherwise I'm not.

• If a response is sent via app.subroute().all, that method will not be added to OPTIONS since express doesn't know what method was used.
• I was under the impression that app.get accepted HEAD requests but doesn't send a body as per HTTP spec, but I guess that isn't the case in express (I have a test commented out). Not sure how to handle it.
• All routes have to be declared in the same app.subroute() for OPTIONS to work.

Also routes for app.subroute(routes) can be an array, though I'm not sure if you'd want that.
I added that because in my case, I have a lot of similar routes.

Then there's less important issues like I don't know where to put stuff, and coding styles.

Let me know what you think.

[adambrod]

This looks really interesting. I have found myself wanting to group sets of subroutes together and I think this would help with that. Lemme play around with it and see how it works.

[adambrod]

I really like this syntax. It feels like it definitely cleans things up and simplifies routing. One question I have: if I want to enable CORS, would I need to access subroute.methods? I'm thinking something like adding middleware after the subroute definition...

fooRoute = app.subroute('/foo')
  .use(allMiddleware)
  .get(getMiddleware)
  .post(postMiddleware)
  .all(allMiddleware2)
  .use(corsMiddleware)
  .end()
fooRoute.use( function(req, res, next) {
    res.header('Access-Control-Allow-Origin', '*');
    res.header('Access-Control-Allow-Methods', fooRoute.methods);
    res.header('Access-Control-Allow-Headers', 'Content-Type, Authorization, Content-Length, X-Requested-With');
    return next();
  };
);

Or is there a more elegant way to do this? I feel like there is probably a better way to do it. I just need the ability to get fooRoute.methods after the methods are defined.

[tj]

you can already do affectively the same thing as-is without additional api. I had a similar chainable issue open years ago but I cant find it now :( there was some discussion there as well. If anything more reflection apis would be nicer I think. We used to have a default OPTIONS behaviour but it's not really all that useful, tough call, I'll see if I can dig up that old issue

[tj]

found it: #812

this would have to be a 4x change in case people are chaining methods currently which just return the app again

[jonathanong]

I guess for CORS, it would be something like options. and you should be doing it before .end().

app.subroute() is just syntactical sugar on top of app[VERB] so it shouldn't break backwards compatibility, so people can use app[VERB] normally

and no way to hierarchical routes!

[tj]

well all of the CORS header fields are really app-specific, no point adding that stuff to express at all

[adambrod]

Yeah, I agree that Express shouldn't do the CORS stuff automatically. It just feels like CORS is the type of thing you'd want to attach to a subroute (e.g., allow CORS on /content routes but not /user routes). It's nice if you have access to route.methods so you know which verbs are allowed.

[jonathanong]

shouldn't be hard. i have an object of methods used at app.subroute().methods = {}. you can just lazy load those methods in a CORS middleware, but it should be one of the first middleware.

[shesek]

I'm playing around with an alternative implementation, with a subroute() function that calls another function with the context of a new app that's bound to a base/default path. Might be easier to just see the code: https://gist.github.com/shesek/5122225 (written in CoffeeScript, compiled source here)

[jonathanong]

• i can't read coffeescript and i dont think tj can either
• it looks hierarchical, which tj said no to
• app.configure() is deprecated
• each instance of an app creates significant overhead.
  express shouldn't be adding this overhead itself, or should at least be using a Router() instance.

[shesek]

• i can't read coffeescript and i dont think tj can either

Sorry about that, its just much quicker to hack on coffeescript :) I can write a JavaScript version if the compiled source isn't readable enough.

• it looks hierarchical, which tj said no to

I didn't know that. Why was that decided?

• app.configure() is deprecated

yeah, I know, but its a quick way to execute a function in the context of the app. I usually use an helper function for that (using express(), -> ...), but I didn't want to clutter the example with that.

• each instance of an app creates significant overhead. express shouldn't be adding this overhead itself, or should at least be using a Router() instance.

I'm just creating new objects with their [[Prototype]] set to the app, and override some methods. It shouldn't take significant overhead to do that.

It should probably be noted that I'm not saying its better than the original proposal... I just stumbled on this ticket and wanted to share the way I ended up implementing subroutes. Also, since function definitions and accessing this properties is more verbose with JavaScript, the original API is much more concise than my proposal with non-CoffeeScript code.

Edit: An interesting use-case for my variation is that it lets you call other modules with an app bound to some base path, so that all the routes added from that module are relative (or defaults) to the base path.

// main
app.subroute('/user', require('./user'))

// user.js
module.exports = function (app) {
  app.get(function(){ }); // GET /user
  app.post(function(){ }); // POST /user
  app.get('/signup', function(){ }); // GET /user/signup
}

Currently you can do something similar by mounting sub-apps, but they might not be appropriate in some cases.

[jonathanong]

#812 which was mentioned above. you didn't state your intentions (alternative, in express or as a plugin, etc.) so i just stated my thoughts.

[shesek]

@jonathanong you meant #812 w.r.t hierarchical structure? I saw it, but I don't see anything tj said against that.

[shesek]

If anyone is interested, I modified my script to support OPTIONS and 405 Method Not Allowed, and moved it to https://github.com/shesek/express-subroute.

[shesek]

I was under the impression that app.get accepted HEAD requests but doesn't send a body as per HTTP spec, but I guess that isn't the case in express (I have a test commented out). Not sure how to handle it.

I was just browsing the Sinatra's code and noticed that "Defining a GET handler also automatically defines a HEAD handler" [1]. I think is makes sense for express to do this too, doesn't it? Anyone knows how other frameworks are handling this?

[1] https://github.com/sinatra/sinatra/blob/v1.3.5/lib/sinatra/base.rb#L1245

[tj]

@shesek we already delegate HEAD to GET if there's no HEAD route defined

[rauchg]

-1

[jonathanong]

-1 for me too now. Thinking of a layer on top of express instead now.