cannot router.use(router)

[dougwilson]

The following spec makes me think this should work:

use([path], [callback...], callback) -- path can be a single path or an array of paths. callback can be a function or another Router. Chainable.

var Router = require('express-optimized')
var a = new Router()
var b = new Router()
a.use(b)

Error: You must specify string path(s) when mounting a Router
    at Router.routerUse (node_modules\express-optimized\lib\router.js:237:11)
    at repl:1:4
    at REPLServer.self.eval (repl.js:110:21)
    at Interface.<anonymous> (repl.js:239:12)
    at Interface.EventEmitter.emit (events.js:95:17)
    at Interface._onLine (readline.js:202:10)
    at Interface._line (readline.js:531:8)
    at Interface._ttyWrite (readline.js:760:14)
    at ReadStream.onkeypress (readline.js:99:10)
    at ReadStream.EventEmitter.emit (events.js:98:17)

[dougwilson]

Since [path] means the path is optional and it clearly says that callback can be a Router.

[mscdex]

Fixed in 595274130c.

[dougwilson]

Cool :+1: Here is the current express tests run against this module (with a downgraded path-to-regexp): https://travis-ci.org/visionmedia/express/builds/30703218

[dougwilson]

It seems like the main failures are from this pattern not working:

var a = new Router()
var b = new Router()
a.use('/foo', b)
b.use('/bar', function (req, res) {
  res.end('i\'m foo bar')
})

GET /foo/bar Not Found

[mscdex]

Router mounting fixed in 8e2ee40c0e. Thanks for the heads up!

[dougwilson]

Nice :) Here is the result with the new commit: https://travis-ci.org/visionmedia/express/builds/30704948 (the commit actually changed the failed test count to go from 116 to 143; I have not yet investigated).

[dougwilson]

I have not yet investigated

Looks like the issue is because req.url is not getting trimmed down as it traverses .use() calls. In the example above, where res.end is, the req.url should be "/bar"; this pattern is unfortunately engrained into the entire connect-based middleware community.

[mscdex]

Yeah, there's going to be some big incompatibilities (including things like no res.send()) that affect existing express-related modules.

[dougwilson]

As of now, I'm just treating this module as a replacement for just the Router part of express; as such express is still adding all the stuff to the prototype (like res.send). The req.url is at the Router-level, though, which is why I mentioned it here :) Without it, you cannot even serve static files without mimicking the URL on disk (since you can't just use req.url aligned to a base path any more).

[dougwilson]

Anyway, any thoughts on supporting the req.url stripping here? I'm just wondering, no one says it's a requirement :)

[mscdex]

I have other stuff that needs my attention more at the moment unfortunately, but I'm open to pretty much any PR that doesn't severely affect performance :-)

[dougwilson]

the commit actually changed the failed test count to go from 116 to 143

The cause was #6 but after filing it, I see your docs actually say the .VERB is supposed to work just like .use, so it seems like it's intentional, but it definitely makes it odd to use :)