Skip to content

Support raw schemas in addition to Swagger/OpenAPI documents #263

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 3 commits into from
Oct 8, 2020

Conversation

henhal
Copy link
Contributor

@henhal henhal commented May 26, 2020

This adds support for supplying raw v3 schemas or v2 definitions as an alternative to supplying full Swagger/OpenAPI documents, using an option --raw-schema in combination with --version.

Previously, swagger-to-ts would require components to be present in the given document, even though it's an optional property. When dealing with large documents, it's common practice to
split it over several files, putting e.g. schemas in separate files.

This change enables producing types for such stand-alone schemas by adding an option to parse the given file as a map of schemas rather than a document. For this to work, the version must also be passed since it's not present in raw schemas/definitions.

For example, if the full document is:

openapi: 3.0.2
components:
  schemas:
    Foo:
      type: object
    Bar:
      type: object
      properties:
        foo: {$ref: '#/components/schemas/Foo'}

Then it's now possible to parse the schemas separately by passing a file with the following contents, and --raw-schema --version 3 as options:

Foo:
  type: object
Bar:
  type: object
  properties:
    foo: {$ref: '#/Foo'}

For V3, the output differs from the output produced for OpenAPI3 documents in that
schemas: { ... } is the exported interface instead of
components: { schemas: { ... } }.

For V2, definitions: { ... } is still exported.

henhal added 2 commits May 26, 2020 15:01
This adds support for supplying raw schemas instead of Swagger/OpenAPI documents,
using an option --raw-schema in combination with --version.

This enables producing types for stand-alone schemas, which is useful for large
APIs where schemas have been extracted to separate files, and where there may
be no components object in the document.

For V3, the output differs from the output produced for OpenAPI3 documents in that
`schemas: { ... }` is the exported interface instead of
`components: { schemas: { ... } }`.

For V2, `definitions: { ... }` is still exported.
@henhal
Copy link
Contributor Author

henhal commented May 26, 2020

This relates to #262.

Note that no new test cases are added etc, as this was somewhat whipped together to get early feedback on if this is something you want at all.
Possible improvements could include automatically detecting the type of passed document etc?

Anyway, all existing tests work and my manual tests of my schema files also produce correct output.
If interesting to proceed with, tests should of course be added.

@henhal
Copy link
Contributor Author

henhal commented May 27, 2020

I thought some more about this, and while I await your feedback, I've realized that perhaps this tool should focus on parsing documents and not raw schemas. Also, it may be fine to not have components/schemas in your document if you split it into several files, but the dereferenced/bundled single document will be very verbose with no re-use of schemas at all.
I've come to think that it would suit me better to use some pre-processor such as yaml-import to build my document from separate parts and still have components/schemas.

However, I still think that swagger-to-ts should not enforce components being present since it is an optional property, and that it should instead also parse schemas from other parts of the document.
I added my thoughts in #236

@drwpow
Copy link
Contributor

drwpow commented Aug 8, 2020

Hey thanks for the patience on this. I’ve left Manifold and lost access to this repo, and recently they’ve agreed to add me back as a maintainer.

This PR does introduce different code paths, and I’m a bit hesitant to add a flag that works for one version but not the other. And with the merging of #258 it does make the output logic more complicated. Is there any reason you can’t accomplish the same thing outside the library? e.g.:

type Schemas = OpenAPIV3['components']['schemas']
type Thing1 = Schemas['Thing1'];

Open to pushback on this; right now with limited time I’m trying to keep the codebase as simple as possible.

@drwpow
Copy link
Contributor

drwpow commented Aug 8, 2020

Sorry—I started with the PRs first and then read both issues you commented in. I understand your use case a bit better, so just ignore my previous comment.

I think I’m in favor of this PR now. I wanted to highlight the central goal of this project which is what I think you’re hinting at, which is this project should convert Swagger to TypeScript 1:1 and not transform anything. I don’t want to enable any options where the generated types deviate from your schema, and that was a realization I reached in v1 of this project. Adopting the 1:1 philosophy for v2 fixed some big bugs, reduced a lot of code, and IMO is much simpler to use.

With that said, you do make a pretty good case for adding this, because Swagger allows it. If Swagger allows it, so should this project 🙂. Sorry for the trouble, but if you could fix the merge conflicts from #258 I’d be happy to merge.

@henhal
Copy link
Contributor Author

henhal commented Aug 17, 2020

Thanks @drwpow. I've fixed the merge conflicts, but I'm a bit short of time today so honestly I haven't had time to test this just yet. Ideally some unit tests should be added of course.

@SmuliS
Copy link

SmuliS commented Oct 1, 2020

I am also running into #263. Looks like this PR is approved, so is there something that is holding back the merge if this?

@drwpow
Copy link
Contributor

drwpow commented Oct 8, 2020

Hey @SmuliS, thanks for checking in. Originally I was asking the original author to add some tests, but you raise a good point—better to release something without tests than not release something at all. Will merge this (and will add tests if I have time).

@drwpow drwpow merged commit 99bcf0e into openapi-ts:master Oct 8, 2020
@drwpow
Copy link
Contributor

drwpow commented Oct 8, 2020

@all-contributors please add @henhal for code, docs

@allcontributors
Copy link
Contributor

@drwpow

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

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.

3 participants