solid icon indicating copy to clipboard operation
solid copied to clipboard

`on:*` with `...props` override not working

Open 5P5 opened this issue 5 months ago • 2 comments

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

import { render } from "solid-js/web";

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

5P5 avatar Aug 04 '25 10:08 5P5

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.

titoBouzout avatar Aug 05 '25 21:08 titoBouzout

PR in https://github.com/ryansolid/dom-expressions/pull/472

titoBouzout avatar Nov 02 '25 23:11 titoBouzout