Type Parameter Issues for Context

[wolfy1339]

Discussed in #1966

^{Originally posted by tmercswims February 5, 2024}
This discussion is essentially the same problem as #1680, so I'll try to summarize and simplify as much as I can. The solutions given on that issue are not working for me, and rather than resurrect a closed issue I figured I'd start fresh.

The bot I'm making is in TypeScript. As such, I want/need to use type parameters on Context in order for it to have the properties that it should have on its payload. Typically that looks something like this:

async function onPullRequestLabeled(context: Context<'pull_request.labeled'>): Promise<void> {
    // do something
};

The problem I'm facing is whenever I try to do anything else with the Context which implies some difference in the types, even if that difference "should" be allowed. Consider this situation:

// attached to a webhook event
async function onPullRequestLabeled(context: Context<'pull_request.labeled'>): Promise<void> {
    await checkReadyToMerge(context);
};

// attached to a webhook event
async function onPullRequestReviewed(context: Context<'pull_request_review.submitted'>): Promise<void> {
    await checkReadyToMerge(context);
};

// not attached to a webhook event
async function checkReadyToMerge(context: Context<'pull_request.labeled' | 'pull_request_review.submitted'>): Promise<boolean> {
    // do something
};

In this case, I care about two different webhook events, which aren't the same but are similar enough that I can do the same processing to both of them. I've set the types up to support that, as you can see.

However, this does not work. TypeScript gets mad on whichever call of checkReadyToMerge() it encounters first during complication, with the following error:

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

If I change things so that type parameters of everything here match exactly, for example make them all Context<'pull_request.labeled'>, then that error does not occur. But that isn't what I want, I want onPullRequestReviewed to take a context of type Context<'pull_request_review.submitted'>. This happens if there is a mismatch in the type parameter of any two Context types, assuming that one calls the other or something like that. It even happens if one of them's type parameter is omitted entirely, or is set to any.

I have also tried unioning on multiple Contexts, each with a single, different type parameter, (Context<'pull_request_review.submitted'> | Context<'pull_request.labeled'> instead) but the same error occurs when compiling.

The same problem also happens if I'm using sub-types of a webhook, like one handler for pull_request.opened and another for pull_request.labeled which both call a helper function which is set up to take Context<'pull_request'>. I'd expect either of pull_request.opened and pull_request.labeled to satisfy the type requirement for just pull_request, and maybe it would, but TypeScript complains with the same complexity error before I can know.

I can always get around these issues by strategically casting things to any, but I'd really rather not do that if I can at all help it.

I'm looking for any advice on what to do here. Thank you!

Versions of relevant things:

• @octokit/webooks: 12.1.0 (latest)
• @octokit/webhooks-types: 7.3.2 (latest)
• probot: 13.0.2 (latest)
• typescript: 5.3.3 (latest)

[wolfy1339]

That's the only solution I have for you right now, sorry.

The type issues don't happen with the payload nor Octokit, and I don't think it's the logger as that is the same method regardless of the event.

I'm thinking it's probably the convenience methods that are causing this, as they are heavily tied to the input event names when instantiating the Context class

Originally posted by @wolfy1339 in #1966 (reply in thread)

GitHub

Type Parameter Issues for `Context` · probot probot · Discussion #1966

This discussion is essentially the same problem as #1680, so I'll try to summarize and simplify as much as I can. The solutions given on that issue are not working for me, and rather than resurrect...

[ajschmidt8]

I suffered from this issue for a while.

I was able to work around it by separating the context and payload arguments.

So instead of:

// not attached to a webhook event
async function checkReadyToMerge(context: Context<'pull_request.labeled' | 'pull_request_review.submitted'>): Promise<boolean> {
    // do something
};

try this:

// not attached to a webhook event
async function checkReadyToMerge(context: Context, payload: Context<'pull_request.labeled' | 'pull_request_review.submitted'>["payload"]): Promise<boolean> {
    // do something
};

Hope this helps.