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

Repository search result: owner might be null #338

Open
creativecreatorormaybenot opened this issue Jul 1, 2021 · 9 comments · Fixed by #353
Open

Repository search result: owner might be null #338

creativecreatorormaybenot opened this issue Jul 1, 2021 · 9 comments · Fixed by #353
Labels
Status: Blocked Blocked by GitHub's API or other external factors Type: Support Any questions, information, or general needs around the SDK or GitHub APIs typescript Relevant to TypeScript users only
Projects

Comments

@creativecreatorormaybenot

What happened?

With the upgrades:

  • Bump @octokit/types from 6.17.1 to 6.17.3
  • Bump @octokit/rest from 18.6.3 to 18.6.4

My code stopped working because repo.owner might now be null in:

type Repo = GetResponseDataTypeFromEndpointMethod<
  typeof octokit.search.repos
>['items'][0]

What did you expect to happen?

I expect Repo.owner to never be null because every repo must have an owner. What would a repo without an owner be?

What the problem might be

The actual type is now:

    owner: {
        name?: string | null | undefined;
        email?: string | null | undefined;
        login: string;
        id: number;
        node_id: string;
        avatar_url: string;
        ... 14 more ...;
        starred_at?: string | undefined;
    } | null;

Before it was not | null.

Related

In the same upgrade Repo.description also became nullable and I am not sure why that is. Is there some kind of changelog available that explains the reasons for these breaking changes? Thanks for all the work 🙏

@creativecreatorormaybenot creativecreatorormaybenot added the Type: Bug Something isn't working as documented, or is being fixed label Jul 1, 2021
@ghost ghost added this to Bugs in JS Jul 1, 2021
@gr2m gr2m added the typescript Relevant to TypeScript users only label Jul 2, 2021
@gr2m
Copy link
Contributor

gr2m commented Jul 2, 2021

I am not able to reproduce the problem, I tried in several ways, see my typescript playground

@gr2m gr2m added awaiting response Type: Support Any questions, information, or general needs around the SDK or GitHub APIs and removed Type: Bug Something isn't working as documented, or is being fixed labels Jul 2, 2021
@ghost ghost moved this from Bugs to Awaiting response in JS Jul 2, 2021
@creativecreatorormaybenot
Copy link
Author

@gr2m for me, it shows 3 errors:

image

Errors in code

Object is possibly 'null'.
Object is possibly 'null'.
Object is possibly 'null'.

Or what do you mean by you were not able to reproduce?

@gr2m
Copy link
Contributor

gr2m commented Jul 2, 2021

wow I haven't seen these errors yesterday, but do so today. It works when disabling the strictNullChecks option. But I do think it should work with strictNullChecks enabled.

image

I'll have another look

@gr2m
Copy link
Contributor

gr2m commented Jul 2, 2021

So I've found that the owner can indeed be null for a repository in the search repositories endpoint response

https://unpkg.com/browse/@octokit/openapi-types@8.1.4/types.d.ts#L10397

THat's because GitHub's official OpenAPI spec define's the owner property as nullable:
https://unpkg.com/browse/@octokit/openapi@2.23.1/generated/api.github.com.json#L53106

As to why that is, or whether that is a bug, the best place to bring that up is https://github.com/github/rest-api-description/. Could you file an issue there? You can ping me and reference this issue, but then I'd close this issue because there is nothing actionable for us to do, we have to resolve this at the source of the OpenAPI spec

@gr2m gr2m added Status: Blocked Blocked by GitHub's API or other external factors and removed awaiting response labels Jul 2, 2021
@ghost ghost moved this from Awaiting response to Blocked in JS Jul 2, 2021
@creativecreatorormaybenot
Copy link
Author

@gr2m To be honest, I am completely lost here. I tried to figure out what github/rest-api-description is and what kind of issue I should open, but I am only confused.

It seems there are like 5 different repos having some kind of type definitions and all of them are only auto-updated by some bot. It seems like none of them have any actual human work, making the changes:

  • octokit/types.ts
  • octokit/types
  • octokit/openapi
  • openapi-types.ts
  • github/rest-api-description
  • and to be honest, the list goes on.

All of them seem like they could be relevant but I was not able to find the relevant change anywhere. I mean it must have happened in the last few days that owner was made nullable since it was non-nullable before (I know that because I can track it in my CI builds + dependabot).

Can you help me figure out where the change happened and what kind of issue I should open them? I did not feel like I have enough information to open an issue at github/rest-api-description...

@gr2m
Copy link
Contributor

gr2m commented Jul 3, 2021

blocked by github/rest-api-description#440

@gr2m
Copy link
Contributor

gr2m commented Jul 12, 2021

Does not appear possible to be null on both repository and minimal-repository. Fix is incoming. It appears to have been this way since the launch of this description however. Thanks folks.

github/rest-api-description#440 (comment)

@creativecreatorormaybenot
Copy link
Author

@gr2m repo.owner is still nullable for me:

/** Repo Search Result Item */
    "repo-search-result-item": {
      id: number;
      node_id: string;
      name: string;
      full_name: string;
      owner: components["schemas"]["nullable-simple-user"];

Should it not be simple-user?

@gr2m
Copy link
Contributor

gr2m commented Sep 14, 2021

I don't know, maybe there is a case when owner can be null, e.g. when an account was abusive and was deleted? I asked in the repository of the official OpenAPI spec: github/rest-api-description#549

@gr2m gr2m reopened this Sep 14, 2021
@ghost ghost moved this from Done to Blocked in JS Sep 14, 2021
@gr2m gr2m changed the title owner might be null Repository search result: owner might be null Sep 14, 2021
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: Support Any questions, information, or general needs around the SDK or GitHub APIs typescript Relevant to TypeScript users only
Projects
No open projects
JS
  
Blocked
2 participants