-
-
Notifications
You must be signed in to change notification settings - Fork 211
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow optional use of req.url
#857
Conversation
486fb66
to
cd4cf8b
Compare
cd4cf8b
to
481c7b3
Compare
Thanks for the change. I will review in next few days. Good question on the wiki.... Since migrating the doc to wiki, I do not currently have a good mechanism for updates. Open to suggestions. I found this on SO https://stackoverflow.com/questions/10642928/how-can-i-make-a-pull-request-for-a-wiki-page-on-github Perhaps post the doc content in the comments here and I will manually integrate into the wiki upon merge |
@nikkegg Thanks for picking this up! Excited to use this! |
@@ -49,6 +49,7 @@ export class OpenApiValidator { | |||
if (options.$refParser == null) options.$refParser = { mode: 'bundle' }; | |||
if (options.validateFormats == null) options.validateFormats = true; | |||
if (options.formats == null) options.formats = {}; | |||
if (options.useRequestUrl == null) options.useRequestUrl = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you help me understand why this is an option controlled by the client? what is the downside using this behavior i.e. req.path
and req.url
for all requests? for instance, can we make both paths work withour requiring the client to configure the behavior
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
req.path
and req.url
are localized by express to be router specific, so if we nest router A in router B req.path
/req.url
will only have router A data (/test
), no router B data (/api/
). #211 introduced the current default behavior as the desired use case was that users were validating against a single spec that included router B data. I don't think there's an easy way without naive searching of the spec to handle both.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cdimascio what @sennyeya said - using req.path
or req.url
is scoped to the router using it and is unaware of the url defined by the parent router. The flip side of this is that you can't use separate schemas in the parent router and child router, because originalUrl
is always set to the one defined by the parent router i.e /api
. But using req.url
in the child router allows us to decouple child and parent routers and schemas. Also req.url
is intended to be used like so by the express framework I believe.
@cdimascio just a little bump on this 🙏🏻 |
@nikkegg the change is in v5.1.0, can you provide a brief doc write up for its usage. i will include it in the doc |
Solves #843 and #113
@cdimascio I could not find where to update the docs wiki, but am happy to do so if you point me in the right direction.