Skip to content

Conversation

@rschristian
Copy link
Member

In the interest of reducing the maintenance burden of having per-element type interfaces AND the legacy AllHTMLAttributes interface, which should theoretically be kept in sync, this converts the latter into an extension of every per-element interface we have.

Doing so has caught a few errors already, though there's a limited few properties that have to become any when used from this interface (HTMLInputElement.type vs HTMLAnchorElement.type, for instance).

@coveralls
Copy link

coveralls commented Feb 24, 2025

Coverage Status

coverage: 99.609%. remained the same
when pulling 61d9e86 on types/all-html-attributes
into 55e9f52 on main.

interface DetailsHTMLAttributes<T extends EventTarget = HTMLDetailsElement>
extends HTMLAttributes<T> {
open?: Signalish<boolean | undefined>;
onToggle?: GenericEventHandler<T> | undefined;
Copy link
Member Author

@rschristian rschristian Feb 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Luke saw this in #4694 -- essentially this type with a GenericEventHandler predates the better type we backported in #4615. Toggle events do trigger from popover targets now, so it's better as a global event type rather than a per-element type

h('form', { onSubmit: onSubmit });

// Should accept onToggle
const onToggle = (e: h.JSX.TargetedToggleEvent<HTMLDetailsElement>) => {};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be HTMLDialogElement?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Er yes, yes it should. I think that was a typo.

On the other hand, it does show that our h and createElement types aren't quite correct as they'll accept handlers with generics for other elements than their own. IIUC, the way to fix this is add a h & createElement type definition for every single element.

@rschristian rschristian force-pushed the types/all-html-attributes branch 2 times, most recently from dc5544e to 61d9e86 Compare February 26, 2025 20:06
@rschristian rschristian merged commit daba002 into main Feb 26, 2025
4 checks passed
@rschristian rschristian deleted the types/all-html-attributes branch February 26, 2025 20:08
@JoviDeCroock JoviDeCroock mentioned this pull request Feb 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants