search

ryansolid / dom-expressions

A Fine-Grained Runtime for Performant DOM Rendering
MIT License
863 stars 125 forks source link

feat: add support for attr: and prop: in spread() #211

Closed MarkChrisLevy closed 1 year ago

MarkChrisLevy commented 1 year ago

Currently spread() and eventually assign() do not support attr: and prop: props. This cause issues when spread() is used or custom element with <Dynamic/> - basically you cannot set attributes and cannot set camelCase props: https://playground.solidjs.com/anonymous/ef6367d8-6c4e-4ea2-856f-392de1666d6d

The change in assignProp is fairly ease, I can make appropriate PR if you agree with this.

ryansolid commented 1 year ago

Ok.. yeah. I was aware of this and was getting ever conscious of additions to spread/assign but these are pretty core and make sense to be present. We do need to consider them for SSR ssrSpread and not is the only consideration. Assigning to prop: for ssr basically disappears it I guess and attr is more or less a pass-through.

Does make me wonder if the decisions around handling custom elements were too premature not considering SSR sufficiently for libraries that don't follow Solid Elements convention. Well doesn't matter much for this issue.

MarkChrisLevy commented 1 year ago

@ryansolid Pls take a look at #213

The one thing I'm unsure is whether Properties (and maybe Booleans) should also be element/tag aware?