Skip to content
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

Svelte 5: export generic EventHandler type #13062

Open
hyunbinseo opened this issue Aug 29, 2024 · 9 comments
Open

Svelte 5: export generic EventHandler type #13062

hyunbinseo opened this issue Aug 29, 2024 · 9 comments

Comments

@hyunbinseo
Copy link
Contributor

Describe the problem

Since DOM event forwarding is removed, event handlers must be typed in props:

Instead of doing <button on:click> to 'forward' the event from the element to the component, the component should accept an onclick callback prop.

Svelte exports multiple event handlers, but the generic EventHandler is not one of them.

However, it is required to define these event handlers:

// Document Events
onvisibilitychange?: EventHandler<Event, T> | undefined | null;

// Global Events
onclose?: EventHandler<Event, T> | undefined | null;
oncancel?: EventHandler<Event, T> | undefined | null;

Describe the proposed solution

  1. Export EventHandler as is
  2. Export GenericEventHandler as EventHandler<Event, T>
// Export this,
type EventHandler<E extends Event = Event, T extends EventTarget = Element> = (
	event: E & { currentTarget: EventTarget & T }
) => any;

// Or export this
export type GenericEventHandler<T extends EventTarget> = EventHandler<Event, T>;

// Existing EventHandlers with generic type:
export type FormEventHandler<T extends EventTarget> = EventHandler<Event, T>;
export type ChangeEventHandler<T extends EventTarget> = EventHandler<Event, T>;

Importance

nice to have

@jasonlyu123
Copy link
Member

jasonlyu123 commented Aug 29, 2024

While there isn't much reason against exporting it, the type isn't that important either. It's more of a utility type for a more concise type definition. It's not required for typing props. You can just type it as onclose: (e: Event) => void. Also, the type for forwarded event in svelte 3/4 doesn't use the EventHandler type either, it is just something like (e: HTMLElementEventMap['click']) => any. It doesn't have the currentTarget override.

@hyunbinseo
Copy link
Contributor Author

Didn't know about the currentTarget overriding. I guess E1 is good enough?

I am typing props for my library, and I thought E2 was better for DX, but it was a bit too long.

// NOTE Svelte uses `any` for the return type
type E1 = (e: Event) => void;
type E2 = (e: Event & { currentTarget: EventTarget & HTMLDialogElement }) => void;

@dummdidumm
Copy link
Member

Are you forwarding everything to the dialog element? In that case, extending from HTMLDialogAttributes would be better.

@hyunbinseo
Copy link
Contributor Author

I am forwarding oncancel and onclose for now.

https://github.com/hyunbinseo/svelte-html-modal/blob/41f3917e84895e3f92d2c8a7833247210d82076e/src/lib/Modal.svelte#L3-L21

HTMLDialogAttributes does work. Hope the advised method is documented somewhere.

import type { HTMLDialogAttributes } from 'svelte/elements';
type Handlers = Pick<HTMLDialogAttributes, 'onclose' | 'oncancel'>;

@dummdidumm

This comment was marked as resolved.

@hyunbinseo

This comment was marked as resolved.

@adiguba
Copy link
Contributor

adiguba commented Aug 29, 2024

Hello,

You can use Pick<HTMLDialogAttributes, 'oncancel' | 'onclose'> :

	let {
		showModal = $bindable<boolean>(),
		closeWithBackdrop = false,
		preventCancel = false,
		showTransition = true,
		oncancel,
		onclose,
		children
	}: { showModal: boolean } 
+          & Pick<HTMLDialogAttributes, 'oncancel' | 'onclose'>
           & Partial<{
		closeWithBackdrop: boolean;
		preventCancel: boolean;
		showTransition: boolean;
-		oncancel: EventHandler<Event, HTMLDialogElement>;
-		onclose: EventHandler<Event, HTMLDialogElement>;
		children: Snippet;
	}> = $props();

@dummdidumm

This comment was marked as resolved.

@rChaoz
Copy link
Contributor

rChaoz commented Aug 31, 2024

This is definitely a documentation issue, the new Svelte 5 docs should have a section dedicated to typing props with omit, pick svelte/elements and all that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants