-
-
Notifications
You must be signed in to change notification settings - Fork 520
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
Update "Typing Events" section to discuss TargetedEvent and subtypes #1098
Comments
Absolutely, any time/writing you'd be willing to donate would be massively appreciated! The docs are fairly simple, every page is a markdown file living in I think it's fine to say
Types are generally added to compat when someone asks for them. If something's missing that's needed, feel free to add. Should just be a matter of re-exporting here. I don't think there's any intent on not offering anything. Otherwise, this sounds brilliant! Anything you add would be super appreciated, we obviously haven't touched the TS doc in a while. |
Thanks for the orientation, @rschristian! Looks straightforward. Thanks also for your comment on SO, sorry for my misunderstanding. Is there someone on the team we could check with about this? I want to be sure to provide the right guidance in the docs. It seems to me that using |
No worries of course, I just didn't want to lead you down the wrong path is all. @marvinhagemeister and/or @JoviDeCroock might be more familiar, if they have the time (context, preactjs/preact#4309). |
@marvinhagemeister @JoviDeCroock I'm still keen to contribute an update to this section, but I don't know what to write, it really depends on what the Preact team's position is on typing event handlers.
TIA |
I personally point folks at these types https://github.com/preactjs/preact/blob/main/src/jsx.d.ts line 1496 MouseEventHandler for instance. When defining the event handler outside of JSX these helpers should enable you |
@JoviDeCroock Thanks! Those use I usually type the event handler's parameter rather than the handler as a whole (not least so I can use a |
What I'm saying is that from my point of view both work, I personally am a big fan of using the |
@JoviDeCroock Thanks, sounds good! Here's my revised list of what I propose to cover:
If folks are happy with that, I'll get going on it in the next couple of weeks. |
Sounds great to me! Thanks again for looking into this. |
Currently the typing events section of the TypeScript documentation just mentions DOM event types and using a
this
type annotation to control the type of the element associated with the event (which gives a useful type tothis
but not toevent.currentTarget
). But now,TargetedEvent
and its subtypes are used in the definitions of event types on HTML elements and provide a way to typecurrentTarget
via a type argument. I think the section should be updated and expanded. I'd be happy to do a PR for it. (I may need some guidance, I haven't done any Preact docs stuff before.)I'm thinking the update would:
TargetedEvent
and its subtypes.event
).TargetedEvent
is onJSX
currently, that may change (if it's still true that it may change?).TargetedEvent
is re-exported bypreact/compat
(but its subtypes aren't). (?)HTMLAttributes
on your own components that wrap DOM elements.Would a PR along those lines be useful? If so, should it cover anything else?
The text was updated successfully, but these errors were encountered: