lukeed / trouter

:fish: A fast, small-but-mighty, familiar fish...errr, router*
MIT License
634 stars 23 forks source link

Ability to add sub-routers? #2

Closed iCyberon closed 6 years ago

iCyberon commented 6 years ago

It would really be great to have ability to add sub-routers. Don't know if it's on your roadmap, if it is I can work on it and submit a PR.

lukeed commented 6 years ago

Hey!

This is actually a Polka-level (or user-level) implementation, imo, because a sub-router is really just another router that begins looking when the previous router gave up.

You have a few ways of going about it, but the easiest is to just break apart routes with the same base into a single router.

const routers = [];
const noop = () => {};

// Main (T)Router
routers.push( new Trouter().get('/', noop).get('/foo', noop) );

// "Sub" (T)Router
routers.push( new Trouer().get('/items', noop).get('/items/:id', noop) );

//~> Handling
let i=0, len=routers.length, tmp;
for (; i < len; i++) {
  // Search for combo within current router
  // ~> false ==> next router
  if (tmp = routers[i].find(METHOD, PATH)) return tmp;
}
return 'Not found';

The other option would rely on a .all() method (#1) that accepted all /items/* combinations, only to pass off to the secondary router's matching logic.

// "Sub" (T)Router
const items = new Trouer().get('/items', noop).get('/items/:id', noop);

const onError = noop;

// Main (T)Router
const main = new Trouter();

main
  .get('/', noop)
  .get('/foo', noop)
  .all('/items/*', (req, res) => {
    // Trim the url
    req.url = '/' + req.url.substring(6);
    return items.find(req.method, req.url);
  });

//~> Handling
let obj = main.find(METHOD, PATH);
if (obj) return obj.handler(...);
// Not found
return (onError || noop);

That said, I'm still on the fence about .all() 😕 I haven't given it much thought yet, but will after Polka is mostly set.

Also worth mentioning, the 1st option is essentially what Polka is doing within the local refactor. It works pretty well!

Let me know if that helps 😄

iCyberon commented 6 years ago

Thanks for the answer @lukeed.

The problem is that once have more than certain amount of routes it becomes "messy" to manage child routes. Here's an example of what I mean.

/posts
/posts/:pid
/posts/:pid/comments/
/posts/:pid/comments/:cid
/posts/:pid/comments/:cid/likes
/posts/:pid/comments/:cid/likes/:lid

This will become:

// main
polka()
.use(posts)
.listen(PORT).then( _ => {
    console.log(`> Running on localhost:${PORT}`);
});

// posts
polka()
.get('/posts, noop )
.get('/posts/:pid', noop )
.use(comments)
.listen(PORT).then( _ => {
    console.log(`> Running on localhost:${PORT}`);
});

// comments
polka()
.get('/posts/:pid/comments, noop )
.get('/posts/:pid/comments/:cid, noop )
.use(likes)
.listen(PORT).then( _ => {
    console.log(`> Running on localhost:${PORT}`);
});

// ...

As you can see we're basically repeating parent route path for every child router. It just hit me, polka can accept base option inside opts and prepend it (inside Trouter's add method) to every registered handler.

What do you think?

lukeed commented 6 years ago

No problem~

Yeah, for Trouter it gets messy. This is why this logic belongs to the server itself, because there are a lot of different ways one may want to handle this — both defining the routes & retrieving them.

This isn't live yet, but Polka 0.3.0 has greatly improved sub-app support. Your example will look like this:

let posts = 
  polka()
    .get('/', noop)
    .get('/:pid', noop)
    .use('comments', 'comments')

let comments = 
  polka()
    .get('/', noop)
    .get('/:cid', noop)
    .use('likes', likes)

// main
polka()
  .use('posts', posts)
  .listen(PORT).then( _ => {
    console.log(`> Running on localhost:${PORT}`);
  });

By the way, you should only be calling listen on the main app.

iCyberon commented 6 years ago

Yeah, that was copy-paste error. Unless I'm missing something use is a variadic function and I like it, changing use was my first idea, but dropped it because of that.

Apparently changing add a little bit is enough.

add(method, pattern, handler) {
    // Save decoded pattern info
    this.routes[method].push(parse(this.opts.base || '' + pattern));
    // Save route handler
    this.handlers[method][this.opts.base || '' + pattern] = handler;
    // Allow chainable
    return this;
}
lukeed commented 6 years ago

In the next version, yes, it's variadic.

You should update the pattern once & early. Also need to apply logic to make sure you're not ending up with double slashes somewhere.