bind-event-listener
bind-event-listener copied to clipboard
alexreardon
Improve TS autocompletion for the type option in `bind`
Before
After
Can be tested on this TS playground
The same thing could be done for bindAll but the code for this function is way more complex and I didn't have time to dive into this right now. I would also recommend dropping overloads from there and handle everything in a single signature.
@devanshj is it possible to get bindAll typed using a single signature in a way that the listener for each item would be typed based on the type property of that item AND the type property would work with autocomplete? I've tried to do this but I've failed, this probably needs to use Self technique and Tuple constraint right? but I kinda cant figure out how to process each item separately
My latest, poor, stab at this can be found here. I can't make the Bindings to be inferred as tuple, so the key in the mapped type gets served to me as number, instead of the concrete index. The autocomplete is also completely broken here.
Yep it's possible but you need none of that, not even the tuple inference trick.
The thing is these existing types are unnecessarily complex, in particular the bindAll type (and hence yours too are so). You see they are making binding.type generic, but it's not! We already have a static union of possible binding.type, namely all event names of the passed target. So only one thing needs to be generic ie the target, that's it.
I'm about to rewrite the types and send a PR, here's a preview.
The key here is to realize the events and their names[^1] are not existentially generic, that is to say (for a given target) we have a fixed set of values, we can represent it like so...
type HtmlEvent<N extends "click" | "copy" | ...> =
{ click: MouseEvent
, copy: ClipboardEvent
, ...
}[N]
It would have been existentially generic if were something like this...
type MyCustomEvent<N extends string> =
N extends `on${infer X}Changed`
? { type: N } & { [_ in X]: number }
: never
...here MyCustomEvent is a function whose domain ie string (and hence the image too) is infinite, it can be anything, so you can't create a map of it.
So your bind type which loosely is...
declare const bind:
<T extends EventTarget, N extends EventName<T>>
(target: T, binding: { type: N, listener: (e: Event<T, N>) => void }) =>
() => void
...can be written as...
declare const bind:
<T extends EventTarget>
( target: T
, binding:
EventName<T> extends infer N ? N extends unknown ?
{ type: N, listener: (e: Event<T, N>) => void }
: never : never
) =>
() => void
Now for bind even I would write the former (for simplicity, convenience, maybe performance, etc) but in case of bindAll we would have to write latter because the former approach simply won't work without fixed number of overloads or some complex types which you were attempting.
And now with the latter approach the bindAll's type becomes really straightforward:
declare const bindAll:
<T extends EventTarget>
( target: T
, bindings:
( EventName<T> extends infer N ? N extends unknown ?
{ type: N, listener: (e: Event<T, N>) => void }
: never : never
)[]
) =>
() => void
So if I were to give a takeaway (because reading so much must be rewarded :P), if you have a generic U which is a function of T, and making T concrete also makes U concrete, then U is not a generic in principle. So it should be made a generic as long as it makes the types simple (like in bind) but not when it makes types complex (like in bindAll).
Thanks for reading my blog, written in a github comment :P
[^1]: Calling event types (click, copy, etc) "name" because "event type" can mean the names or their typescript type (MouseEvent, ClipboardEvent, etc), the code does the same.
Opened #45.
It turns out that this approach has one downside when supporting untyped events ("supporting" meaning fallbacking to Event for events that were not found on the target as on${eventName}). I'll point out the downside in the PR, but that being said this PR also does not support untyped events (which is a breaking change), so the question is do we want to support it?
If yes, then we'll have to figure out how to make the types and autocomplete work, especially autocomplete because "click" | string becomes string and you'd have to employ tricks like "click" | (string & {}) which only work in some cases. In this case it's probably not possible to make the autocomplete work so.
If no, then my approach would work and the types become simple.
And remember no means that there will be no typescript support, the runtime would obviously work, so they'd have to make assertions. Or we could provide them with an opaque type { [$$isUnknownEventType]: true } (which would not interfere with autcomplete) so they could do "foo" as UnknownEventType and then we'd fallback to Event. Edit: Actually we could even support something like "foo" as any or "foo" as never which is much easier.
I think this PR is a good starting point towards improving the types. Would you be okay if i merged it as is @Andarist?
(I don't have any opinions, just giving a heads up that this PR as it is will be a breaking change)
@alexreardon I've pushed out improvements for both bind and bindAll. Everything should be autocompletable and you no longer need the huge list of overloads to cover bindAll. It is a breaking change for bindAll though
It is a breaking change for bindAll though
What are the breaking change(s)?
What are the breaking change(s)?
Just that you no longer can pass as many generics to the bindAll as you could in the past. That's basically the required change on the call sites:
- bindAll<HTMLInputElement, 'focus', 'blur'>(/* ... */)
+ bindAll<HTMLInputElement, ['focus', 'blur']>(/* ... */)
I don't think this is a big deal though since the inference improvements are quite nice here and both the old and the proposed types were targeting mostly inference-based scenarios. But if somebody was supplying those type arguments manually then it's a breaking change for them.
Sorry, it's been a busy few days! I am keen to push this forward. Are you happy with where this PR is at @Andarist?
Post-merge things to do:
- [ ] Make release notes
- [ ] Make breaking change