Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: Using koa-router to extend application instance has undocumented side effects #120

Closed
mdvorak opened this issue Apr 23, 2015 · 3 comments

Comments

@mdvorak
Copy link

mdvorak commented Apr 23, 2015

When one extends koa app as is in docs:

app.use(router(app))

It does not always behave as one would assume at first glance. Docs does not mention, that router itself is published as app.router and that not all methods are actually propagated, especially use.

Behavior of use for this use-case comes from koa architecture, but it has taken me by surprise:

var koa  = require('koa');
var router = require('koa-router');
var app = koa();

app.use(function*(next) {
  console.log("First MW");
  yield next;
});

app.use(router(app));

app.use(function*(next) {
  console.log("Defined after router");
});

app.get('/sample', function*() {
  this.body = "sample";
});

This has a little unexpected output, since it writes only "First MW" and ignores another middleware. It is logical, since router handles the request that is registered using app get, and it is itself middleware. But if one uses router directly, the use method registers middleware directly to the router and it works as expected.

To get this going, one needs to do

app.router.use(function*(next) ... );

but thats not obvious and should be at least mentioned in the docs. Question is, whether use method should not be overriden in this case completely as well.

@alexmingoia
Copy link
Collaborator

Yes, this should be better documented. Honestly, there's no reason to allow this behavior at all but it was a holdover from a previous API that made it easier for people to transition their apps without rewriting a bunch of code.

var koa = require('koa');
var Router = require('koa-router');

var app = koa();
var router = new Router();

router.get('/test', function *(next) {
  // ...
});

router.use(...); // only use middleware at router level

app.use(router.routes());

@mdvorak
Copy link
Author

mdvorak commented Apr 24, 2015

Well, then I would recommend either deprecating the feature, or making it behaving as expected - that is, replace app.use with implementation from the router.
Thanks for looking into it.

@alexmingoia
Copy link
Collaborator

Newly released 5.0.0 does no include the ability to extend an app with router methods.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants