refactor(types): rename EmitterWebhookEventName -> EmitterEventName

[jablko]

Is there any objection to renaming EmitterWebhookEventName -> EmitterEventName, now that "*" and "error" have been removed?

• v8.0.0 #444 (comment)
• v8.0.0 #444 (comment)

---

View rendered README.md

[G-Rath]

Is there any reason to? The name is still accurate, and I'd prefer being explicit for now than saving a few characters.

The current name was picked to try and make it a bit clearer what the types represented, which still has value given we've only just released the new major that removes the * and error events.

Plus this'll be another change for TS users updating to v8, as they'll have to update their types all over again.

[jablko]

Is there any reason to? The name is still accurate, and I'd prefer being explicit for now than saving a few characters.

The current name was picked to try and make it a bit clearer what the types represented, which still has value given we've only just released the new major that removes the * and error events.

I think EmitterEventName helps to distinguish EmitterWebhookEventName (check_run or check_run.completed) from WebhookEventName (check_run only) https://github.com/octokit/webhooks/blob/03b633b2aed02709c55e3df897747f278ad24397/schema.d.ts#L5729

and EmitterWebhookEvent ({ id, name, payload }) from WebhookEvent (payload only)?
https://github.com/octokit/webhooks/blob/03b633b2aed02709c55e3df897747f278ad24397/schema.d.ts#L5731

I think "Webhook" makes these names more similar rather than more explicit, without clarifying what they represent (since we don't emit anything else)?

EmitterEventName is also more consistent with e.g. emitterEventNames and TEmitterEvent vs. TWebhookEvent, which I think helps reinforce the distinction:

webhooks.js/src/types.ts

Line 8 in 1891034

export type EmitterWebhookEventName = typeof emitterEventNames[number];

webhooks.js/src/types.ts

Line 11 in 1891034

> = TEmitterEvent extends `${infer TWebhookEvent}.${infer TAction}`

Plus this'll be another change for TS users updating to v8, as they'll have to update their types all over again.

Yes, although EmitterEventName is in some cases the name of the type in v7:

webhooks.js/src/index.ts

Line 30 in 79d9db0

public on: <E extends EmitterEventName>(

[G-Rath]

While I agree with you about them being more consistent, I don't think the difference is great enough to counter my previous points.

I'd prefer to leave the names as they are for now to give the types a chance to settle.

[gr2m]

I'd prefer to leave the names as they are for now to give the types a chance to settle.

I agree. The change is not worth the friction it will cause.

Thanks for bringing it up @jablko, we will revisit the exported type names in future anyway. We will likely split up @octokit/webhooks into two packages (@octokit/webhooks and @octokit/events), and that will be a good time to revisit the naming conventions.

[jablko]

I'm happy with punting this change until then 👍

[gr2m]

I'll close this PR for now as there is nothing actionable. Let's revisit once we did the split into @octokit/webhooks and @octokit/events