tsc fails when using octokit

[neenjaw]

Please avoid duplicates

• I checked all open bugs and none of them matched my problem.

Reproducible test case

https://www.typescriptlang.org/play?target=6&jsx=0&module=1&strict=true#code/JYWwDg9gTgLgBAbzgeQMYwga2PAvnAMyghDgCIJ0scyAoWgUwA9JY4BDAZwE8A7VQgFd+MYBF6EGMVAAsAFAEpEtOHFTjO8ACrIA0gFEAcnAC8cAOTN24ADYM4GTA17mVajfEqOcpuLwYA7ihU2DBySOyCMDIAXHA6Bsa4Cm7qvJqIcAAm7DDscPhm7AHsPl7UMAB0AOZQ7GAAjjZybqoABq2qcA2CDFDccGCCNjYASgw9DJqccgAkEAH+UHEAyjBQwLzVAIQANHCzUAyQq+ubO-uzvIIgcQCSvPBmAKwADJc27ABGDDaccQBtNYbLbbAC6SgQnS6cCOkE4OGg3DkCyWcXmiz6+141gY6LhEEh0Jhg2GYwmvWmck+Pz+6Jpv04+wIwCgmnR1xA+00uSmcWQAAUjESSaK-BAslNlGKxZyflBiTLRDA7IqxYIoDY1TDcNqCmrdaLDV02rtOlCZai+nFzP5nAArYrmM0ygk2u28R0BVw6twpNxHGAaiQ5PK0XBAA

Please select the environment(s) that are relevant to your bug report

• TypeScript
• Enterprise
• Browsers
• Node
• Deno

Version

├── [email protected]

What happened?

When compiling with tsc v4.3.2, I receive a type error which I don't see with earlier versions (e.g. 4.1.3), nor do I see it on the TS playground example. The error:

src/lib/gh.ts:33:21 - error TS2339: Property 'graphql' does not exist on type 'never'.

33   } = await octokit.graphql(
                       ~~~~~~~

I am able to resolve this by using the @octokit/graphql and using the graphql api directly, e.g.:

import { graphql } from '@octokit/graphql'

// ...

const graphqlWithAuth = graphql.defaults({
    headers: {
      authorization: process.env.GITHUB_TOKEN,
    },
  })

const { data } = await graphqlWithAuth(
  `
    query here
  `
)

Not sure if the wrong type is being exported?

Would you be interested in contributing a fix?

• yes

[gr2m]

It works in VS Code. It's most likely a problem with the playground, can you check in with them?

[neenjaw]

My issue is not that the auto complete doesn't work. My issue is that I get a tsc error that the type doesn't exist when I am compiling it on the command line.

[gr2m]

can you please create a minimal test repository to reproduce the problem? It is probably related to your tsconfig settings or and/or typescript version

[SleeplessByte]

The typescript playground is actually telling us why it's failing inside the playground.

.

@neenjaw can you check in your VSCode if you see the same issue/similar issue? Ping me on slack again if you want to work on a repro for Gregor.

[gr2m]

The typescript playground is actually telling us why it's failing inside the playground.

that's great, but it's still a problem with the playground that I cannot reproduce locally.

[neenjaw]

Yea, I can make one tonight, will post here with it

[neenjaw]

Minimal test repo here: https://github.com/neenjaw/octocat-minimal-issue

can recreate tsc error locally using npm install && npm run build

[gr2m]

It wasn't a problem with Octokit, but with the structure of your code. This PR fixes it:
neenjaw/octocat-minimal-issue#1

Does that work for you?

[neenjaw]

It would, can you explain how the previous way caused the tsc error so I can learn from it? I would have thought the if statement after the ternary would have narrowed the type appropriately, or at least excluded the never case?

[gr2m]

I wish I could explain it in detail, but I still don't fully understand the inner working of TypeScript myself. And I think there were some changes in type narrowing as part of v4.3.

The TypeScript community is very helpful, and there are some very skilled folks looking for TypeScript questions on StackOverflow, if you'd like to ask someone more skilled than myself.

I'd also recommend reading https://www.typescriptlang.org/docs/handbook/2/narrowing.html if you haven't yet.

[SleeplessByte]

@gr2m I checked your PR on @neenjaw repro and yes it solves his issue, but I did find the actually issue with the type.

The type of Octokit is, regardless of what Tim does in his repro:

Octokit 
  & void 
  & { paginate: PaginateInterface } 
  & Api

This results in your PR working because octokit.graphql reaches into Octokit as can be seen here:

So far, so good, right?

The problem is that void that's there. This makes it so that you can't narrow this value when doing a truthy (or falsy) check, which is what Tim is running into. Let me demonstrate:

const octokit = new Octokit({ auth: process.env.GITHUB_TOKEN! })

if (octokit) {
  octokit.graphql // nope
}

This is understandable behaviour. Something that is void can never be truthy. We can further proof that this is what happens by inspecting the else case:

if (octokit) {
} else {
  octokit.graphql // yep!
}

The source octokit.d.ts has this faulty type:

export declare const Octokit: typeof OctokitCore & import("@octokit/core/dist-types/types").Constructor<void & {
    paginate: import("@octokit/plugin-paginate-rest").PaginateInterface;
} & import("@octokit/plugin-rest-endpoint-methods/dist-types/types").Api>;

@neenjaw the reason this worked with earlier versions is that void didn't trump other types in the union, and now it does. As far as I can tell this has been the change that broke the type, but the change is correct. It shouldn't be void & { shape } but just be { shape }.

[gr2m]

can you please open a separate issue for this @SleeplessByte?