Open jyasskin opened 2 hours ago
👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labeled with Status: Up for grabs
. You & others like you are the reason all of this works! So thank you & happy coding! 🚀
What happened?
I tried to call
verifyAndReceive
withname
=request.headers["x-github-event"]
, and got a type error because the header's type isstring
, butverifyAndReceive
expectsWebhookEventName
, which isn't even exported from the library. It turns out that https://github.com/octokit/webhooks.js/blob/6531c971903ac2cc41bd2bd5b8c40ac7741002d5/src/middleware/node/middleware.ts#L78 is casting without checking (and then https://github.com/octokit/webhooks.js/blob/6531c971903ac2cc41bd2bd5b8c40ac7741002d5/src/middleware/node/middleware.ts#L98 unnecessarily casts toany
on top of that), which indicates thatverifyAndReceive
should probably just take astring
.Alternatively, the library could expose a function to validate the event name and maybe payload structure, but that seems like more work than just loosening up the types.
A secondary question is whether
receive
should also take looser types forname
andpayload
. https://github.com/octokit/webhooks.js/blob/6531c971903ac2cc41bd2bd5b8c40ac7741002d5/src/verify-and-receive.ts#L41-L45 casts without checking, which seems to indicate that it should, but doing that takes more surgery onBaseWebhookEvent
to make it correctly infer the payload type when the event name does happen to be constrained.Versions
Typescript 5.6
Relevant log output
No response
Code of Conduct