feat: export operations interface
#353
Conversation
Codecov Report
@@ Coverage Diff @@
## main #353 +/- ##
==========================================
+ Coverage 92.57% 92.91% +0.33%
==========================================
Files 5 5
Lines 256 268 +12
Branches 88 90 +2
==========================================
+ Hits 237 249 +12
Misses 13 13
Partials 6 6
Continue to review full report at Codecov.
|
| * Pet not found | ||
| */ | ||
| '404': unknown | ||
| put: operations['updatePet'] |
| /** | ||
| * Multiple status values can be provided with comma separated strings | ||
| */ | ||
| findPetsByStatus: { |
There was a problem hiding this comment.
I’m curious how moving this JSDoc affects VS Code, if at all. Not really a concern; just curious (I actually received a lot of comments about this feature! It’s surprisingly popular)
There was a problem hiding this comment.
good question, I don't know but I'll check it out and report back
There was a problem hiding this comment.
in VS code the description now only shows up for operations["findPetsByStatus"], but not for paths["/pet/findByStatus"]["get"], which is a bummer ... I feel like that's something that should be fixed in the VS Code integration though ... I wouldn't add the same description to both places. We don't do that when we reference other components either. Let me know what you think
There was a problem hiding this comment.
Ah gotcha. Yeah I guess you could maybe hoist the comments, but that is one of the most annoying parts of the code. I wouldn‘t worry about it until someone raises it because it‘s not a great payoff
| }; | ||
| expect(swaggerToTS(schema)).toBe( | ||
| format(` | ||
| export interface operations {} |
There was a problem hiding this comment.
🤔 at first I was going to ask to not omit this if empty, but after thinking about it, I think I like always having it there, rather than being conditionally there or not. Seems safer to always omit
tests/v3/index.test.ts
Outdated
| describe("cli", () => { | ||
| ["github", "stripe", "manifold", "petstore"].forEach((file) => { | ||
| it(`reads ${file} spec (v3) from file`, () => { | ||
| it.only(`reads ${file} spec (v3) from file`, () => { |
There was a problem hiding this comment.
oh oops ☝🏼 will remove
There was a problem hiding this comment.
Oh my gosh didn‘t catch that. Glad you did!
| /** | ||
| * Multiple status values can be provided with comma separated strings | ||
| */ | ||
| findPetsByStatus: { |
There was a problem hiding this comment.
good question, I don't know but I'll check it out and report back
|
no idea why tests fail on CI, they do not fail locally ... any idea what it might be? |
Oh try rebasing off latest |
🤦🏼 could have thought of that. On it |
gr2m
force-pushed
the
341/operations-interface
branch
from
31b2744 to
bc10c32
Compare
|
let me know what you think of #353 (comment). Other than that this is good to merge & release |
Oh sorry; I wasn‘t clear. This LGTM as-is; I wouldn‘t worry too much about it for now. |
|
Published |
This change now always exports an
operationsinterface, even if there are no operations withoperationIds. I think that works better for predictability.I really like how the
pathsexport becomes very nice overview of the whole schema, e.g. seegithub.tscloses #341