`on:*` with `...props` override not working
Describe the bug
I understood the last passed prop win on multiple identical props passed.
But it does not work for something like on:change
See play here
I think in CompA the on:change should be overwritten with the passed function. Yet both are triggered!
In CompB the explicit ?? assign overwrites as expected.
Am I misunderstanding something or is this a bug?
Your Example Website or App
https://playground.solidjs.com/anonymous/8911364f-e059-4a3b-99f0-35b9afbeb994
Steps to Reproduce the Bug or Issue
const hi=()=>console.log(11);
const CompA=(props)=><input on:change={hi} {...props} placeholder="fix"/>; const CompB=(props)=><input on:change={props["on:change"]??hi} placeholder="default" {...props}/>;
const App=()=><> A=<CompA on:change={e=>console.log(22)} placeholder="A"/> B=<CompB on:change={e=>console.log(33)} placeholder="B"/> </>
render(() => <App />, document.getElementById("app")!);
Expected behavior
on:change prop should be overwritten with the passed on:change function
Screenshots or Videos
No response
Platform
anywhere
Additional context
No response
I was just thinking of this behaviour a week ago. What happens is that solid compiler inlines things as it finds new attributes. If we look at the solid playground "output" tab https://playground.solidjs.com/anonymous/bc7a35ee-4049-4f54-9eb8-f00177c860b8
For code as
function App(props) {
<div on:click={()=>console.log(1)} on:click={()=>console.log(2)} />;
}
will generate
function App(props) {
(() => {
var _el$ = _tmpl$();
_$addEventListener(_el$, "click", () => console.log(2));
_$addEventListener(_el$, "click", () => console.log(1));
return _el$;
})();
}
This behaviour happens with any attribute that's duplicated, and also sometimes happens when using spreads as illustrated in the example of the reported issue.
mergeProps can be used to alleviate the problem, something on the lines of
https://playground.solidjs.com/anonymous/fe700dee-9aae-4598-93ad-e64886e25cb7
docs https://docs.solidjs.com/reference/reactive-utilities/merge-props
This behaviour has been exploited in the wild as some sort of performance optimization. Think of on where updating complex2 won't cause recomputation of setting complex1
<div style={complex1()} style={complex2()}/>
also it has been used as some sort of easing for organization:
<div ref={modelStuff} ref={viewStuff}/>
or
<div on:click={()=>{ code related to something}} on:click={ code related to something completely different}/>
We had a chat about this yesterday, with Ryan, components authors, members of the community, and it seems we all agree that this behaviour shouldn't happen.
We also discussed if fixing this is a breaking change and seems like the conclusion is that it's a breaking change, as for the exploits demonstrated before about this behaviour.
So this will very likely be fixed for Solid v2. Meanwhile, when expecting to have duplicated props, or using spreads, mergeProps should be used. There's also a related splitProps https://docs.solidjs.com/reference/reactive-utilities/split-props These utilities were meant to help keeping reactivity when splitting/joining objects, but it does help with this issue too.
PR in https://github.com/ryansolid/dom-expressions/pull/472