solidjs / solid

A declarative, efficient, and flexible JavaScript library for building user interfaces.
https://solidjs.com
MIT License
32.56k stars 932 forks source link

Boolean attributes #2158

Closed trusktr closed 2 months ago

trusktr commented 6 months ago

Continued from

Describe the bug

If I write

return <custom-element foo={someBoolean}></custom-element>

there is no guarantee that the custom element will have a JS property that is designed to handle the boolean value or to reflect it back to an attribute. We need explicit control.

Your Example Website or App

n/a

Steps to Reproduce the Bug or Issue

  1. try using various custom elements and realize you can't explicitly control boolean attributes

Expected behavior

As a user we expect to have more control over boolean attributes vs boolean JS properties.

Screenshots or Videos

No response

Platform

Additional context

No amount of automatic rules or heuristics can solve this. We need bool:foo= to make it explicitly pickable by the user.

For example,

There's absolutely no way we can determine a rule for this that works for everyone.

The only thing we can do is provide bool:foo= to give users the choice that they need.

Lit's html template tag supports the following:

The Lit templating is fully decoupled from the concept of how any element underneath handles attributes and properties, it is 100% up to the user to decide if they set attribute strings, JS properties, or add/remove boolean attributes. This makes Lit's templating 100% compatible with 100% of all possible built-in and custom elements that could ever exist.

Solid's JSX and html template does the following:

But Solid has no bool:foo=, which is the missing piece. We 100% need this piece if we want to be fully compatible with custom elements (we will never know the set of all boolean attributes that arbitrary custom elements can define, and not all custom elements will automatically map boolean JS properties to boolean attributes).

In comparison, React's new custom elements support,

is not as good as Lit's or Solid's above because it is automatic with no manual control (cc @josepharhar, @sebmarkbage, @gaearon, @rickhanlonii), although this is a much better state than before that at least allows custom elements to have a path such that their implementation fully works in React when CE authors follow certain rules (always handle JS properties, and liberally reflect primitive values back to attributes). But not all CE authors are going to follow these rules; some of them may not even know or care about React.

To get this support or lack of support recognized across frameworks, we need to update custom-elements-everywhere so we can test this in all frameworks:

josepharhar commented 6 months ago

If the custom element is upgraded and has a setter for the property, then when react renders it should assign the boolean you're providing in the JSX into the custom element's property instead of attribute.

sparecycles commented 3 months ago

Not sure if this is the right place to comment but I recently hit the <button draggable>...</> doesn't work snag which was discussed in the previous issue.

I'm perfectly happy with the runtime behavior but I was wondering if we could change

interface HTMLAttributes {
    ...
-    draggable?: boolean | "false" | "true" | undefined;
+    draggable?: "false" | "true" | undefined;

just for the DX?

image
ryansolid commented 3 months ago

Not sure if this is the right place to comment but I recently hit the <button draggable>...</> doesn't work snag which was discussed in the previous issue.

I'm perfectly happy with the runtime behavior but I was wondering if we could change just for the DX?

I don't believe so. It would be inconsistent special casing. The DOM has boolean and psuedo boolean attributes. And the problem with the psuedo booleans like draggable is that the way we set boolean attributes won't hold up for them. I think we used to have it this way and it would lead to behavior that was unexpected. So this is where we ended on the last issue. If memory serves the problem was that false wouldn't set it to false but actually just not make the attribute. It was something like that.

trusktr commented 3 months ago

@ryansolid what are your thoughts on the syntax proposed in the OP, which would allow explicit control over this no matter what element a boolean attribute is being used on?

TLDR:

// true means add the attribute
return <button bool:draggable={true}>...</button>
// false means remove the attribute
return <button bool:draggable={false}>...</button>

This would work regardless of the underlying element, regardless if a custom element has or does not have a draggable JS property (basically the behavior of Lit html, but different syntax).

It is also worth noting that Pota provides JSX and html alternatives for Solid.js, including Lit-style syntax (f.e. ?draggable={true}) as well as the bool: syntax described above.

This is all thanks to @titoBouzout. We have been keen on making a PR to update html to use Pota's technique, which is much more stable than Solid's, but we haven't had a chance to do that yet. It is also more flexible, f.e. it allows <Show>...</Show> instead of the more cumbersome <${Show}>...</${Show}>. These are some goodies that can be brought to Solid's!

trusktr commented 3 months ago

I forgot to mention, that explicit values can currently be achieved with attr:, f.e. <button attr:draggable={"false"}> or <button attr:draggable={"true"}> should always work for the above use case. The key here is that a framework that tries to special case everything can never capture every case, which is why syntax giving them 100% coverage would be great.

ryansolid commented 2 months ago

Will be in 1.9.

Added in commit: https://github.com/solidjs/solid/commit/2a3a1980643268b9b285976e58070ce646e09247