svecosystem / runed

Magical utilities for your Svelte applications (WIP)
https://runed.dev
MIT License
557 stars 28 forks source link

Cleanup addEventListener #16

Closed abdel-17 closed 6 months ago

abdel-17 commented 6 months ago

The overloads for addEventListener define the event parameter as a string, but the implementation define it as string | string[].

This PR removes the unnecessary code needed to support string[].

changeset-bot[bot] commented 6 months ago

🦋 Changeset detected

Latest commit: 5e9f3cd61f8180befa8f7215b3495252cb8ad48d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package | Name | Type | | ----- | ----- | | runed | Patch |

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

github-actions[bot] commented 6 months ago
built with Refined Cloudflare Pages Action

âš¡ Cloudflare Pages Deployment

Name Status Preview Last Commit
runed ✅ Ready (View Log) Visit Preview 5e9f3cd61f8180befa8f7215b3495252cb8ad48d
huntabyte commented 6 months ago

We should be able to support an array of events, no? For example, multiple events with the same handler.

abdel-17 commented 6 months ago

You can do that and modify the overloads instead, though I'd just leave it like this.

abdel-17 commented 6 months ago

Also, we would need to modify the types for useEventListener as well.

huntabyte commented 6 months ago

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?

TGlide commented 6 months ago

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

abdel-17 commented 6 months ago

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.