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

[FEAT]: Export EventHandler type #1100

Open
1 task done
gbidkar-capsule opened this issue Feb 5, 2025 · 3 comments
Open
1 task done

[FEAT]: Export EventHandler type #1100

gbidkar-capsule opened this issue Feb 5, 2025 · 3 comments
Labels
Status: Triage This is being looked at and prioritized Type: Feature New feature or request

Comments

@gbidkar-capsule
Copy link

Describe the need

When using the @octokit/webhooks library, I'm managing the webhooks HTTP receiving through an external HTTP library for granular control.

With that being said, I'm still using the built-in EventHandler by invoking createEventHandler.
But, I want it in a typed property, which isn't exposed directly from the index.ts - and instead has to be imported this way:

import { EventHandler } from 'node_modules/@octokit/webhooks/dist-types/event-handler';

And later used as:

private readonly eventHandler: EventHandler<EmitterWebhookEvent>;

SDK Version

Latest

API Version

Latest

Relevant log output

Code of Conduct

  • I agree to follow this project's Code of Conduct
@gbidkar-capsule gbidkar-capsule added Status: Triage This is being looked at and prioritized Type: Feature New feature or request labels Feb 5, 2025
Copy link
Contributor

github-actions bot commented Feb 5, 2025

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

@wolfy1339 wolfy1339 transferred this issue from octokit/webhooks Feb 5, 2025
@wolfy1339
Copy link
Member

I have questions:
Why isn't the included middleware good enough?

If the included middleware isn't good enough, why don't you write your own?

Why are you using internal imports?

The createEventHandler() function is typed already.

private readonly eventHandler: EventHandler<EmitterWebhookEvent>;

That is actually wrong. The type parameter isn't needed if you aren't adding properties to the event.
It's supposed to be used as the type of an object of which you are extending the default webhooks event with.

@octokit octokit deleted a comment from github-actions bot Feb 5, 2025
@wolfy1339
Copy link
Member

The types will get fixes in #1101

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Triage This is being looked at and prioritized Type: Feature New feature or request
Projects
Status: 🆕 Triage
Development

No branches or pull requests

2 participants