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

[BUG]: Type ... is not assignable to type 'never' when calling requestReviewers #534

Closed
1 task done
jakebailey opened this issue Apr 28, 2023 · 8 comments
Closed
1 task done
Labels
Status: Blocked Blocked by GitHub's API or other external factors Type: Bug Something isn't working as documented, or is being fixed

Comments

@jakebailey
Copy link

What happened?

With just:

import { Octokit } from "@octokit/rest";

const gh = new Octokit();

gh.pulls.requestReviewers({
	owner: "owner",
	repo: "repo",
	pull_number: 1234,
	reviewers: ["foo", "bar", "baz"],
})

All properties of the options are flagged as errors.

You can see this at the moment in the TS playground (which does download the types): Playground Link

Versions

@octokit/types@9.1.4

Relevant log output

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@jakebailey jakebailey added Status: Triage This is being looked at and prioritized Type: Bug Something isn't working as documented, or is being fixed labels Apr 28, 2023
@jakebailey
Copy link
Author

Per #528 (comment) I may have been too quick at the draw, but, the just released v9.2.0 also has this problem.

@wolfy1339
Copy link
Member

This is a different problem than #528...

This is caused by GitHub itself. The OpenAPI spec isn't quite correct for many endpoints

Example endpoint:
PATCH /gists/{gist_id}

"gists/update": {
   [ ... ]
    requestBody: {
      content: {
        "application/json":
          | ({
              [ ... ]
              files?: {
                [key: string]:
                  | (
                      | ({
                          /** @description The new content of the file. */
                          content?: string;
                          /** @description The new filename for the file. */
                          filename?: string | null;
                        } & (
                          | Record<string, never>
                          | Record<string, never>
                          | Record<string, never>
                        ))
                      | null
                    )
                  | undefined;
              };
            } & (Record<string, never> | Record<string, never>))
          | null;
      };
    };
  };
"/gists/{gist_id}": {
[ ... ]
"patch": {
        "summary": "Update a gist",
        "description": "Allows you to update a gist's description and to update, delete, or rename gist files. Files from the previous version of the gist that aren't explicitly changed during an edit are unchanged.",
        "tags": [
          "gists"
        ],
        "operationId": "gists/update",
        "externalDocs": {
          "description": "API method documentation",
          "url": "https://docs.github.com/rest/reference/gists/#update-a-gist"
        },
        "parameters": [
          {
            "$ref": "#/components/parameters/gist-id"
          }
        ],
        "requestBody": {
          "required": true,
          "content": {
            "application/json": {
              "schema": {
                "properties": {
                   [ ... ]
                  "files": {
                    [ ... ]
                    "type": "object",
                    "additionalProperties": {
                      "type": "object",
                      "nullable": true,
                      "properties": {
                        "content": {
                          "description": "The new content of the file.",
                          "type": "string"
                        },
                        "filename": {
                          "description": "The new filename for the file.",
                          "type": "string",
                          "nullable": true
                        }
                      },
                      "anyOf": [
                        {
                          "required": [
                            "content"
                          ]
                        },
                        {
                          "required": [
                            "filename"
                          ]
                        },
                        {
                          "type": "object",
                          "maxProperties": 0
                        }
                      ]
                    }
                  }
                },
                "anyOf": [
                  {
                    "required": [
                      "description"
                    ]
                  },
                  {
                    "required": [
                      "files"
                    ]
                  }
                ],
                "type": "object",
                "nullable": true
              },
            [ ... ]
              }
            }
          }
        },
  [ ... ]
      },

@wolfy1339
Copy link
Member

I've narrowed it down to the following circumstance:

The spec is trying to specify that either reviewers or team_reviewers is present at a time, by trying to use anyOf and required.
That isn't how the JSON schema spec that OpenAPI is built on works. It will always result in undefined behaviour

Before openapi-typescript handled these quirks, but doesn't anymore.

@wolfy1339 wolfy1339 added Status: Blocked Blocked by GitHub's API or other external factors and removed Status: Triage This is being looked at and prioritized labels Apr 28, 2023
@wolfy1339
Copy link
Member

/cc @timrogers @nickfloyd
Can you point out this bug to the relevant team so it can get fixed

@kfcampbell
Copy link
Member

I've brought it up in Slack, though I can't speculate at a time frame for resolution.

@kfcampbell
Copy link
Member

@wolfy1339 now that your override has been merged, we should get automated dependency updates with the next lock file maintenance PRs, right?

@wolfy1339
Copy link
Member

Sometimes, that may be the case.

Most of the time updates to this package is handled by octokitbot

@wolfy1339
Copy link
Member

I'm closing out this issue as everything seems to be in working order now.

Please file a new issue if a problem arises

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Blocked Blocked by GitHub's API or other external factors Type: Bug Something isn't working as documented, or is being fixed
Projects
Archived in project
Development

No branches or pull requests

3 participants