sveltejs / svelte

web development for the rest of us
https://svelte.dev
MIT License
79.96k stars 4.25k forks source link

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

Closed adiguba closed 1 month ago

adiguba commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 ! -->