What is up with our types?

[G-Rath]

I've started using @octokit/rest for a few things, and the types seem to be in an interesting shape - which is to say they seem to be overly complex in places and I suspect it's limiting IDEs ability to provide intellisense and autocompletes.

I have a limited understanding of how the whole system ties together, and so understand a lot of this is because we're using a plugin system, auto generating stuff, and that there are maintenance overheads among other things so I'm going to try not to challenge the whole thing as I think that'd not get anywhere.

Instead, I'd like to discuss current behaviours of the client and what we're actually wanting to achieve & support in the hopes of gaining enough understanding that I can dive into some of the types and see if I can do some improving.

I also suspect we're probably overextending our reach to support a few patterns that require much weaker types, like what we were doing in octokit/webhooks.js, which would be better supported by using a new method to separate things out.

Right now I'd like to focus on the fact that this is apparently valid:

const github = new Octokit(() => {});

While it doesn't cause the client to crash at runtime, it doesn't strike me as something we should be allowing if possible at compile-time.

The respective type for this looks like this:

export declare const Octokit: (new (...args: any[]) => {
    [x: string]: any;
}) & {
    new (...args: any[]): {
        [x: string]: any;
    };
    plugins: any[];
} & typeof Core & import("@octokit/core/dist-types/types").Constructor<void & {
    paginate: import("@octokit/plugin-paginate-rest").PaginateInterface;
} & import("@octokit/plugin-rest-endpoint-methods/dist-types/generated/method-types").RestEndpointMethods>;

(tbh, the bulk of what I'm asking here is: are people ok if I try to make our types a bit stricter, and with possibly breaking a few builds both temporarily and permanently in the process?)

[wolfy1339]

For sure, it would be great to have stricter types.

Currently, one can use an unknown property and TS won't warn them about it since there's { [key: string]: any } in the type

[gr2m]

Currently, one can use an unknown property and TS won't warn them about it since there's { [key: string]: any } in the type

This is for plugins, some plugins expect/accept their own options, such as https://github.com/octokit/plugin-throttling.js/#usage

I think the better solution would be to have a global interface for OctokitOptions that plugins could extend, see my comment below.

[gr2m]

Please run with it

are people ok if I try to make our types a bit stricter, and with possibly breaking a few builds both temporarily and permanently in the process

YES!

To share some context: I learned (and still am learning) how to TypeScript on @octokit. There is a lot of stuff that could be much more elegant, and I'm nothing but enthusiastically welcoming anyone who wants to help out in improving it.

I have some bigger plans/ideas that I will lay out as soon as I find the time (April), but one thing in particular that I would like to do is to use a global Octokit namespace with interfaces that can be extended by the core and user plugins, instead of importing types as they are from @octokit/types.

There are 3 things that I'd like to address that way

1. Not everyone is using TypeScript. Installing @octokit/endpoint downloads 2.4mb right now, only 0.4mb being the actual library, the rest is @octokit/types and @octokit/openapi-types.
2. GitHub Enterprise Server versioning support. When Octokit the library is decoupled from the types for all the REST API endpoints (and Webhooks, and probably others), then we can provide different types for api.github.com and the different GitHub Enterprise Server versions.
3. Extendability. REST API endpoints (e.g. undocumented endopints or private previews), Webhooks (custom events), Authentication Strategies, plugins could extend the types for options passed to the Octokit constructor, etc.

I hope that helps to put the current Types and what I imagine them to be in future into context