Skip to content
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

Endpoints['POST /repos/{owner}/{repo}/check-runs']['parameters'] gives broken types in version newer than v6.3.2 #274

Closed
tlbdk opened this issue Feb 27, 2021 · 9 comments · Fixed by octokit/openapi#55 or #278
Assignees
Labels
Type: Bug Something isn't working as documented, or is being fixed

Comments

@tlbdk
Copy link

tlbdk commented Feb 27, 2021

How to recreate the issue

Create a test project:

npm init -y && npm install typescript @octokit/types

Generates this package.json file:

{
  "name": "test-repo",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "keywords": [],
  "author": "",
  "license": "ISC",
  "dependencies": {
    "@octokit/types": "^6.11.0",
    "typescript": "^4.2.2"
  }
}

Create test.ts:

import { Endpoints } from '@octokit/types'

type listUserReposParameters = Endpoints['POST /repos/{owner}/{repo}/check-runs']['parameters']

const test: listUserReposParameters = {
    name: '',
    head_sha: '',
    owner: '',
    repo: '',
    status: 'completed'
}
tsc test.ts

Gives the following error and the type that it constructed seems very broken:

test.ts:8:5 - error TS2322: Type 'string' is not assignable to type 'never'.

8     status: 'completed'
      ~~~~~~

  node_modules/@octokit/openapi-types/dist-types/generated/types.d.ts:19740:21
    19740                     status?: "completed";
                              ~~~~~~
    The expected type comes from property 'status' which is declared here on type '{ owner: string; repo: string; } & Partial<{ status?: "completed"; } & { [key: string]: any; }> & Partial<{ status?: "queued" | "in_progress"; } & { [key: string]: any; }>'


Found 1 error.

Trying with v6.3.2 works:

npm install @octokit/[email protected]
tsc test.ts

Diff is here:
v6.3.2...v6.4.0

@tlbdk tlbdk added the Type: Bug Something isn't working as documented, or is being fixed label Feb 27, 2021
@tedspare
Copy link

I'm seeing a (possibly?) similar error from the generated Endpoints.d.ts of @octokit/types^6.11.0:

ERROR in /.../node_modules/@octokit/types/dist-types/generated/Endpoints.d.ts(58,292):
58:292 Type '"responses"' cannot be used to index type 'R'.
    56 |     [K in KnownJsonResponseTypes & keyof T]: T[K];
    57 | }[KnownJsonResponseTypes & keyof T];
    58 | declare type ExtractOctokitResponse<R> = "responses" extends keyof R ? SuccessResponseDataType<R["responses"]> extends never ? RedirectResponseDataType<R["responses"]> extends never ? EmptyResponseDataType<R["responses"]> : RedirectResponseDataType<R["responses"]> : SuccessResponseDataType<R["responses"]> : unknown;

This occurs when serving locally and when compiling. Runtime is unaffected (as expected for a TS error) but the errors break our CI/CD. We're also using @octokit/core^3.1.2 and have tried @octokit/types^5.4.0.

Thanks for bearing with me if this is a misplaced comment - I'm new to commenting on others' repos! 😄

@gr2m
Copy link
Contributor

gr2m commented Feb 28, 2021

There have been some recent OpenAPI updates to the checks APIs, I recall the changes included some .anyOf/.oneOf conditionals. My guess is that our transpilation in https://github.com/octokit/openapi-types.ts is not working well. I'm looking into it

@gr2m gr2m self-assigned this Mar 1, 2021
@gr2m
Copy link
Contributor

gr2m commented Mar 1, 2021

Just to keep track of my digging into this. The problem is the operations["checks/create"] type from @octokit/openapi-types

  "checks/create": {
    parameters: {
      path: {
        owner: components["parameters"]["owner"];
        repo: components["parameters"]["repo"];
      };
    };
    responses: {
      /** response */
      201: {
        content: {
          "application/json": components["schemas"]["check-run"];
        };
      };
    };
    requestBody: {
      content: {
        "application/json": Partial<
          {
            status?: "completed";
          } & { [key: string]: any }
        > &
          Partial<
            {
              status?: "queued" | "in_progress";
            } & { [key: string]: any }
          >;
      };
    };
  };

The source from GitHub's OpenAPI spec is this

