pillarjs / router

Simple middleware-style router
MIT License
409 stars 102 forks source link

Remove `layer.path` and `layer.params` as class properties #80

Closed golopot closed 5 years ago

golopot commented 5 years ago

These two properties, layer.path and layer.params, should not be exposed as class properties. Their really role is the return value of Layer.prototype.match.

https://github.com/pillarjs/router/blob/51c56e61b40ab486a936df7d6b09db006a3e9159/lib/layer.js#L108

dougwilson commented 5 years ago

Does It really matter? The Layer object and it's methods are not public API nor are they documented. It's just internals.

golopot commented 5 years ago

I was trying to understand the internal of express through a repl and was confused by layer.path and layer.params. Especially when they stand next to to route.path, and route.params but have different meanings. The issue is about a small improvement in code readability.

dougwilson commented 5 years ago

You can just make a pull request then if you like. It works well enough and we're unlikely to put the effort to refactor internals for no benefit. If you think it is easier to debug and read, then you're welcome to make the effort to refactor with a pull request and articulate in the request how specifically it is easier, etc :+1: