Feature: Shared route handlers within a router #35

Originally posted this over at expressjs #2828.

I think the new title better describes what I was going for. It's basically a convenience method if you ever have to share route handlers in router.method() calls. There were a few mentions of the issue, but for now I'm posting this for discussion.

I've got two example implementations that I had originally put together below. These were made off of express/master, but if enough people think this would be useful I can put together a PR from this repo.

(Note: the terminology is a bit confused. Originally I called this "private middleware", but then it settled in that old issue to "implicit middleware". Now I'm thinking it's easier to understand as "shared route handlers", but I'd love some input there)

App/Router .with(implicitMiddleware): see this branch
New method to register an array of shared route handlers (implicitly executed middleware). You setup Apps/Routers exactly the same, but there is a new method on instances: .with([fns]). This is much better than overloading .use because it won't confuse the current functionality and doesn't allow you to use implicit middleware with a mount-path (doesn't make sense because the route has the path, not the implicit middleware). Note: this now behaves better with declared order as @dougwilson mentioned.

Full example:

var express = require('express');
var app = express();
var middleApp = express();
var leafApp = express();

middleApp.with(function(req, res, next) {
    req.someProperty = true;
    next();
});

function sendSomeProp(req, res) {
    res.send(req.someProperty || 'not set');
}

app.get('/', sendSomeProp);
middleApp.get('/', sendSomeProp);
leafApp.get('/', sendSomeProp);

app.use('/middle', middleApp);
middleApp.use('/leaf', leafApp);

var server = app.listen(1234, function() {});

// RESULTS:
//   GET '/': 'not set'
//   GET '/middle': 'true'
//   GET '/leaf': 'not set'

Implicit Middleware as a create option: see here
This is something closer to what @blakeembrey was suggesting. Instead of some wrapper though, I put the implicit middleware in the options that get passed into the router:

var router = new Router({
  caseSensitive: false,
  implicitMiddleware: [fn1, fn2, ...]
});

I also changed the create method of an express app to take in an options object similar to Router.

Full example:

var express = require('express');
var app = express();
var middleApp = express({implicitMiddleware: [function(req, res, next) {
    req.someProperty = true;
    next();
}]});
var leafApp = express();

function sendSomeProp(req, res) {
    res.send(req.someProperty || 'not set');
}

app.get('/', sendSomeProp);
middleApp.get('/', sendSomeProp);
leafApp.get('/', sendSomeProp);

app.use('/middle', middleApp);
middleApp.use('/leaf', leafApp);

var server = app.listen(1234, function() {});

// RESULTS:
//   GET '/': 'not set'
//   GET '/middle': 'true'
//   GET '/leaf': 'not set'

Some Notes

Both of the implementations do not handle fallthrough with implicit middleware. Example:

...

middleApp.with(function(req, res, next) {
    req.someProperty = 1;
    next();
});

function sendSomeProp(req, res) {
    res.send(req.someProperty ? '' + req.someProperty : 'not set');
}

app.get('/', sendSomeProp);
middleApp.get('/', function(req, res, next) {
    req.someProperty = req.someProperty + 1;
    next();
});
middleApp.all('/', sendSomeProp);
leafApp.get('/', sendSomeProp);

...

// RESULTS:
//   GET '/': 'not set'
//   GET '/middle': '1', should be '2' but the implicit middleware was called twice
//   GET '/leaf': 'not set'

It wouldn't be hard to make it so the implicit middleware is only called once during the lifetime of a request, but I feel like this could be more confusing than just knowing the pitfalls of chaining with .all.