grantila / typeconv

Convert between JSON Schema, TypeScript, GraphQL, Open API and SureType
MIT License
421 stars 8 forks source link

TS to OpenAPI does not produce descriptions from the comments #13

Closed rattrayalex closed 2 years ago

rattrayalex commented 3 years ago

If I have a type like this:

type Point = {
  /** The distance from the left in mm */
  x: number;
  /** The distance from the top in mm */
  y: number;
};

I would hope for an OpenAPI spec like this:

components:
  schemas:
    Point:
      type: object
      properties:
        x:
          type: number
          description: The distance from the left in mm
        y:
          type: number
          description: The distance from the top in mm

The primary reason I would want to convert my TS types to OpenAPI specs is so people can look at documentation for the API param/response types in a web browser.

Can this be done?

grantila commented 3 years ago

Not sure if this has been a bug, but in the latest version this does work, however these will be saved into title rather than description.

I made a unit test out of this and god the following result:

    components:
      schemas:
        Point:
          properties:
            x:
              title: The distance from the left in mm
              type: number
            'y':
              title: The distance from the top in mm
              type: number
          required:
            - x
            - 'y'
          additionalProperties: false
          type: object

Let me know if this is an issue.

rattrayalex commented 3 years ago

Yeah, sorry, title is a different thing - it really should be description. Title if anything should be the type name of the top level item, like:


/** This is useful for such and such */
type Foo = { a: number }

Becomes

title: Foo
description: This is useful for such and such
type: object
properties:
  a: 
    type: number

You can take a look at how stripe-node's TS types are printed from their openapi spec for a real world example

grantila commented 2 years ago

OpenAPI forwards the title field straight from JSON Schema, which is a bit unclear about this: http://json-schema.org/draft/2020-12/json-schema-validation.html#rfc.section.9.1 but the "A title will preferably be short" means you're probably right.

I'll try to fix this throughout the core-types packages (it affects quite a few packages), it does make sense, thanks for enlightening me!

rattrayalex commented 2 years ago

Awesome, thanks so much!

github-actions[bot] commented 2 years ago

:tada: This issue has been resolved in version 1.7.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

rattrayalex commented 2 years ago

Amazing!! That was quick.

FWIW I would actually recommend against adding titles like Point.x, which I see in the spec, but the addition of the title for things like Point is definitely awesome. I don't think it can really hurt per se to add Point.x, so I don't think you should feel obligated to remove it (though now would be the easiest time, before anyone starts relying on it) but it definitely seems nonstandard/redundant to my OpenAPI eyes.

grantila commented 2 years ago

Great feedback, I'll look into configuring it by command line arguments. Defaulting to only top-level types probably makes sense.

jguerinet commented 2 years ago

Hi @grantila, did you ever configure the removal of the title via command line arguments by any chance? I find it fairly redundant as well and would like to remove it but I don't seem to see an option to (or maybe I'm just not looking in the right spot).