Hosting a service not at root causes incorrect path generation about express-openapi HOT 6 CLOSED

I'd like to open this up again, if you don't mind.

Don't mind at all! I think this is a good thing and exactly why I published the module in the first place, to collaborate on it!

However, you've suggested adding a basePath option, which I think is nice and modular. If I understand you correctly, this would be a new option that you can pass as the _opts parameter in the initial setup.

Yep exactly.

Presumably, we would pass this option to the generateDocument function, and do the same stripping from the path as we do in the aforementioned PR,

Yes that sounds right off the top of my head. Might need a bit of testing (exmples, manual or automated) to make sure we cover the use cases. But probably better to open the initial PR (maybe even as draft) so we can all discuss.

regardless of the server URLs

Yeah I agree a naive approach might have this issue, but I think we could find a way around that. Maybe if basePath is a string it does it to all, but if it is an object, the keys are server url?

from express-openapi.

Hey, I am so sorry for not responding. I was pretty checked out of my oss work for a while. I am catching up on this package now though, I hope you found a way in the mean time!

Now to the issue. I am honestly not sure if there is a way around it with the code as it is. I do think thought that passing a different baseUrl as a config option would be a great addition to override the default detection it does against the express router. Would happily accept a PR to support this behavior!

from express-openapi.

Hey! No worries at all 😄 We fixed it ourselves in this Unleash PR, so we're all good for now.

Because of this, I don't think that we'll prioritize creating the PR for now, if that's alright with you? But at least now we know in case we want to revisit this in the future ☺️

from express-openapi.

Hey @thomasheartman, Was it my lack of response what led to it not being up-streamed here? I could probably translate your PR, but I and others who use this would benefit from contributing back if you can.

from express-openapi.

No, sorry, not at all 😅 But we ended up handling it by making a change that I suspect is only useful to us. Here's the PR on our fork: Unleash#11. I forgot we changed anything there when I made my comment earlier.

Because we'll only use a single server the way we do it, we ended up using that as a way to strip paths. So in other words, we didn't introduce a baseUrl config option, and I didn't think this solution would be useful to other people.

If you think otherwise, I'd of course be happy to see if we can't upstream it. It just sounded as if you wanted a different solution, and I'm not sure we want to spend the time to craft that option if it's not something that'll be useful to other people. What do you think?

from express-openapi.

@wesleytodd I'd like to open this up again, if you don't mind. We've been reviewing this repo and would like to avoid having an internal fork if possible. So I'd be very happy to upstream the changes we've made if you'll have them 😄

But before making the PR, I'd like to discuss the solution a bit with you, so we can avoid doing more work than what is necessary.

The solution we've come to locally is this:
Because we only ever use a single server in the list of servers, we've gone ahead and stripped the end of that path from the start of the path (these three lines, specifically, from Unleash#11).

However, you've suggested adding a basePath option, which I think is nice and modular. If I understand you correctly, this would be a new option that you can pass as the _opts parameter in the initial setup.

Presumably, we would pass this option to the generateDocument function, and do the same stripping from the path as we do in the aforementioned PR, just using the supplied basePath instead. Is that correct? (The changes for that seem pretty small and I've got a working local version doing just that for now).

If that is the way to go, though, I'm assuming we'll do the same thing indiscriminately to all the supplied paths, regardless of the server URLs? Just a straightforward replace, yeah? I don't know which cases people would have to use multiple servers, but could this cause problems for them? I mean, if they don't use it, it obviously shouldn't, and if they do want to use it, we should probably wait to cross that bridge till we get there, yeah (and find someone who knows how it should work in those cases).

from express-openapi.