orval
orval copied to clipboard
anymaniax
fix: allParamsOptional implemented in params and props
Status
Ready
Fix #1322
Description
- Enhanced the current build setting allParamsOptional (refer to https://github.com/anymaniax/orval/issues/1322).
- Fixed/removed the TODO comment (
Maybe ensure that 'paramType' isn't already undefined or null). In my view, optional signifies a property marked with a ?, rather than extending the type with| undefined | null.
I experimented with these adjustments locally in a playground project using Orval clients: default (axios), vue-query, and Angular. I have created a draft PR to ensure that my changes pass all the pipeline tests. These adjustments are to the core, so I hope that by testing them with all the mentioned setups, I have thoroughly validated my changes.
Please feel free to double-check before approving my PR
Related PRs
n/a
Todos
n/a
Steps to Test or Reproduce
n/a
@Maxim-Mazurok you were recently in there as well want to give these a review?
@wouterkroes
I agree with they. I think this change is a workaround and essentially requires revising the definition of OpenAPI itself for the relevant use case. What do you think?
First of all, thanks to everyone for the responses.
This entire discussion has made me rethink what I wanted to achieve and the implementation I had in mind. As a result, I've changed my plan and no longer need the changes proposed in my pull request. Therefore, I suggest we close this pull request.
It would be beneficial if we could enhance the Orval documentation. @Maxim-Mazurok suggestion to "fix this issue by renaming allParamsOptional to allPathParamsOptional and/or updating the documentation" makes sense.
@wouterkroes
I got it. Thanks to you for suggesting this, I found a way to improve it with another approach, so I appreciate it. I'd prefer documenting the options instead of destructively renaming them, as it's a minor improvement, but would you update the documentation in a separate PR?