-
-
Notifications
You must be signed in to change notification settings - Fork 42
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
Cleanup addEventListener #16
Conversation
🦋 Changeset detectedLatest commit: 5e9f3cd The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
built with Refined Cloudflare Pages Action⚡ Cloudflare Pages Deployment
|
We should be able to support an array of events, no? For example, multiple events with the same handler. |
You can do that and modify the overloads instead, though I'd just leave it like this. |
Also, we would need to modify the types for |
I only ask this because we're doing this exact thing in Bits (handling a list of events with a single handler) which is nice. I do wonder how often it is used and if it's worth adding, thoughts @TGlide @AdrianGonz97? |
I can see the use case for sure, not the most common thing, but definitely useful |
I restored the old behavior and fixed the overloads to match it. Though, I will say, autocomplete doesn't really work when you pass an array with two or more values. Not a big deal, just something worth mentioning in case there's a way to fix it. Edit: nevermind autocomplete works. Was probably TS server being weird. |
The overloads for
addEventListener
define theevent
parameter as astring
, but the implementation define it asstring | string[]
.This PR removes the unnecessary code needed to support
string[]
.