{
  "requestBody": {
    "content": {
      "application/json": {
        "schema": {
          "type": "object",
          "properties": {
            "name": {
              "type": "string",
              "description": "The name of the check. For example, \"code-coverage\"."
            },
            "head_sha": {
              "type": "string",
              "description": "The SHA of the commit."
            },
            "details_url": {
              "type": "string",
              "description": "The URL of the integrator's site that has the full details of the check. If the integrator does not provide this, then the homepage of the GitHub app is used."
            },
            "external_id": {
              "type": "string",
              "description": "A reference for the run on the integrator's system."
            },
            "status": {
              "type": "string",
              "description": "The current status. Can be one of `queued`, `in_progress`, or `completed`.",
              "enum": ["queued", "in_progress", "completed"],
              "default": "queued"
            },
            "started_at": {
              "type": "string",
              "description": "The time that the check run began. This is a timestamp in [ISO 8601](https://en.wikipedia.org/wiki/ISO_8601) format: `YYYY-MM-DDTHH:MM:SSZ`."
            },
            "conclusion": {
              "type": "string",
              "description": "**Required if you provide `completed_at` or a `status` of `completed`**. The final conclusion of the check. Can be one of `action_required`, `cancelled`, `failure`, `neutral`, `success`, `skipped`, `stale`, or `timed_out`. When the conclusion is `action_required`, additional details should be provided on the site specified by `details_url`.  \n**Note:** Providing `conclusion` will automatically set the `status` parameter to `completed`. You cannot change a check run conclusion to `stale`, only GitHub can set this.",
              "enum": [
                "action_required",
                "cancelled",
                "failure",
                "neutral",
                "success",
                "skipped",
                "stale",
                "timed_out"
              ]
            },
            "completed_at": {
              "type": "string",
              "description": "The time the check completed. This is a timestamp in [ISO 8601](https://en.wikipedia.org/wiki/ISO_8601) format: `YYYY-MM-DDTHH:MM:SSZ`."
            },
            "output": {
              "type": "object",
              "description": "Check runs can accept a variety of data in the `output` object, including a `title` and `summary` and can optionally provide descriptive details about the run. See the [`output` object](https://docs.github.com/rest/reference/checks#output-object) description.",
              "properties": {
                "title": {
                  "type": "string",
                  "description": "The title of the check run."
                },
                "summary": {
                  "type": "string",
                  "maxLength": 65535,
                  "description": "The summary of the check run. This parameter supports Markdown."
                },
                "text": {
                  "type": "string",
                  "maxLength": 65535,
                  "description": "The details of the check run. This parameter supports Markdown."
                },
                "annotations": {
                  "type": "array",
                  "description": "Adds information from your analysis to specific lines of code. Annotations are visible on GitHub in the **Checks** and **Files changed** tab of the pull request. The Checks API limits the number of annotations to a maximum of 50 per API request. To create more than 50 annotations, you have to make multiple requests to the [Update a check run](https://docs.github.com/rest/reference/checks#update-a-check-run) endpoint. Each time you update the check run, annotations are appended to the list of annotations that already exist for the check run. For details about how you can view annotations on GitHub, see \"[About status checks](https://help.github.com/articles/about-status-checks#checks)\". See the [`annotations` object](https://docs.github.com/rest/reference/checks#annotations-object) description for details about how to use this parameter.",
                  "maxItems": 50,
                  "items": {
                    "type": "object",
                    "properties": {
                      "path": {
                        "type": "string",
                        "description": "The path of the file to add an annotation to. For example, `assets/css/main.css`."
                      },
                      "start_line": {
                        "type": "integer",
                        "description": "The start line of the annotation."
                      },
                      "end_line": {
                        "type": "integer",
                        "description": "The end line of the annotation."
                      },
                      "start_column": {
                        "type": "integer",
                        "description": "The start column of the annotation. Annotations only support `start_column` and `end_column` on the same line. Omit this parameter if `start_line` and `end_line` have different values."
                      },
                      "end_column": {
                        "type": "integer",
                        "description": "The end column of the annotation. Annotations only support `start_column` and `end_column` on the same line. Omit this parameter if `start_line` and `end_line` have different values."
                      },
                      "annotation_level": {
                        "type": "string",
                        "description": "The level of the annotation. Can be one of `notice`, `warning`, or `failure`.",
                        "enum": ["notice", "warning", "failure"]
                      },
                      "message": {
                        "type": "string",
                        "description": "A short description of the feedback for these lines of code. The maximum size is 64 KB."
                      },
                      "title": {
                        "type": "string",
                        "description": "The title that represents the annotation. The maximum size is 255 characters."
                      },
                      "raw_details": {
                        "type": "string",
                        "description": "Details about this annotation. The maximum size is 64 KB."
                      }
                    },
                    "required": [
                      "path",
                      "start_line",
                      "end_line",
                      "annotation_level",
                      "message"
                    ]
                  }
                },
                "images": {
                  "type": "array",
                  "description": "Adds images to the output displayed in the GitHub pull request UI. See the [`images` object](https://docs.github.com/rest/reference/checks#images-object) description for details.",
                  "items": {
                    "type": "object",
                    "properties": {
                      "alt": {
                        "type": "string",
                        "description": "The alternative text for the image."
                      },
                      "image_url": {
                        "type": "string",
                        "description": "The full URL of the image."
                      },
                      "caption": {
                        "type": "string",
                        "description": "A short image description."
                      }
                    },
                    "required": ["alt", "image_url"]
                  }
                }
              },
              "required": ["title", "summary"]
            },
            "actions": {
              "type": "array",
              "description": "Displays a button on GitHub that can be clicked to alert your app to do additional tasks. For example, a code linting app can display a button that automatically fixes detected errors. The button created in this object is displayed after the check run completes. When a user clicks the button, GitHub sends the [`check_run.requested_action` webhook](https://docs.github.com/webhooks/event-payloads/#check_run) to your app. Each action includes a `label`, `identifier` and `description`. A maximum of three actions are accepted. See the [`actions` object](https://docs.github.com/rest/reference/checks#actions-object) description. To learn more about check runs and requested actions, see \"[Check runs and requested actions](https://docs.github.com/rest/reference/checks#check-runs-and-requested-actions).\" To learn more about check runs and requested actions, see \"[Check runs and requested actions](https://docs.github.com/rest/reference/checks#check-runs-and-requested-actions).\"",
              "maxItems": 3,
              "items": {
                "type": "object",
                "properties": {
                  "label": {
                    "type": "string",
                    "maxLength": 20,
                    "description": "The text to be displayed on a button in the web UI. The maximum size is 20 characters."
                  },
                  "description": {
                    "type": "string",
                    "maxLength": 40,
                    "description": "A short explanation of what this action would do. The maximum size is 40 characters."
                  },
                  "identifier": {
                    "type": "string",
                    "maxLength": 20,
                    "description": "A reference for the action on the integrator's system. The maximum size is 20 characters."
                  }
                },
                "required": ["label", "description", "identifier"]
              }
            }
          },
          "required": ["name", "head_sha"],
          "anyOf": [
            {
              "properties": { "status": { "enum": ["completed"] } },
              "required": ["conclusion"],
              "additionalProperties": true
            },
            {
              "properties": {
                "status": { "enum": ["queued", "in_progress"] }
              },
              "additionalProperties": true
            }
          ]
        }
      }
    }
  }
}

