expose the matched routes for a request

[evanshortiss]

I'm open to suggestions on improving this one. Basically if a route is "/foo/:num/bar/:num" and a client requests "/foo/12/bar/32" I would like to be able to see that they hit "/foo/:num/bar/:num":

Here's how this PR does that:

app.get('/foo/:num/bar/:num', function (req, res) {
  var matched = req.matchedRoute; // "/foo/:num/bar/:num"
  var originalUrl = req.originalUrl; // "/foo/12/bar/32"
});

It works for nested routes also. This can be seen in the new tests added.

It would also be possible to expose an Array instead if desirable, e.g ['/foo/:num/bar/:num'] with each matched layer/router inside.

[evanshortiss]

This is a little verbose, and makes me feel that perhaps an Array structure might be better suited since we can then avoid this logic.

[evanshortiss]

I've made this toward master, but perhaps that was a mistake. I'm hoping to add it to 4.x also 👍

[dougwilson]

So my immediate thoughts are that I don't see any tests for regular expression routes or arrays of routes. How does this PR support those? We would want to provide support for all the methods of declaring routes out of the box, if that means either restrict the types of ways you can declare routes (not great) or just support them all seamlessly (better).

[evanshortiss]

Excellent point. I've only catered to my specific use case at present. Switching to an Array would resolve this issue since then any type of expression is supported. Will update shortly.

[evanshortiss]

Updated to use the Array structure discussed so as to preserve the different route declaration types. Thanks for taking the time to review 😄

[evanshortiss]

What are your thoughts on the updated implementation @dougwilson? I think it covers the issues raised.

[dougwilson]

I am unclear on how a user would benefit from the feature, especially how would they recompose the array of routes to generate something meaningful, especially if they are a mix of strings and regex literals.

[evanshortiss]

Currently express only tells you the exact route that was called, with its specific parameters. If you need the pattern it matched, you need to do some extra work. It would be beneficial to expose the matched route sans the unique route params to avoid that work.

Personally, this would be beneficial from a tracking standpoint. For example, assume we have the following route:

app.get('/users/:id/tasks/', statisticsMiddleware, routeHandler);

If we want to track calls to "/users/:id/tasks/" req.url and req.originalUrl are unique per user so rather than a single counter, we have n counters where n is the number of active users. If we had a way to access the original "/users/:id/tasks/" (req.matchedRoute or similar) it would simplify things.

We could do this, but it's less elegant:

const routeDef = '/users/:id/tasks/';
app.get(routeDef, statisticsMiddleware(routeDef), routeHandler);

I aimed to design as per the comment here, but I'm happy to continue discussion! My understanding is that an Array of matched patterns was desired, and as @danieljuhl said users could be responsible for building these into a desired format, or they could just do req.matchedRoutes.pop().toString() to get the current matched route. As a module author I can respect that req.matchedRoutes.pop().toString() might be a little less elegant than desired though, so instead of just matchedRoutes, you could have curMatchedRoute (forgive the nomenclature) also - similar to req.url and req.originalUrl.

[evanshortiss]

Also in terms of "to generate something meaningful"; I think that will vary, but the ability to generate it would be terrific.

[dougwilson]

Sorry, I have been away, and haven't been able to respond fully yet, but did want to ask for your example above: could you not achieve the same thing using the existing req.route? Why or why not?

[dougwilson]

Basically whatever lands, it will be very difficult to change in the future, especially since a middleware cannot influence the version of express easily. I'm basically trying to walk through how the feature will be used so we can come to a conclusion on if it will work well, how to direct people to use it, and more. Ideally, if we can get more people in this conversation that would help a lot, at least to understand if this will actually meet the needs which drove them to open the issues and PRs originally.

[evanshortiss]

Thanks for sticking with this! I will bring discussion back to the original issue.

[dougwilson]

So this pull request is quite old, and a lot of things have shuffled around. This is the current pull request incarnation of this feature: pillarjs/router#86