Skip to content
This repository has been archived by the owner on Jan 12, 2022. It is now read-only.

Add support for sub routers with parameterized routes #80

Merged
merged 3 commits into from
Mar 5, 2019

Conversation

ghmeier
Copy link
Contributor

@ghmeier ghmeier commented Nov 21, 2018

Previously, when using a sub router on a parameterized route like
app.use('/foo/:param', router), findRouteName would not correctly
resolve the route parameter for the parent route. This happened because
req.baseUrl contained the url with the value, not the parameter. To
solve this, this PR adds code to translate from the url with the value,
back to the parameterized version (i.e. /foo/value -> /foo/:param).

Previously, when using a sub router on a parameterized route like
`app.use('/foo/:param', router)`, `findRouteName` would not correctly
resolve the route parameter for the parent route. This happened because
`req.baseUrl` contained the url with the value, not the parameter. To
solve this, this PR adds code to translate from the url with the value,
back to the parameterized version (i.e. `/foo/value -> /foo/:param`).
@ghmeier
Copy link
Contributor Author

ghmeier commented Nov 21, 2018

Quick note - it looks like the tests failed in node 4, but that also occurs when running them on master without these changes due to a major version change of debug, a dependency of mocha 1.6.0 that's installed with * in that version. Seems like it'd be impossible to run the tests in node 4 without manually adding the debug dependency and pinning it to ^3.2.6.

@ghmeier
Copy link
Contributor Author

ghmeier commented Mar 2, 2019

Hi @msiebuhr, any update on merging this? We've been running this for quite some time with no issues.

@msiebuhr
Copy link
Owner

msiebuhr commented Mar 4, 2019

But tests on node-4 fails. Since it's probably time to ditch it anyway, would you mind fixing the config for travisci up, so we can get rid of the error?

@ghmeier
Copy link
Contributor Author

ghmeier commented Mar 4, 2019

Agreed since node 4 has reached EOL. I'll update the config 👍

@msiebuhr msiebuhr merged commit ad68360 into msiebuhr:master Mar 5, 2019
@ghmeier ghmeier deleted the ghmeier/handle-sub-router-params branch March 5, 2019 16:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants