JSX `popover={true}` sets invalid HTML attribute

[fongandrew]

Describe the bug

Setting popover={true} in your JSX will generate popover="true" in the HTML. This sort of works in most browsers, but it is invalid and can cause Chrome (with SSR-generated HTML at least) and other tools to complain.

Per https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/popover, the only valid values for popover are "" (same as "auto") , "manual", "auto", and "hint" (experimental) .

However, the TypeScript definitions for this attribute are boolean | "manual" | "auto" | undefined, and setting it to a boolean gives you values of "true" and "false" instead of "" or unsetting it.

Your Example Website or App

https://playground.solidjs.com/anonymous/1748d882-88b6-4aea-b8e2-c792e17c38ea

Steps to Reproduce the Bug or Issue

Set popover={true}, popover={false}, or just popover as a JSX attribute.

Expected behavior

As a developer, I expect:

• popover={true} and popover to generate popover="" or just popover in the HTML.
• popover={false} to unset the attribute
• popover="" to validate with TypeScript.

Screenshots or Videos

No response

Platform

• OS: macOS
• Browser: Chrome
• Version: 133.0.6943.127

Additional context

No response

[tanieltari]

https://docs.solidjs.com/reference/jsx-attributes/bool

[titoBouzout]

Thanks for the report, the type needs an update.

This is an enumerated attribute (the value comes from a list of possible options "" | "manual" | "auto" | "hint"), is not a boolean attribute, so true | false are not a valid type.

As noted on the previous message, a boolean value may be only used with thebool: namespace.

[fongandrew]

Is there a way to type this so that <div popover> is acceptable JSX but <div popover={true}> is not? The former seems to just work because of how Solid's JSX templating behaves and it'd be a shame to break that.

I tried updating the types to "" | "manual" | "auto" | "hint", but this creates a type error for <div popover>.

[titoBouzout]

We are heavily invested in fixing this for good for Solid 2.0, the problem presented is very interesting. Personal notes.

Our main goal is to not have to introduce any "SolidJS rule" or behaviour and instead for anything related to attributes/properties (which could be already confusing by the way html works) is learned from MDN. So we follow HTML as is, and by knowing about it, it should just work in Solid. Is also future-proof which is nice (besides maybe lacking types till updates, but behaviour should just work[tm])

This is not about being html purist, is about removing html from being in the learning curve of Solid by not having any solidjs-specific rules about it.

That being said, kind of unfortunate that JSX doesn't follow HTML semantics.

For JSX, <div popover/> means <div popover={true}/> but in HTML <div popover/> is equivalent to <div popover=""/> as @edemaine noted in ryansolid/dom-expressions#391 (comment)
https://www.w3.org/TR/2012/WD-html-markup-20120329/syntax.html#syntax-attr-empty

empty attribute syntax is exactly equivalent to specifying the empty string as the value for the attribute, as in the following example. <input disabled="">

Thinking out loud, for future clarity and for myself, sorry if I am being reiterative, I will explain the issue we are facing here.

Boolean attributes:
The attribute is present or not present <el attr/> vs <el/>, in solid toggled as <el attr={true | false}/>

Enumerated attributes:
Accepts 1 value from a list of possible values "maybe" | "perhaps" | "sometimes"

Problems:
In the case of popover there's an inconsistency when not accepting boolean values, as possible values are "" | "manual" | "auto" | "hint" means we cannot write <el popover/> we will need to write <el popover=""/> which is inconsistent with boolean attributes that don't need the ="" for the attribute to be present.

A possible solution would be:

• accept (bool)false to remove attributes for all attributes: boolean or enumerated, this shouldn't be an issue because (bool)false should never be a possible enumerated value, as enumerated values are strings.
• accept (bool)true for boolean attributes [as we already do], and for these enumerated attributes that accept an empty string

Possible solutions may cause unwanted problems:
I'm tempted to say we shrug this under the rug by putting ourselves in context which is to also embrace the way JSX works, so is not "solid-specific" (doesn't break our original goal)

Examples:
<video autoplay/> adds the attribute
<video autoplay={false}/> removes the attribute
<div popover/> adds the attribute
<div popover={false}/> removes the attribute
<div popover="manual"/> enumerated attribute, written as is
<custom-element a={false}/> removes the attribute, we default to attributes, so for setting the (bool)false property, prop: should be used as <custom-element prop:a={false}/>

If I am missing something please comment.

Addendum:
There's the awkward situation of what Ryan calls pseudo booleans, for example draggable which accepts the strings "true" | "false". I consider this a non-issue, because these are enumerated attributes(a possible string value of many), so for all we care the values could have been <div draggable="onsundays"/>, the fact that just happen to be "true" | "false" shouldn't be relevant here. Granted <div draggable="false"/> is not the same as <div draggable={false}/> and could be confusing but can be learned from MDN https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/draggable

[titoBouzout]

PR with types implementation about what's discussed here at ryansolid/dom-expressions#407

[ryansolid]

Ok types updated in 1.9.8