webhooks.js icon indicating copy to clipboard operation
webhooks.js copied to clipboard

[BUG]: Public interface types should expect strings

Open jyasskin opened this issue 1 year ago • 3 comments

What happened?

I tried to call verifyAndReceive with name=request.headers["x-github-event"], and got a type error because the header's type is string, but verifyAndReceive expects WebhookEventName, 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 to any on top of that), which indicates that verifyAndReceive should probably just take a string.

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 for name and payload. 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 on BaseWebhookEvent 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

  • [X] I agree to follow this project's Code of Conduct

jyasskin avatar Sep 20 '24 06:09 jyasskin