Type Parameter Issues for Context

[tmercswims]

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]

Do you absolutely need to pass through the whole context?
I just tested it and simply passing through the payload fixes the error

import { Probot, Context } from 'probot';
export = (app: Probot) => {
  app.on('pull_request.labeled', onPullRequestLabeled);
  app.on('pull_request_review.submitted', onPullRequestReviewed);
};

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

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

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

[tmercswims]

Hi, thank you for the quick reply!

I suppose the truthful answer is that I don't need to, but my helper functions sometimes get quite complicated, call other things, instantiate whole classes, or whatever else, and end up using many pieces of the context (.octokit, the .issue(), .repo(), etc. convenience methods, .log, .payload, etc.). So it's easier and cleaner to pass the whole thing around, rather than take it apart.

[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

[tmercswims]

I don't know enough about the inner workings to say whether I agree, so I will take your word for it!

Thank you very much for your help, I really do appreciate it! I now have another possible workaround, and I'll have to be satisfied with that for now.

Thanks again!