Skip to content

Add paths support #236

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

Closed
drwpow opened this issue May 5, 2020 · 4 comments
Closed

Add paths support #236

drwpow opened this issue May 5, 2020 · 4 comments
Labels
enhancement New feature or request

Comments

@drwpow
Copy link
Contributor

drwpow commented May 5, 2020

Suggested in #112:

any plans for components.parameters?

Looking into it, parameters is part of components.paths, which is the same as Swagger 2.0’s paths. So adding support for v3 should also include v2. That said… what should the schema look like for this? I propose something like :

interface components {
  paths: {
    "/v1/checkout/sessions": {
      get: {
        parameters: {
          ending_before?: string;
          expand?: string[];
        }
      }
    }
  }
}

Aliasing that would look like;

type parameters = components["paths"]["/v1/checkout/sessions"]["get"]["parameters"];

This ticket is a proposal for implementation. This looks the most straightforward to me, but are there any other considerations when it comes to paths? Is this usage intuitive and does this work for most people?

I’d love to hear stories from using paths to confirm if this would help or if any additional info would be helpful.

@drwpow drwpow added the enhancement New feature or request label May 5, 2020
@henhal
Copy link
Contributor

henhal commented May 27, 2020

I would really appreciate support for paths, however, putting them under components seems odd since that's not where they originate from in the document?

Also, parameters can be header, query, path or cookie. It doesn't make sense to create one parameters type with all of them if the purpose is to be able to process headers in a request handler etc?

Let's consider a typical OpenAPI3 document:

  components:
    parameters:
      Foo:
        name: x-foo
        in: header
        required: true
        schema:
          type: string
    schemas:
      Person:
        type: object
        additionalProperties: false
        required: [id, name]
        properties:
          id:
            type: string
          name:
            type: string
          age:
            type: number
      Error:
        type: object
        additionalProperties: false
        required: [message]
        properties:
          message:
            type: string
  paths:
    /person:
      post:
        operationId: addPerson
        parameters:
          - $ref: '#/components/parameters/Foo'
          - name: x-bar
            in: header
            schema:
              type: string
          - name: baz
            in: query
            schema:
              type: boolean
        requestBody:
          content:
            application/json:
              schema:
                type: object
                additionalProperties: false
                required: [name]
                properties:
                  name:
                    type: string
                  age:
                    type: number
        responses:
          200:
            headers:
              x-baz:
                schema:
                  type: string
            content:
              application/json:
                schema:
                  $ref: '#/components/schemas/Person'
          400:
            content:
              application/json:
                schema:
                  $ref: '#/components/schemas/Error'

