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

subrouters and middlewares #41

Open
gsabran opened this issue Jul 15, 2016 · 5 comments
Open

subrouters and middlewares #41

gsabran opened this issue Jul 15, 2016 · 5 comments

Comments

@gsabran
Copy link

gsabran commented Jul 15, 2016

It's not clear what is the excepted behavior with middleware at the subrouter level. At the moment, this happens:

const fakeMiddelware = (i) => (req, res, next) => {
  req.MiddelwareCount = req.MiddelwareCount || 0;
  req.MiddelwareCount += i;
  console.log('fakeMiddelware', i, 'run');
  next();
};

Picker.middleware(fakeMiddelware(1));

const group1 = Picker.filter();
group1.middleware(fakeMiddelware(2));

const group2 = Picker.filter();
group2.middleware(fakeMiddelware(4));

Picker.route('/ping', (params, req, res) => {
  console.log('ping received');
  console.log('MiddelwareCount is', req.MiddelwareCount);
  res.end('ok');
});

group1.route('/ping1', (params, req, res) => {
  console.log('ping1 received');
  console.log('MiddelwareCount is', req.MiddelwareCount); // only the first 2 middleware are run, so this is 3
  res.end('ok');
});

group2.route('/pin2', (params, req, res) => {
  console.log('ping2 received');
  console.log('MiddelwareCount is', req.MiddelwareCount); // all three middleware are run, so this is 7
  res.end('ok');
});

Would it not make more sense for the middlewares to be applied only if the route matches, and for a subrouter to apply its parent router's middlewares first and then the subrouter level middlewares?

@gsabran
Copy link
Author

gsabran commented Jul 15, 2016

I've made a PR: #42

@came
Copy link

came commented Feb 26, 2017

this makes total sense to me ...why should middleware of group1 run on a group2 route?

@gsabran
Copy link
Author

gsabran commented Mar 3, 2017

@arunoda is the project still active ?

@MaxGuitet
Copy link

If someone else faces the same issue, the reason for it lies in the code. In the example given, you would expect MiddelwareCount to be 3 (1 + 2) for group1 and 5 (1 + 4) for group2. But when you send a request to /ping2, because the group1 has no filterFunction, the group1 middlewares are processed as per here.
The problem is the same if you have a filter on method only. For example if you define the two groups as this:

const group1 = Picker.filter(req => req.method === 'POST');
group1.middleware(fakeMiddelware(2));

const group2 = Picker.filter(req => req.method === 'POST');
group2.middleware(fakeMiddelware(4));

you will end up with the same problem because the filterFunction returns true for both (see here ).

One solution I found is to filter by the route of the filtered group stored in this.routes:

import _ from 'lodash';

const mailboxRoutes = Picker.filter(function(req) {
  return req.method === 'POST' && _.some(this.routes, routeRegEx => routeRegEx.test(req.url));
});

@chfritz
Copy link

chfritz commented Mar 5, 2021

For anyone interested in route-specific middleware and not wanting to bother with filters, I've figure that out and added a wiki page for it: https://github.com/meteorhacks/picker/wiki/Example-of-route-specific-middleware

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

4 participants