Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG]: Public interface types should expect strings #1055

Open
1 task done
jyasskin opened this issue Sep 20, 2024 · 1 comment
Open
1 task done

[BUG]: Public interface types should expect strings #1055

jyasskin opened this issue Sep 20, 2024 · 1 comment
Labels
Type: Bug Something isn't working as documented, or is being fixed

Comments

@jyasskin
Copy link

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

const eventName = request.headers["x-github-event"] as WebhookEventName;
is casting without checking (and then
name: eventName as any,
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.

return state.eventHandler.receive({
id: event.id,
name: event.name,
payload,
} as EmitterWebhookEvent);
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

  • I agree to follow this project's Code of Conduct
@jyasskin jyasskin added Status: Triage This is being looked at and prioritized Type: Bug Something isn't working as documented, or is being fixed labels Sep 20, 2024
Copy link
Contributor

👋 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! 🚀

@kfcampbell kfcampbell moved this from 🆕 Triage to 🔥 Backlog in 🧰 Octokit Active Sep 23, 2024
@kfcampbell kfcampbell removed the Status: Triage This is being looked at and prioritized label Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working as documented, or is being fixed
Projects
Status: 🔥 Backlog
Development

No branches or pull requests

2 participants