Rewrite in Typescript

[gr2m]

I was very cautious to write libraries in Typescript, I was always thinking that it might increase barriers to contributors. But we won’t get around having typescript definitions and the constant problems that occur due to JavaScript code being out-of-sync with the Typescript definitions I think it’s time to embrace Typescript.

If you have any concerns, now is the time to raise them. I’ll probably rewrite all @octokit/* libraries to Typescript.

I started experimenting with Typescript definitions for @ocotkit/endpoint a while ago:

type KnownRoute = 
    | 'GET /foo'
    | 'POST /foo'
type Method = 
    | 'DELETE'
    | 'GET'
    | 'HEAD'
    | 'PATCH'
    | 'POST'
    | 'PUT'
type KnownPath =
    | '/foo'
type Options = {
    headers?: Object,
    request?: Object,
    baseUrl?: string,
    data?: any
    [option: string]: any
}
type KnownRouteOptions = {
    method: Method,
    url: KnownPath
}
type CustomRouteOptions = {
    method: Method,
    url: string
}
type RequestOptions = {
    method: Method,
    url: string,
    headers: object,
    body?: any,
    request?: object
}

type GetFooRoute = 'GET /foo'
type GetFooOptions = {
    headers?: object,
    request?: object,
    bar: string,
    baz:
        | 'one'
        | 'two'
        | 'three'
}
type GetFooRequestOptions = {
    method: 'GET',
    url: string,
    headers: object,
    request?: object
}

/**
 * Super funky fresh!
 * 
 * @param route funky
 * @param options fresh
 */
function endpoint (route: GetFooRoute, options: GetFooOptions): GetFooRequestOptions;
function endpoint (route: KnownRoute, options?: Options): RequestOptions;
function endpoint (route: string, options?: Options): RequestOptions;
function endpoint (options: Options & KnownRouteOptions): RequestOptions;
function endpoint (options: Options & CustomRouteOptions): RequestOptions;
function endpoint (route?, options?): RequestOptions {
    return {
        method: 'GET',
        url: '/foo',
        headers: {}
    }
}

My thinking is that we could use @octokit/routes to generate typescript definitions for every known route, I think that would provide a very sleek developer experience.

The same definitions can be inherited by @octokit/request, so people might end up just using this library in order to minimize their bundle size, but still have nice typeahead experience.

[gr2m]

