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

on:event on Component should produce a warning/error #13355

Open
adiguba opened this issue Sep 20, 2024 · 2 comments
Open

on:event on Component should produce a warning/error #13355

adiguba opened this issue Sep 20, 2024 · 2 comments

Comments

@adiguba
Copy link
Contributor

adiguba commented Sep 20, 2024

Describe the problem

Hello,

Currently if we use an on:event on a DOM element :

    <button on:click={click}> label </button>

we have either a warning or a error depending on whether or not other new event handlers are used :

warning : Using on:click to listen to the click event is deprecated. Use the event attribute onclick

error : Mixing old (on:click) and new syntaxes for event handling is not allowed. Use only the onclick

And that's just fine.

...

But it's not the same for components, where all the on:event are silently accepted, even if they will be completely ignored :

    <MyButton on:click={click}> label </MyButton> <!-- no warn/error -->

It's even worse if we have strongly typed our component with the types defined in evelte/elements, since the on:event will then be proposed by the autocompletion (same thing for the and element's bind:prop)

<script lang="ts">
    import type { HTMLButtonAttributes } from "svelte/elements";
    let {
        children,
        ...restProps
    } : HTMLButtonAttributes = $props();
</script>

<button {...restProps}>{@render children?.()}</button>

Describe the proposed solution

I think that Svelte should produce a warning when an on:event is used on a component, at least in runes mode.
This will avoid mistakes when migrating code...

And (if possible) an error if the Component is in Svelte 5 and do no use createEventDispatcher()...

Bonus : svelte/elements should define element's types without the on:event/bind:prop.
Even though there is a possible (but verbose) solution for that with Typescript Template Literal Types :

-    } : HTMLButtonAttributes = $props();
+    } : Omit<HTMLButtonAttributes, `bind:${string}` | `on:${string}`> = $props();

Importance

nice to have

@dummdidumm
Copy link
Member

We're not warning because if that component is not under your control (i.e. from a library) you may have no control over updating it, so you get unactionable warnings. We could warn on createEventDispatcher usage in runes mode though

@adiguba
Copy link
Contributor Author

adiguba commented Sep 21, 2024

In this case there should be a way to prohibit on:events on Svelte 5 components.

Currently I can fix the autocomplete with something like this :

// Button.svelte
<script lang="ts">
    import type { HTMLButtonAttributes } from "svelte/elements";

    type Props = {
        // props of the component
    }
        // props of <button>, omitting bind:* & on:*
    & Omit<HTMLButtonAttributes, `bind:${string}` | `on:${string}`>
    
    let {
        children,
        ...restProps
    } : Props = $props();
</script>

<button {...restProps}>{@render children?.()}</button>

But even if the on:events no longer appear in the autocompletion, I can still use them by mistake without any warning/error :

   <Button on:click={handler} /> <!-- no warn/error, but on:click is ignored ! -->

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

No branches or pull requests

2 participants