Non-delegated event listeners are not removed when spreading reactive props

[otonashixav]

Describe the bug

When spreading reactive props, non-delegatable event listeners are attached via element.addEventListener, but are not removed when the props change.

Your Example Website or App

https://playground.solidjs.com/?hash=1933220788&version=1.4.1
Better example: https://playground.solidjs.com/?hash=-380073634&version=1.4.1

Steps to Reproduce the Bug or Issue

1. Hover on the button, Hovered is logged one time.
2. Click on the button to change the count.
3. Hover on the button again, Hovered is logged two times.

Expected behavior

The old event listener should be removed when it is no longer present in props.

Screenshots or Videos

No response

Platform

• OS: Windows 10
• Browser: Firefox
• Version: 101.0

Additional context

No response

[atk]

It's in the docs: "Events are never rebound and the bindings are not reactive, as it is expensive to attach and detach listeners.". If you need reactivity in your events, you need to handle them yourself or use something like @solid-primitives/event-listener to do it for you.

[otonashixav]

They are being rebound, so at the very least that is a bug.

[fabien-ml]

This thing actually break solid-aria which heavily use combineProps from @solid-primitives/props for chaining event handlers in createMemo calls. Each time a memo dependency changes combineProps add new event handlers to the dom without removing previous ones.

[ryansolid]

Yeah eventually spread becomes basically a mini vdom patch implementation. I was sort of hanging on prioritizing this until the need occurred as @atk I warn against it which for the most part has kept it out of patterns but yeah something needs to be done here. As I've suggested in other threads its probably time to just accept spread as a de-opt and build to that.

I think this will get around the events bind once thing since we assume spreads work on objects so there is no function ambiguity. It will be one of those weird framework quirks and it will be fine. It's not worth preventing rebinding.