ReviewOS

stacks/stx

A boolean prop set to "false" turns the thing on: 103 of 104 across the component library

#2006
Closed glennmichael123 opened this 9 days ago ·
9 days ago

Found while adding Dialog's focus trap (#1973): <Dialog open="false"> renders open, over the page, with body scroll locked.

Cause

Every component in the library reads its boolean props the same way:

export const disabled = $props.disabled || false
export const open = $props.open ?? false

A prop written as a plain attribute arrives as a string, so "false" || false is "false" — truthy. The component then takes the "on" branch for a prop you explicitly set off.

Measured

Each component rendered twice — once with the prop set to "false", once without it — and the output compared (scope ids and the serialised props normalised away). A difference is only possible if "false" was read as on.

103 of 104 boolean props differ. The affected list includes every one that matters in practice:

ComponentPropWhat "false" does today
Dialogopenopens over the page and locks body scroll
CommandPaletteopensame
Radio, Checkbox, Switch, Select, Textarea, ComboboxInput, DropdownButton, DropdownItem, PopoverButton, ComboboxOption, RadioGroupOptiondisabledthe control is disabled
Buttondisabled, loading, fullWidthall three
Audio, Videoautoplay, loop, mutedautoplays, loops, muted
Textarea, Select, Checkboxrequired, error, readonlyrequired / errored / read-only
Cardhover, clickableclickable
Imagepriority, rounded, zoomOnHover
Switchcheckedchecked
TableHeadersortable
Badgedot, removable
Teleportdisabled, defer
…103 in all

The 24 props that default to true have the mirror of the same bug: $props.animate ?? true leaves "false" truthy as well, so <Skeleton animate="false"> still animates.

Why it went unreported

The symptom misdirects. A prop that turns something on when you ask for it off reads as the component ignoring the prop, so the next thing anyone looks at is the component, not the coercion. And the common case — <Button disabled> or :disabled="expr" — works, so the bug only shows when someone writes the value out.

Note on HTML semantics

HTML's own rule is the opposite: disabled="false" means present. But these are component props, not HTML attributes, and nobody writing open="false" means open. Vue reaches the same conclusion through a declared Boolean prop type; an stx <script server> has no declared types, so the coercion has to be asked for.

Fix

A $bool engine binding beside $props, so the coercion is explicit at the call site rather than magic in the renderer:

export const disabled = $bool($props.disabled)       // false when unset
export const animate  = $bool($props.animate, true)  // fallback when unset

"" is true (that is what a bare attribute arrives as), "0"/"off"/"no"/"false" are false, and a non-string value passes through ordinary truthiness so :open="someExpr" keeps whatever the expression produced.

Fix and a library-wide sweep on the way. The sweep discovers both spellings — the $bool call and the bare || false idiom — so re-introducing the broken one does not opt a prop out of being checked.

Sign in to comment on this issue.