Skip to content

fix: remove all potentially undefined parameter types#555

Merged
gr2m merged 1 commit intomainfrom
fix-never-parameters
Jun 10, 2023
Merged

fix: remove all potentially undefined parameter types#555
gr2m merged 1 commit intomainfrom
fix-never-parameters

Conversation

@wolfy1339
Copy link
Copy Markdown
Member

This doesn't affect the parameters themselves

Resolves #554


Behavior

Before the change?

  • Since the parameter types could be potentially undefined, the type could return never

After the change?

  • The type now removes the potentially undefined type and it no longer returns never

Other information


Additional info

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Added the appropriate label for the given change

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

  • Yes (Please add the Type: Breaking change label)
  • No

If Yes, what's the impact:

  • N/A

Pull request type

Please add the corresponding label for change this PR introduces:

  • Bugfix: Type: Bug

This doesn't affect the parameters themselves
@wolfy1339 wolfy1339 added the Type: Bug Something isn't working as documented, or is being fixed label Jun 9, 2023
@wolfy1339 wolfy1339 marked this pull request as ready for review June 9, 2023 20:38
@wolfy1339
Copy link
Copy Markdown
Member Author

Here is an example from @octokit/openapi-types.
Because all the query parameters are optional, the query object is also optional.

"repos/list-for-org": {
    parameters: {
      query?: {
        /** @description Specifies the types of repositories you want returned. */
        type?: "all" | "public" | "private" | "forks" | "sources" | "member";
        /** @description The property to sort the results by. */
        sort?: "created" | "updated" | "pushed" | "full_name";
        /** @description The order to sort by. Default: `asc` when using `full_name`, otherwise `desc`. */
        direction?: "asc" | "desc";
        per_page?: components["parameters"]["per-page"];
        page?: components["parameters"]["page"];
      };
      path: {
        org: components["parameters"]["org"];
      };
    };

With the change, the type will simply remove the potential of it being undefined, and the parameters are returned like they should.

This doesn't affect the individual parameters. If they are set as optional, they are still returned as optional

Copy link
Copy Markdown
Contributor

@gr2m gr2m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work figuring that out 👏🏼

@gr2m gr2m merged commit d7ad4ba into main Jun 10, 2023
@gr2m gr2m deleted the fix-never-parameters branch June 10, 2023 20:10
@octokitbot
Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 9.3.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@octokitbot
Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 10.0.0-beta.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Labels

released on @beta released Type: Bug Something isn't working as documented, or is being fixed

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

[BUG]: ExtractParameters<T> can return never

3 participants