I might be approaching this wrong. Experimenting with the Typescript definitions above does not provide such a good typeahead experience as I would have hoped. For example, I was hoping that when I start typing endpoint(' it would not only suggest a known route such as "GET /foo" in that case, but also make sure I set the options as defined in GetFooOptions. But it does not do that because the definitions also allows for more flexible options in order to allow folks to use endpoint with custom routes.

Maybe the better solution would be Code Editor extensions instead? Or am I missing something?

[cliffkoh]

Is this the typeahead you're referring to? Seems to work fine for me...

[gr2m]

Ideally, I’d like a typeahead that is aware of the route I pass in as the first argument. So when I type octokit("POST /foo", { I’d like it to start suggesting GetFooOptions, especially the required ones. Do you know if that is possible?

[cliffkoh]

I tried making the overloads mutually exclusive e.g.

type PostFooRoute= "POST /foo"; 
function endpoint(route: GetFooRoute, options: GetFooOptions): GetFooRequestOptions;
function endpoint(route: PostFooRoute, options?: Options): RequestOptions;
function endpoint(route: Exclude<string, GetFooRoute | PostFooRoute>, options?: Options): RequestOptions;

and wasn't able to quite get the behavior you desire. It looks like this is just not supported by TypeScript's intellisense service yet - but I think if you do it this way, as and when TypeScript eventually adds this feature, it will work in the future :)

cc @DanielRosenwasser who can keep me honest here.

[DanielRosenwasser]

I'd file a minimal "what we'd like to do" on the repo as a suggestion, and we'll figure out if we can make it happen. My understanding is it should be doable

[weswigham]

Setting it up as a single overload does what you want. The multi-overload case doesn;t work like you'd like, since even when the route is "known", if the argument type doesn't match, it can fall back to the unknown route overload. By not using any overloads at all and encoding all of the logic into the type of a single overload, we can guarantee that we switch the argument and return types to the desired types.

[cliffkoh]

Thanks for the trick @weswigham.

Out of intellectual curiosity, why doesn't the overload case work even when the overloads are mutually exclusive?

type PostFooRoute= "POST /foo"; 
function endpoint(route: GetFooRoute, options: GetFooOptions): GetFooRequestOptions;
function endpoint(route: PostFooRoute, options?: Options): RequestOptions;
function endpoint(route: Exclude<string, GetFooRoute | PostFooRoute>, options?: Options): RequestOptions;

The 3rd overload that has an unknown route excludes the known "GET /foo" route, so

"if the argument type doesn't match, it can fall back to the unknown route overload"

should not happen right?

[weswigham]

Until we merge negated types, the false branch of a conditional (like exclude) can't actually track mutual exclusivity on non-generic types. You'll find that Exclude<string, GetFooRoute | PostFooRoute> is just string if you actually make a type alias of that type and check it's type.

[gr2m]

Setting it up as a single overload does what you want.

Thanks @weswigham, that works!

I wonder if something similar that would work to also support endpoint(options)? Ideally, when typing

endpoint({
  method: "GET",
  url: "/get",
  // autocomplete now suggests "foo" and "bar" for GetFooOptions
})

I tried this but am not getting anywhere

interface ByMethodAndUrl {
    "GET": {
        "/foo": {
            endpointOptions: { method: "GET", url: "/foo" } & GetFooOptions,
            requestOptions: GetFooRequestOptions
        }
    }
}

function endpoint<M extends Method, U extends string>(
    options: {
        method: M,
        url: U
    } & U extends keyof ByMethodAndUrl[M] ? ByMethodAndUrl[M][U]["endpointOptions"] : EndpointOptions
): RequestOptions;

// Error: Type 'U' cannot be used to index type 'ByMethodAndUrl[M]'.

Here is my current playground

Thanks so much for all your help 👍

Update

I tried this

interface ByMethodAndUrl {
    "GET": ByGetUrl,
    "POST": ByPostUrl,
}

interface ByGetUrl {
    "/foo": {
        endpointOptions: GetFooOptions,
        requestOptions: GetFooRequestOptions
    }
}
interface ByPostUrl {
    "/foo": {
        endpointOptions: PostFooOptions,
        requestOptions: PostFooRequestOptions
    }
}

function endpoint<M extends keyof ByMethodAndUrl, U extends string>(
    options: U extends keyof ByMethodAndUrl[M] ? ByMethodAndUrl[M][U]["endpointOptions"] : EndpointOptions
): RequestOptions;

But getting

Type '"endpointOptions"' cannot be used to index type 'ByMethodAndUrl[M][U]'.

Although it looks to me just like

function endpoint<R extends string>(
    route: keyof ByRoute | R,
    options: R extends keyof ByRoute ? ByRoute[R]["endpointOptions"] : RouteOptions
): R extends keyof ByRoute ? ByRoute[R]["requestOptions"] : RequestOptions;

¯\_(ツ)_/¯

[gr2m]

@weswigham one thing that your solution is lacking for endpoint(route, options) is a typeahead for the route option.

When I start typing

endpoint('GET /f

it would be great if Typescript would suggest completing to GET /foo. I’m not sure if that’s possible?

Update

That seems to do that trick

  function endpoint<R extends string>(
-     route: R,
+     route: keyof ByRoute | R,
      args: R extends keyof Routes ? Routes[R]["options"] : CustomRouteOptions
  ): R extends keyof Routes ? Routes[R]["requestOpts"] : RequestOptions;

[weswigham]

I wonder if something similar that would work to also support endpoint(options)?

Surely I will be struck down for sharing types this arcane, but yes, it is probably possible, and with only a single source of truth for both varieties.

[gr2m]

okay wow that looks like it's not meant to be :) I also don’t think we can make both work side-by-side

1. endpoint(route, options)
2. endpoint(options)

Where both get validated based on known routes?

I’ll leave the issue open, maybe it will be possible with a future version of Typescript. Thanks everybody!

[weswigham]

I also don’t think we can make both work side-by-side

No, that works just fine.

Your implementation signature is internal to the function implementation itself and you just have two (compatible) public overloads.

[gr2m]

Okay, wow, thanks!

Now one last thing:

For the first example, it correctly shows that the passed options are not compatible with the definition: the baz option is missing.

For the 2nd example though there is no such error. It should require the bar and baz option because method is set to GET and url is set to /foo, so it’s a known route.

Any idea? 🙏

[weswigham]

This maybe?

[gr2m]

One error I now run into is that I cannot pass custom routes to endpoint({})

endpoint({
    method: 'GET',
    url: '/unknown',
    funky: 'fresh'
})

Type '"/unknown"' is not assignable to type '"/foo"'.

---

I’ve another question if you don’t mind.

Is there a way in Typescript to support something like this

const GetFooEndpoint = endpoint.defaults({
  method: 'GET',
  url: '/foo'
})

GetFooEndpoint({
  // would require to set `"bar"` and `"baz"` options
  // as `method: 'GET'` and `url: '/foo'` is set implicitly
})

I ask because that is basically how the entire https://github.com/octokit/rest.js API is defined. Right now I generate Typescript definitions for all endpoint methods such as octokit.repos.get(). But if Typescript could figure that out because I have a GetRepositoryOptions type that is automatically applied because of this (simplified) code:

octokit.repos.get = endpoint.defaults({ method: 'GET', url: '/repos/:owner/:repo'})

That’d be pretty rad :)

[weswigham]

The exact implementation depends on the type of endpoints, but yeah, that should totally be doable. Omit<GetFooOptions, "method" | "url"> is pretty much the right output argument type (Omit is a pretty publicly well-know type which'll ship as a global in the next standard lib, but it's just type Omit<T, U> = Pick<T, Exclude<keyof T, U>>).

[gr2m]

@weswigham I’ve updated your playground example: link

The current definitions show an error for

endpoint({
    method: "GET",
    url: "/unknown"
});
// Type '"/unknown"' is not assignable to type '"/foo"'.

But custom routes should be allowed. I tried to figure out how to fix that but failed :(

[gr2m]

Are their any tricks to debug a complex definitions such as

function endpoint<T extends OptionsByUrlAndMethod[keyof OptionsByUrlAndMethod][keyof OptionsByUrlAndMethod[keyof OptionsByUrlAndMethod]]["options"]>(
    options: T | (T extends OptionsByUrlAndMethod[keyof OptionsByUrlAndMethod][keyof OptionsByUrlAndMethod[keyof OptionsByUrlAndMethod]]["options"] ? OptionsByUrlAndMethod[T["url"]][T["method"]]["options"] : CustomRouteOptions)
): T extends OptionsByUrlAndMethod[keyof OptionsByUrlAndMethod][keyof OptionsByUrlAndMethod[keyof OptionsByUrlAndMethod]]["options"] ? OptionsByUrlAndMethod[T["url"]][T["method"]]["requestOpts"] : RequestOptions;

When I test it against

endpoint({
    method: "GET",
    url: "/unknown"
});

I’d love to understand which conditional types apply

[gr2m]

In this declaration:

type KnownOptions = OptionsByUrlAndMethod[keyof OptionsByUrlAndMethod][keyof OptionsByUrlAndMethod[keyof OptionsByUrlAndMethod]]["options"];
function endpoint<T extends KnownOptions>(
  options:
    | T
    | (T extends KnownOptions
        ? OptionsByUrlAndMethod[T["url"]][T["method"]]["options"]
        : CustomRouteOptions)
): T extends KnownOptions
  ? OptionsByUrlAndMethod[T["url"]][T["method"]]["requestOpts"]
  : RequestOptions;

Why

  options:
    | T
    | (T extends KnownOptions
        ? OptionsByUrlAndMethod[T["url"]][T["method"]]["options"]
        : CustomRouteOptions)

why not just

  options: T extends KnownOptions
    ? OptionsByUrlAndMethod[T["url"]][T["method"]]["options"]
    : CustomRouteOptions

[weswigham]

Here.

Why
why not just

In the second, there's actually no location for us to infer T from under our current rules, since a conditional condition isn't something anything other than another conditional can infer to/from.

Are their any tricks to debug a complex definitions such as

Break it out into smaller chunks of types and test them on individual input types. That's about the best advice I can give. OptionsByUrlAndMethod[keyof OptionsByUrlAndMethod][keyof OptionsByUrlAndMethod[keyof OptionsByUrlAndMethod]]["options"] isn't actually a conditional - it's fully resolved and static, and is simply the union of all known specialized option types. Extracting it to a type alias would probably be pertinent, as I've done in the link above.

[gr2m]

Thank you so much for all your help. I ended up implementing the type definitions for endpoint(route, parameters) only for now, as a compromise. endpoint(options) was too complicated and hard to debug. And it’s not the primary usage of the method anyway.

One unfortunate bug (?) I run into is the autocomplete for enum strings that include a forward slash.

See my playground.

When you start typing "one" it will correctly list the three possible options

As soon as I type "one/" the autocomplete disappears

When I continue typing "one/two" all the options appear again, although only one of them should match

When I then select the "one/two/three" option, it adds the whole string

I see the same problem in the Typescript plugin for VS Code. This makes the definitions less usable. Could you point me to the right place where I can file a bug report for this particular issue?

Thanks again for all your help 🙏

[gr2m]

And do you know if there is any way I could add a description to the autocomplete options, instead of just showing the same text a 2nd time on the right side?

[weswigham]

And do you know if there is any way I could add a description to the autocomplete options, instead of just showing the same text a 2nd time on the right side?

In completions, the LHS is the "identifier" and the RHS is the "kind of type" (eg, interface). For a string literal both happen to be the same - it's not a documentation position, really.

One unfortunate bug (?) I run into is the autocomplete for enum strings that include a forward slash.

If you could open an issue on the TypeScript repo about that, it'd be great. I think it looks like we're picking up / as a trigger character even within the string literal when we probably shouldn't.

[gr2m]

Done: microsoft/TypeScript#31304. Thanks!

[gr2m]

@weswigham would you mind me bugging you with a follow up question to your playground that you posted above: #23 (comment)

This line is causing problems once there are route URLs with different methods:

type WellKnownOptions = OptionsByUrlAndMethod[keyof OptionsByUrlAndMethod][keyof OptionsByUrlAndMethod[keyof OptionsByUrlAndMethod]]["options"];

keyof OptionsByUrlAndMethod[keyof OptionsByUrlAndMethod] is now returning never. I tried to figure out how to workaround it but no luck :(

I've made a reduced test case here: playground

You'll see that the KnownMethods type on the last line is set to never

[gr2m]

I think I found a workaround by extending the definition of OptionsByUrlAndMethod

  type OptionsByUrlAndMethod = UnionToIntersection<
    {
      [K in keyof Routes]: {
        [TUrl in Routes[K]["request"]["url"]]: {
-         [TMethod in Routes[K]["request"]["method"]]: {
-           request: Routes[K]["request"],
-           options: Routes[K]["options"] & {
-             url: TUrl;
-             method: TMethod;
-           }
-         }
+         [TMethod in Method]: TMethod extends Routes[K]["request"]["method"] ? {
+           request: Routes[K]["request"],
+           options: Routes[K]["options"] & {
+             url: TUrl;
+             method: TMethod;
+           }
+         } : never
        }
      }
    }[keyof Routes]
  >;

That way the keys for all http methods are always set, not only the ones for the existing routes.

playground

[gr2m]

Okay so I got it working in octokit/types.ts#50, but unfortunately I've now hit what looks like a TypeScript limit:

Expression produces a union type that is too complex to represent.

😭

[ShaileshBankar]

I was very cautious to write libraries in Typescript, I was always thinking that it might increase barriers to contributors. But we won’t get around having typescript definitions and the constant problems that occur due to JavaScript code being out-of-sync with the Typescript definitions I think it’s time to embrace Typescript.

If you have any concerns, now is the time to raise them. I’ll probably rewrite all @octokit/* libraries to Typescript.

I started experimenting with Typescript definitions for @ocotkit/endpoint a while ago:

type KnownRoute = 
    | 'GET /foo'
    | 'POST /foo'
type Method = 
    | 'DELETE'
    | 'GET'
    | 'HEAD'
    | 'PATCH'
    | 'POST'
    | 'PUT'
type KnownPath =
    | '/foo'
type Options = {
    headers?: Object,
    request?: Object,
    baseUrl?: string,
    data?: any
    [option: string]: any
}
type KnownRouteOptions = {
    method: Method,
    url: KnownPath
}
type CustomRouteOptions = {
    method: Method,
    url: string
}
type RequestOptions = {
    method: Method,
    url: string,
    headers: object,
    body?: any,
    request?: object
}

type GetFooRoute = 'GET /foo'
type GetFooOptions = {
    headers?: object,
    request?: object,
    bar: string,
    baz:
        | 'one'
        | 'two'
        | 'three'
}
type GetFooRequestOptions = {
    method: 'GET',
    url: string,
    headers: object,
    request?: object
}

/**
 * Super funky fresh!
 * 
 * @param route funky
 * @param options fresh
 */
function endpoint (route: GetFooRoute, options: GetFooOptions): GetFooRequestOptions;
function endpoint (route: KnownRoute, options?: Options): RequestOptions;
function endpoint (route: string, options?: Options): RequestOptions;
function endpoint (options: Options & KnownRouteOptions): RequestOptions;
function endpoint (options: Options & CustomRouteOptions): RequestOptions;
function endpoint (route?, options?): RequestOptions {
    return {
        method: 'GET',
        url: '/foo',
        headers: {}
    }
}

My thinking is that we could use @octokit/routes to generate typescript definitions for every known route, I think that would provide a very sleek developer experience.

The same definitions can be inherited by @octokit/request, so people might end up just using this library in order to minimize their bundle size, but still have nice typeahead experience.

I am trying to consume await octokit.request('GET /users') from my Angular component (TypeScript) but gettgin the below error -

and there are more errors like these.

This is how I tried to consume this api from my service and that service from a component -

Please provide some solution.

[wolfy1339]

Can you please open a new issue and not use old & closed issues.

Also, you have posted a screenshot of your GitHub Personal Access Token, you should revoke it and generate a new one