Svelte 5: Adding local event handler to elements with spread props looks like a pitfall

[brunnerh]

Describe the bug

Imagine that you have a component like this:

<script>
	const props = $props();
</script>
<input {...props}>

Now you want to add some local logic and add an event:

<input {...props} oninput={() => /* ... */}>

This will break all usage sites which also specify oninput.

In Svelte 3/4 this was never an issue because events were forwarded explicity.
The change would just be this:

- <input {...$$restProps} on:input on:...>
+ <input {...$$restProps} on:input={() => /* ... */} on:input on:...>

I don't think this very good, and the workarounds are pretty ugly as well.

If you want to accurately mimic adding multiple event listeners, you would need something like the following (or even more complex) because handlers should not have any effect on each other:

<script>
	const { oninput, ...rest } = $props();
</script>
<input {...rest} oninput={e => {
	try { /* local handler logic */ }
	catch (error) { console.error(error) }
	oninput?.(e);
}}>

Could be partially abstracted away in some function, but this seems like bad DX regardless.
Though the main issue is that you need to recognize the mistake of having accidentally overwritten a handler in the first place.

I would advocate for creating separate event subscriptions (or at least doing something functionally equivalent) when dealing with elements, though then there would be an inconsistency with components...

<CustomInput {...props} oninput={() => /* ... */} />

Reproduction

<!-- App.svelte -->
<script>
	import Component from './Component.svelte';
</script>

<Component oninput={() => console.log('outer')} />

<!-- Component.svelte -->
<script>
	const props = $props();
</script>

<input {...props} oninput={() => console.log('inner')}>

REPL

Logs

No response

System Info

REPL

Severity

annoyance

[trueadm]

I'm not sure how much of an issue this is, if anything this is a good design, as it allows you to compose the events if you want to. In all my many years of composing events via spreading in other frameworks, this has never been a problem for me or the teams I've worked with. One thing you can do though is create a helper utility function.

<script>
  import { composeEvent } from 'some-helper';

  const props = $props();
</script>

<input {…props} oninput={composeEvent(props.oninput, event => { … })} />

This is what you suggested above, but I think it's a great way to handle composition problems and not bad DX as it also scales outside of the Svelte template too (you might want to compose an event that is passed to on within a .svelte.js modules).

[brunnerh]

It requires an additional import & an implementation (if not provided), compared to just adding the event and maybe moving it over to one side depending on what you want to happen first in the old system.

But as I said I'm more concerned with the potential for bugs; it's maybe more an issue on the side of component libraries. The authors will probably start keeping this in mind at some point, but I still see the potential for accidental breakage. You are unlikely to test all possible events locally so publishing a version where now an event is broken seems like a probable thing to happen.

[trueadm]

@brunnerh That's where types are important for those cases. If oninput isn't specified as being in the props, then people shouldn't be using it and it suddenly start breaking things. Generally the benefits of allowing events to be spread outweighs these cases where it has some tradeoffs, especially now we've improve types in Svelte 5.

[brunnerh]

The types won't help; the only sane type to use here is some intersection with HTMLInputAttributes because the rest props are spread onto such an element. Hence it will already contain all known events for that element.

[Rich-Harris]

This seems like a case where a warning would be appropriate. It'd be tricky because the compiler would need to cooperate with the runtime, and we'd need to add a mechanism for ignoring runtime warnings (which we kinda need anyway), but it's doable. We shouldn't change the behaviour.

[paoloricciuti]

It'd be tricky because the compiler would need to cooperate with the runtime

What if we delagate the merging of the object in set_attributes to a separate function in dev that also does the check?

-$.template_effect(() => attributes = $.set_attributes(input, attributes, { ...props, oninput: event_handler }, true, ""));
+$.template_effect(() => attributes = $.set_attributes(input, attributes, $.check_same_events(props, { oninput: event_handler })), true, ""));

if you think it's a decent approach i can try work on it