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.