Skip to content

WIP: Adding requestBodies transform support #474

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

Merged
merged 2 commits into from
Mar 2, 2021

Conversation

radist2s
Copy link
Contributor

@radist2s radist2s commented Feb 9, 2021

According to OpenAPI 3.0 components.requestBodies have a specific schema:

components:
  requestBodies:
    PetBody:
      description: A JSON object containing pet information
      required: true
      content:
        application/json:
          schema:
            $ref: '#/components/schemas/Pet'

Currently components.requestBodies is transformed as normal schema with the wrong output code:

  requestBodies: {
    /** List of user object */
    UserArray: { [key: string]: any };
    /** Pet object that needs to be added to the store */
    Pet: { [key: string]: any };
  };

This PR fix wrong transformation, and makes ability to have a valid output code:

  requestBodies: {
    /** List of user object */
    UserArray: {
      content: {
        "application/json": components["schemas"]["User"][];
      };
    };
    /** Pet object that needs to be added to the store */
    Pet: {
      content: {
        "application/json": components["schemas"]["Pet"];
        "application/xml": components["schemas"]["Pet"];
      };
    };
  };

@radist2s
Copy link
Contributor Author

radist2s commented Feb 9, 2021

Ok, at least tested all suits, I had timeout issue on my side. WIP

@radist2s radist2s changed the title Adding requestBodies transform support WIP: Adding requestBodies transform support Feb 9, 2021
@codecov
Copy link

codecov bot commented Feb 9, 2021

Codecov Report

Merging #474 (7fb007d) into main (026eb74) will decrease coverage by 14.00%.
The diff coverage is 78.13%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main     #474       +/-   ##
===========================================
- Coverage   93.81%   79.80%   -14.01%     
===========================================
  Files           5        9        +4     
  Lines         291      312       +21     
  Branches       97       96        -1     
===========================================
- Hits          273      249       -24     
- Misses         16       63       +47     
+ Partials        2        0        -2     
Impacted Files Coverage Δ
src/transform/index.ts 26.92% <26.92%> (ø)
src/transform/headers.ts 30.00% <30.00%> (ø)
src/transform/responses.ts 86.84% <86.84%> (ø)
src/transform/paths.ts 91.66% <91.66%> (ø)
src/transform/operation.ts 96.42% <96.42%> (ø)
src/transform/schema.ts 98.27% <98.27%> (ø)
src/index.ts 76.47% <100.00%> (-5.35%) ⬇️
src/transform/parameters.ts 100.00% <100.00%> (ø)
src/utils.ts 91.22% <100.00%> (-6.96%) ⬇️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d5e2b28...7fb007d. Read the comment docs.

@radist2s
Copy link
Contributor Author

radist2s commented Feb 9, 2021

@drwpow, what is needed to pass the Codecov test? I tried to open report on Codecov, but have got the error message: "Missing base report".

Copy link
Contributor

@drwpow drwpow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good! Thanks for adding.

@@ -31,3 +33,38 @@ describe("requestBody", () => {
).toBe(`requestBody: components["requestBodies"]["Request"];`);
});
});

describe("requestBodies", () => {
const format = (source: string) => prettier.format(source, { parser: "typescript" });
Copy link
Contributor

@drwpow drwpow Mar 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably remember the previous versions of the test. Here’s some extra context for why it‘s not in the unit tests now.

Prettier saves a lot of code managing indents, quotation style, etc. By just running Prettier at the end, this library can do less formatting, and only has to render valid (albeit messy) TypeScript. But recently I noticed that this library wasn’t generating syntax correctly in places, and Prettier sometimes would format incorrectly. So I wanted to write more tests without Prettier to make sure the generated code was valid TS, and Prettier wasn’t hiding any issues. But that comes with drawbacks, obviously—it’s annoying to write & test unformatted code.

Anyway, I’m fine having Prettier in some tests, and it’s fine here! But in most places it’s been intentionally left out.

UserArray: { [key: string]: any };
UserArray: {
content: {
"application/json": components["schemas"]["User"][];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice test!

@drwpow
Copy link
Contributor

drwpow commented Mar 2, 2021

@drwpow, what is needed to pass the Codecov test? I tried to open report on Codecov, but have got the error message: "Missing base report".

Just fixed that problem now. Sorry about that. This looks good! Thank you.

@drwpow drwpow merged commit 009fc07 into openapi-ts:main Mar 2, 2021
@drwpow
Copy link
Contributor

drwpow commented Mar 2, 2021

@all-contributors please add @radist2s for code, test

@allcontributors
Copy link
Contributor

@drwpow

I've put up a pull request to add @radist2s! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants