Refactor internal octokit usage

[bluwy]

So that internally we use the complete Octokit types, which allows us to easily use the API later. The stub Octokit type is renamed as PartialOctokit with also a simplified shape. I manually tested and it's still possible to detect if .rest or .paginate is missing in the octokit instance, if they didn't set up, for example.

I decided to not export PartialOctokit still as there's not a lot of reason users would need that in practice. It's better to use the type of their own Octokit instance, and then relying on typechecking if it can be passed to our function.

[Andarist]

I preferred the previous setup - it required some manual redeclaration, but at least it didn't require casts. Even though we were not redeclaring the whole thing the compiler would still catch the type errors nicely.

[bluwy]

What's wrong with casts? It's not doing an interim as any/unknown and it should also catch type errors equally.

[Andarist]

In practice, this is probably OK-ish but this certainly does allow invalid invocations through the public API. One could pass literal PartialOctokit's equivalent to our public commitChanges export and the compiler would be fine with all of that unknowns but we'd crash at runtime.

[bluwy]

How and why would someone do that though? I don't think it's worth the effort to protect an edgecase.

If you feel strongly about this though, would you be ok if we keep the previous manual type as PartialOctokit, and keep the cast to Octokit? The initial goal here is that if internally we can use Octokit, it's easier for us to discover and work on the octokit API if we want to update in the future.

[Andarist]

To achieve this goal you can do a "validated casting" like this:

type DeepPartial<T> =
  T extends Function
    ? T
    : T extends object
      ? { [P in keyof T]?: DeepPartial<T[P]> }
      : T;

declare function toOctokit(
  octokit: DeepPartial<Octokit>
): Octokit;

This way if our PartialOctokit stops matching the Octokit the compiler will error at toOctokit call sites. But you should still be able to freely use the return value of this casting helper and experiment

[Andarist]

Alternatively, we could have such validation in our tests (a type-level test would suffice) - and in the implementation we could then cast freely, without the dummy toOctokit helper

[bluwy]

Closed as I don't thing it's worth changing it now