[BUG]: the type ExtractRequestBody returns never

[wolfy1339]

What happened?

While updating @octoit/plugin-rest-endpoint-methods to use the latest release of this package, in octokit/plugin-rest-endpoint-methods.js#632, the tests are erroring out.

I managed to narrow it down to the ExtractRequestBody type, as can be seen in the code snippet below

types.ts/src/generated/Endpoints.ts

Lines 21 to 37 in e795680

	type ExtractRequestBody<T> = "requestBody" extends keyof T
	? "content" extends keyof T["requestBody"]
	? "application/json" extends keyof T["requestBody"]["content"]
	? T["requestBody"]["content"]["application/json"]
	: {
	data: {
	[K in keyof T["requestBody"]["content"]]: T["requestBody"]["content"][K];
	}[keyof T["requestBody"]["content"]];
	}
	: "application/json" extends keyof T["requestBody"]
	? T["requestBody"]["application/json"]
	: {
	data: {
	[K in keyof T["requestBody"]]: T["requestBody"][K];
	}[keyof T["requestBody"]];
	}
	: {};

Versions

@octokit/types v9.1.2
typescript v5.0.4

Relevant log output

The following error is for the PATCH /repos/{owner}/{repo}/labels/{name} endpoint. Every error is similar to it

Argument of type '{ owner: string; repo: string; name: string; color: string; data: string; }' is not assignable to parameter of type 'RequestParameters & Omit<{ owner: string; repo: string; name: string; } & { data: never; }, "baseUrl" | "headers" | "mediaType">'.
  Type '{ owner: string; repo: string; name: string; color: string; data: string; }' is not assignable to type 'Omit<{ owner: string; repo: string; name: string; } & { data: never; }, "baseUrl" | "headers" | "mediaType">'.
    Types of property 'data' are incompatible.
      Type 'string' is not assignable to type 'never'.

Code of Conduct

• I agree to follow this project's Code of Conduct

[wolfy1339]

There seems to be an issue with TypeScript not liking the indexes we pass

Here's a Playground Link demonstrating this issue

[wolfy1339]

The problem is that openapi-typescript generates the requestBody as potentially undefined, which causes issues since we assume that it's never undefined.

A simple workaround is to add a Required<T> to the type before passing it to ExtractRequestBody<T>

[wolfy1339]

This seems like intended behaviour, as some endpoints accept empty request bodies

[octokitbot]

🎉 This issue has been resolved in version 9.1.3 🎉

The release is available on:

• GitHub release
• npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

[jakebailey]

Maybe my issue turned out to be different, but I thought it was the same; I updated to 9.1.3 but still get an error related to this data field being never:

scripts/open-user-pr.mjs:46:41 - error TS2345: Argument of type '{ owner: string; repo: string; pull_number: number; reviewers: string[]; }' is not assignable to parameter of type 'RequestParameters & Omit<{ owner: string; repo: string; pull_number: number; } & { data: never; }, "baseUrl" | "headers" | "mediaType">'.
  Property 'data' is missing in type '{ owner: string; repo: string; pull_number: number; reviewers: string[]; }' but required in type 'Omit<{ owner: string; repo: string; pull_number: number; } & { data: never; }, "baseUrl" | "headers" | "mediaType">'.

 46         await gh.pulls.requestReviewers({
                                            ~
 47             owner: prOwner,
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~
...
 50             reviewers,
    ~~~~~~~~~~~~~~~~~~~~~~
 51         });
    ~~~~~~~~~

  node_modules/@octokit/types/dist-types/generated/Endpoints.d.ts:14:5
    14     data: {
           ~~~~
    'data' is declared here.

scripts/request-pr-review.mjs:42:54 - error TS2345: Argument of type '{ owner: any; repo: any; pull_number: number; reviewers: any[]; }' is not assignable to parameter of type 'RequestParameters & Omit<{ owner: string; repo: string; pull_number: number; } & { data: never; }, "baseUrl" | "headers" | "mediaType">'.
  Property 'data' is missing in type '{ owner: any; repo: any; pull_number: number; reviewers: any[]; }' but required in type 'Omit<{ owner: string; repo: string; pull_number: number; } & { data: never; }, "baseUrl" | "headers" | "mediaType">'.

 42     const response = await gh.pulls.requestReviewers({
                                                         ~
 43         owner: options.owner,
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
...
 46         reviewers,
    ~~~~~~~~~~~~~~~~~~
 47     });
    ~~~~~

  node_modules/@octokit/types/dist-types/generated/Endpoints.d.ts:14:5
    14     data: {
           ~~~~
    'data' is declared here.


Found 2 errors in 2 files.

Errors  Files
     1  scripts/open-user-pr.mjs:46
     1  scripts/request-pr-review.mjs:42

You can see the error here: https://github.com/microsoft/TypeScript/actions/runs/4823551961/jobs/8592186478?pr=53966

We pinned this dep back in TypeScript and are hoping to be able to unpin it again.

Should I file a new issue? Or is it the same and this wasn't actually fixed?

[nickfloyd]

Hey @jakebailey, apologies for the trouble here. I'm looking at this now to figure out what's happening. It appears to be the same or similar issue. We'll let you know - and if it ends up being different, I'll open a new tracking issue for you so we can get moving on a solution more quickly.

[nickfloyd]

I found an instance of what @wolfy1339 found in types.ts in the types reference in plugin-rest-endpoint-method.js I'm going to have a closer look at @octokit/types.ts to see if I can find any more situations where this type of thing might occur. I feel like @wolfy1339's solution is solid, It may just need to be propagated to other areas as well.

[nickfloyd]

So it appears that generation never happened (for whatever reason) so when this got merged in, this was never updated. I'm trying to determine why now.

[nickfloyd]

Ok.. it looks like things are held up on the octokit/openapi side (one of the workflows is not running. Let me see if I can get that unblocked and hopefully, that will get things sorted here.

[wolfy1339]

The openapi side shouldn't affect the types package.
Generating that file and releasing it should help.

If it still happens, I'll have a look this weekend

[nickfloyd]

The openapi side shouldn't affect the types package.

This was mixing up my thinking around that, but I am glad that's not the case. Thanks for the clarity!

[wolfy1339]

Some clarifications,

It is true that that file is generated and overwritten each time the openapi spec changes. The generation happens using the template file I edited.

Because I edited the template, every time the file is regenerated, the changes will still be present.

[jakebailey]

Thanks, that did seem to fix the errors I listed above, but, now there's new ones. I'll make a new issue.

[wolfy1339]

Please wait for #533 to be released here and downstream, before making another issue please.

I'm going to merge them ASAP

[jakebailey]

Ah, sorry, I was too quick and filed #534.

I'll wait a bit more 😄