It would be very nice if swagger-to-ts could parse not only components/schemas (which BTW is optional, see #262) but also the schemas found in paths, parameters etc. However, for this to be useful, I think a simple mapping from the OpenAPI format will not work. For example, parameters are in the form of arrays with an in: header property, which don't map very well to an interface if you want to create types for the headers of an operation. Also, mapping operations rather than paths/methods would probably be more useful.

If the purpose is for this to be useful when you're writing your application with the request handlers, I think the following mapping would be great:

export interface components {
    schemas: {
        Person: {
            id: string;
            name: string;
            age?: number;
        },
        Error: {
            message: string;
        }
    },
    parameters: {
        Foo: {
            header: {
                'x-foo': string;
            }
        }
    }
}

export interface operations {
    addPerson: {
        headers: components['parameters']['Foo']['header'] & {
            'x-bar'?: string;
        },
        query: {
            baz?: boolean;
        },
        body: {
            json: {
                name: string;
                age?: string;
            }
        },
        responses: {
            '200': {
                headers: {
                    'x-baz': string;
                },
                body: {
                    json: components['schemas']['Person']
                }
            },
            '400': {
                body: {
                    json: components['schemas']['Error']
                }
            }
        }
    }
}

Here the paths are parsed to create an operations interface which resolves both inline schemas and references to components. Headers, queries, bodies and responses are all typed.

Then a handler for the addPerson operation, be it implemented in express, AWS lambda etc, could refer to operation types, e.g.

function addPerson(req: Request, res: Response) {
  const body = req.body as operations['addPerson']['body']['json'];

  const result: operations['addPerson']['responses']['200']['body']['json'] = ...

  res.status(200).send(result);
}

@nandorojo
Copy link

Wow, I was just about to create this issue. This would be an awesome addition!

I'm also not sure if it makes sense for paths to go within components, since this doesn't match the swagger json outline. It probably makes most sense for it to be a separate interface.

That said, if it adds a ton of trouble to have it as a separate type, I'm fine with it going in components.

@gr2m
Copy link
Contributor

gr2m commented Sep 29, 2020

I'm currently looking into a tool to transform GitHub's official OpenAPI Spec into TypeScript definitions, for the @octokit libraries that I generate based on that API. The paths are the central part of the types, that's how I share/access them across multiple libraries. Here are the types that I currently generate with my self-cooked scripts that only partially use the scemas, and do not take advantage of the references, because of legacy reasons: https://github.com/octokit/types.ts/blob/acf3cdaa9e8ae33b924b5b18cd572379c050994b/src/generated/Endpoints.ts

The central part of the types looks like this (simplified):

export interface Endpoints {
  /**
   * @see https://developer.github.com/v3/repos/#create-an-organization-repository
   */
  "POST /orgs/:org/repos": {
    parameters: ReposCreateInOrgParameters;
    response: ReposCreateInOrgResponse;
  };
  // ...
}

I'm evaluating if I can use @manifoldco/swagger-to-ts to generate these types, or at least generate an interim types module that I can import into @octokit/types for the final mapping specific to the `@octokit/* libraries.

I'm not familiar with all the differences between v2 and v3, but based on GitHub's OpenAPI spec

simplified excerpt of YAML OpenAPI
openapi: 3.0.3
paths:
  "/orgs/{org}/repos":
    post:
      operationId: repos/create-in-org
      parameters:
        - "$ref": "#/components/parameters/org"
      requestBody:
        content:
          application/json:
            schema:
              type: object
              properties:
                name:
                  type: string
                description:
                  type: string
                homepage:
                  type: string
                private:
                  type: boolean
                  default: false
              required:
                - name
      responses:
        "201":
          content:
            application/json:
              schema:
                "$ref": "#/components/schemas/repository"
        "403":
          "$ref": "#/components/responses/forbidden"
        "422":
          "$ref": "#/components/responses/validation_failed"

components:
  parameters:
    org:
      "$ref": "#/components/schemas/actor"

  responses:
    forbidden:
      content:
        application/json:
          schema:
            "$ref": "#/components/schemas/basic-error"

    validation_failed:
      content:
        application/json:
          schema:
            "$ref": "#/components/schemas/validation-error"

  schemas:
    actor:
      type: object
      properties:
        id:
          type: integer
        login:
          type: string
      required:
        - id
        - login

    repository:
      type: object
      properties:
        id:
          type: integer
        name:
          type: string
        owner:
          nullable: true
          allOf:
            - "$ref": "#/components/schemas/simple-user"
        private:
          default: false
          type: boolean
        description:
          type: string
          nullable: true
        fork:
          type: boolean

    simple-user:
      type: object
      properties:
        login:
          type: string
        id:
          type: integer
        type:
          type: string
      required:
        - id
        - login
        - type
      nullable: true

    basic-error:
      type: object
      properties:
        message:
          type: string
        documentation_url:
          type: string

    validation-error:
      type: object
      required:
        - message
        - documentation_url
      properties:
        message:
          type: string
        documentation_url:
          type: string
        errors:
          type: array
          items:
            type: object
            required:
              - code
            properties:
              message:
                type: string
              code:
                type: string

I could imagine the output to be something like this

TypeScript
export interface Paths {
  "/app/installations/:installation_id": {
    post: Operations["ReposCreateInOrg"];
  };
}

export interface Operations {
  ReposCreateInOrg: {
    parameters: {
      org: Components["Parameters"]["Org"];
    };
    requestBody: {
      content: {
        "application/json": {
          name: string;
          description?: string;
          homepage?: string;
          private?: boolean;
        };
      };
    };
    responses: {
      "201": {
        content: {
          "application/json": Components["Schemas"]["Repository"];
        };
      };
      "403": Components["Responses"]["Forbidden"];
      "422": Components["Responses"]["ValidationFailed"];
    };
  };
}

export interface Components {
  Parameters: {
    Org: Components["Schemas"]["Actor"];
  };

  Schemas: {
    Actor: {
      id: number;
      type: string;
    };

    Repository: {
      id: number;
      name: string;
      owner: Components["Schemas"]["SimpleUser"] | null;
      private: boolean;
      description: string | null;
      fork: boolean;
    };

    BasicError: {
      message: string;
      type: string;
    };

    ValidationErrer: {
      message: string;
      documentation_url: string;
      errors?: {
        code: string;
        message?: string;
      }[];
    };
  };

  Responses: {
    Forbidden: {
      content: {
        "application/json": Components["Schemas"]["BasicError"];
      };
    };
    ValidationFailed: {
      content: {
        "application/json": Components["Schemas"]["ValidationErrer"];
      };
    };
  };
}

I've shortly discussed with @drwpow, I'd be happy to help to make this work if it's within the scope of this library. I'll have to do the work anyway

@drwpow
Copy link
Contributor Author

drwpow commented Sep 30, 2020

@gr2m I can help out next week based on how far you get.

But if it helps, I’d start with the tests: fill out the expected TS result (there are spec tests as well as entire schema tests), then add to the v2 & v3 codebases to generate that.

drwpow added a commit that referenced this issue Oct 8, 2020
drwpow added a commit that referenced this issue Oct 8, 2020
drwpow added a commit that referenced this issue Oct 8, 2020
drwpow added a commit that referenced this issue Oct 8, 2020
drwpow added a commit that referenced this issue Oct 8, 2020
drwpow added a commit that referenced this issue Oct 8, 2020
@drwpow drwpow closed this as completed Oct 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants