Inherit parameters from path item to its operations. #530
Conversation
src/transform/operation.ts
Outdated
| globalParameters?: Record<string, ParameterObject>; | ||
| } | ||
| ): string { | ||
| let output = ""; | ||
|
|
||
| if (operation.parameters) { | ||
| output += ` parameters: {\n ${transformParametersArray(operation.parameters, { | ||
| if (operation.parameters || pathItem?.parameters) { |
There was a problem hiding this comment.
This line seemed to throw ts-jest. If you could rebase off latest main and push again, we should see the tests run.
There was a problem hiding this comment.
optional chaining is not supported by the current typescript settings.
There was a problem hiding this comment.
Thanks @dnalborczyk. But weird, I obviously ran this locally and ts-jest should have used tsconfig.json with the correct transpile options. Could this possibly be related to Node 12 being used in CI while I use Node 14?
But no problem changing it obviously.
There was a problem hiding this comment.
good question. not quite sure. the test run also failed in the ci with node.js v14: https://github.com/drwpow/openapi-typescript/runs/2141031952 . I'd have to pull your PR with that commit and run it locally as well.
There was a problem hiding this comment.
there seems to be a bug in the ci setup: v14.x and v12.x are essentially running node v10.x
There was a problem hiding this comment.
Thanks so much for improving this.
|
I think this looks good! But please run the tests locally yourself, and make sure that they’re all passing (and please update any |
|
|
||
| }`); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Thanks! The tests obviously did work when I ran this locally, but not in CI for some reason. Checking if removing Node 14 stuff solves it.
Codecov Report
@@ Coverage Diff @@
## main #530 +/- ##
==========================================
+ Coverage 84.87% 85.00% +0.12%
==========================================
Files 9 9
Lines 324 340 +16
Branches 105 110 +5
==========================================
+ Hits 275 289 +14
- Misses 46 47 +1
- Partials 3 4 +1
Continue to review full report at Codecov.
|
tests/operation.test.ts
Outdated
| @@ -74,3 +74,112 @@ describe("requestBodies", () => { | |||
| ); | |||
| }); | |||
| }); | |||
|
|
|||
| describe.only("parameters", () => { | |||
A path item may declare common parameters to all operations outside of the operation map. If this is the case, all operations should inherit those parameters and merge them with any operation specific parameters. In case of conflicting parameters, the one defined in the operation has precedence over path item parameters.
drwpow
force-pushed
the
inherited-path-parameters
branch
from
32b75a4 to
6e8f517
Compare
|
@henhal I rebased your branch and fixed the conflict, but kept everything in your code the same. Hope that’s OK. Wanted to merge this to release soon! 🚀 |
|
@all-contributors please add @henhal for code, test |
|
I've put up a pull request to add @henhal! 🎉 |
|
@drwpow Thanks for rebasing and accepting the PR. 🎈 |
A path item may declare common parameters to all operations outside of the operation map.
If this is the case, all operations should inherit those parameters and merge them with
any operation specific parameters.
In case of conflicting parameters, the one defined in the operation has precedence over
path item parameters.
To fix this, operations need a reference to their path item, so modifying the record of operations built during path item parsing to hold { operation, pathItem } objects.