Open IamLizu opened 2 weeks ago
It seems that this.path
is being set to undefined
because we can't determine what path
the Layer
is matched against until runtime.
Let me explain my thought process: Layers are added to the Router instance when the use
and route
methods are executed.
The Router.prototype.run
method only accepts a string as path
, but Router.prototype.use
can accept either a string or an array of strings. You can refer to the Express documentation for more details.
We can't know which of the possible path
values was matched until the layer is actually matched at runtime, as seen here.
To address the issue discussed in express#5961, perhaps we could add an extra property to the Layer instance, such as paths
or pathPatterns
. This would make it possible to list all routes or a router.
@pillarjs/express-tc, can we get your input on this matter?
Thank you for the explanation @carpasse.
I still have some confusion, for example, the actual matching against a request's path happens at runtime. However, if we set the Layer
's path
property when the Layer
is created, will it create any issue?
Edit: Since the path
is from request is being passed to matchLayer
which is checking and setting the path
to Layer
in runtime, it might add a little cost but perhaps its negligible? On the other hand, instead of adding a new property, maybe we could set path
during Layer
creation which helps debugging.
PS: I really have very little idea, I am trying to get a better understanding.
Edit: I wonder whether any other part depends on this.path
being undefined
. I will try to dig in.
@IamLizu I believe the path
argument passed to the Layer
constructor represents either a list of handler paths (as an array) or a single string. To improve clarity, It may be best to rename that constructor parameter to something like pathPatterns
or layerPaths
, as I think it better reflects its purpose.
Additionally, the path
property is expected to hold the matched portion of the runtime path based on the patterns passed to the constructor.
path
is a substring match of the runtime path, not directly one of the paths passed to the constructor. Reference 3path
property cannot be defined until it has been matched against a runtime path. Reference 4layerPath
, and then used to trim off the matched portion from the runtime path. Reference 5cc @wesleytodd as repo captain
Can someone please help me understand why
this.path
is set toundefined
? https://github.com/pillarjs/router/blob/2e7fb67ad1b0c1cd2d9eb35c2244439c5c57891a/lib/layer.js#L43I had checked the router Layer, and the path was
undefined
in almost every route, that shouldn't be the case, right?While I don't know the reason of it being set to
undefined
initially, I can suggest it should be set topath
. I have noticed that thepath
on Layer is no longer undefined if we set,Instead, it gets correct path such as
/users
or whatever the path actually is.