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

fix: remove all potentially undefined parameter types #555

Merged
merged 1 commit into from
Jun 10, 2023

Conversation

wolfy1339
Copy link
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
@ghost ghost added this to Bugs in JS Jun 9, 2023
@wolfy1339 wolfy1339 marked this pull request as ready for review June 9, 2023 20:38
@wolfy1339
Copy link
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
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
7 checks passed
@gr2m gr2m deleted the fix-never-parameters branch June 10, 2023 20:10
@octokitbot
Copy link
Collaborator

🎉 This PR is included in version 9.3.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@octokitbot
Copy link
Collaborator

🎉 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
JS
  
Bugs
Development

Successfully merging this pull request may close these issues.

[BUG]: ExtractParameters<T> can return never
3 participants