playsinline attribute is set wrong in hydrated output version

[danieltroger]

What happens: videoElement.playsinline === true but the browser only respects videoElement.playsInline === true

To fix, solid-js should set videoElement.playsInline instead.

To reproduce: go to https://playground.solidjs.com/anonymous/65323853-6f9d-4954-9b03-2ffd622769ce and select "output"

You will see the output is correct for "client side rendering"

Now, at the bottom, switch compile mode to "Client side rendering with hydration"

You will now notice that solid-js changes the wrong property on the video element

[titoBouzout]

I sympathise with the expectation, but the cost of such expectation is high. I will list a few arguments:

1. Transforming lowercase attributes to their property versions means we will need to ship the map of attributes -> properties
2. Transforming the case of attributes/properties leads to problems, which is something I have been working on trying to avoid attributes/properties transformation & custom elements defaulting to attributes #373
3. This case of setting the attribute directly to boolean is something I have been trying to push against as seen in update types for attributes and properties #380 . We can make an update to dom-expressions that native booleans (a boolean not coming from a signal) to be inlined at compiler time. So for the case at hand, it would have been transformed to <video playsinline=true> directly client and server side. Without the need to setAttribute/setProperty. It is preferred if you use the string version when the value is hardcoded like that, as its inlined.
4. Pretending an attribute is a property is misleading. It would be much better if you use thee camelCase version when you actually want to set the property (as I think it should be). Think we can use properties instead of attributes when that's the case for custom elements without having to use prop:

That's what just came to mind

[danieltroger]

Thank you for the quick answer!

I sympathise with the expectation, but the cost of such expectation is high.

I'm not sure if there's a misunderstanding, I don't care if I write <video playsinline={true} /> or <video playsInline={true} />, but I shouldn't have to jump through more hoops than that to make the video play inline. Is that hard to achieve?

transforming the case of attributes/properties leads to problems, which is something I have been working on trying to avoid #373

Sure, as long as the types say what I should write I'm fine with either casing.

This case of setting the attribute directly to boolean is something I have been trying to push against as seen in #380 . We can make an update to dom-expressions that native booleans (a boolean not coming from a signal) to be inlined at compiler time. So for the case at hand, it would have been transformed to

Aha interesting. Yeah that would be great. Honestly wondered when I looked at the output the first time why playsinline isn't in the template string passed to template()

So you're saying if I write <video playsinline="true" /> it already works today??

Edit: Seems like it does. Ugh, do I have to edit my less elegant workaround now? Ok but then I really hope your MR gets merged soon to remove the boolean from the types and just allow "true" | "false"

Pretending an attribute is a property is misleading. It would be much better if you use thee camelCase version when you actually want to set the property (as I think it should be). Think we can use properties instead of attributes when that's the case for custom elements without having to use prop:

Ahh I didn't know about prop:*

Yeah again I don't care as long as I in my IDE can type plays and get autocompleted by TS whatever I should write to make the video play inline. Would be great if we arrive at that situation soon!

[titoBouzout]

So you're saying if I write <video playsinline="true" /> it already works today??

Yes, that's the preferred way, because it gets inlined in the template and cloned directly without having to do any setAttribute/setProperty.

remove the boolean from the types and just allow "true" | "false"

Boolean attributes are present or not present, these don't have a "false" string case.

An easy way to understand boolean attributes is:

• we have lets say playsInline for video
• reading the property video.playsInline will return true | false (this means this is a boolean attribute)
• video tag has a setter and a getter for such a property, when the property is set to something truthy, the attribute gets added, and when falsy the attribute gets removed.
• as the property only accepts true | false, then any of the following "false", "no", "maybe" are all true

In an example https://playground.solidjs.com/anonymous/33012fcc-ff7b-470d-b38e-a02c9b5d7dc5

So the preferred way would be <video playsinline="true"/> or no attribute by setting it as <video playsinline={false | undefined}/>

[danieltroger]

Okay but if anything is valid as the attribute value and it's just set as long as it exists, then setting <video playsinline={true} /> should also work and get inlined into the template.

I think it's a bug that just following my IDE recommendations doesn't work to make the video play inline. As a user, knowing I have to set it to anything but true or false to make it work is not good devEx..

[titoBouzout]

Right, I will add a PR for that case
For the IDE recommendations/types the PR is waiting to be merged

[danieltroger]

Thank you @titoBouzout!

[titoBouzout]

Added here #385