I'm not sure if the anyOf as child of schema is even allowed (together with properties on the same level). I'll continue to dig into this

@gr2m
Copy link
Contributor

gr2m commented Mar 1, 2021

I've pinged the team who is working on the OpenAPI spec. Let's see what they say: github/rest-api-description#220

@gr2m
Copy link
Contributor

gr2m commented Mar 2, 2021

Looks like the schema is correct, we will have to update https://github.com/drwpow/openapi-typescript. I'll file an issue / start a PR with a failing test

@gr2m
Copy link
Contributor

gr2m commented Mar 2, 2021

The fix is pending: openapi-ts/openapi-typescript#491. Shouldn't be long now to get this resolved

gr2m added a commit to octokit/openapi-types.ts that referenced this issue Mar 2, 2021
gr2m added a commit to octokit/openapi-types.ts that referenced this issue Mar 2, 2021
* build(deps): update `octokit-openapi` to [v3.0.3](https://github.com/drwpow/openapi-typescript/releases/tag/v3.0.3)

* fix: check run and gist types for body parameters

addresses octokit/types.ts#274
@gr2m
Copy link
Contributor

gr2m commented Mar 2, 2021

This was partly fixed via #277. But status is still typed to never. The problem now is a problem with GitHub's OpenAPI spec though: github/rest-api-description#220

@gr2m
Copy link
Contributor

gr2m commented Mar 4, 2021

not resolved yet, but the fix is on its way to @octokit/types

@gr2m
Copy link
Contributor

gr2m commented Mar 4, 2021

The types are not perfect (the conditional restrictions are not transpiled to TypeScript) but at least you no longer get the event type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment