express icon indicating copy to clipboard operation
express copied to clipboard

Router mergerParams setting on app initialization

Open blazmrak opened this issue 1 year ago • 3 comments

Hi,

is there way to make app merge params from the parent app right now? As far as I can tell only the strict and caseSensitive settings are configurable: https://github.com/expressjs/express/blob/33e8dc303af9277f8a7e4f46abfdcb5e72f6797b/lib/application.js#L144-L149 https://github.com/expressjs/express/blob/318fd4b543ffbebf97bf0b6c49188afae45741f5/lib/application.js#L64-L75

My use case is that I have the root app and child apps for view rendering, because I am able to limit the views available to the app, which makes code a bit shorter and cleaner and also project structure is nicer. However I don't have the access to the path parameters in this case.

const root = express()
const app = express()
app .set('views', 'path1')
app .get('/', (req) => console.log(req.params)) // prints '{}'

root.use('/path/:someParam', app)

I can submit a PR as it is only one line of code if I didn't overlook something.

blazmrak avatar Oct 02 '22 12:10 blazmrak

Hello, thank you for your issue. You are right, there is no option for mergeParams for an app, only routers. I don't see any reason off-hand that an app cannot also enable that setting. If you would like to make a PR, you're welcome to do so, just remember to add all the necessary tests around the feature, not just one line to the one file :) . If you're not comfortable doing that, we can still add it for you, just let us know.

dougwilson avatar Oct 02 '22 15:10 dougwilson

I'll take a stab at it. By the way, how are different branches handled? Do I submit PR for both master and 5.0, or just 5.0?

blazmrak avatar Oct 02 '22 20:10 blazmrak

I believe this is just adding a feature (in the form of a new option), so if that is the case just master is all you need.

dougwilson avatar Oct 02 '22 21:10 dougwilson