ReviewOS

stacks/stx

Unguarded lookup-table props are library-wide, not just Skeleton: 21 prop/component pairs render the literal string "undefined" as a class

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

#1995 reported this for Skeleton. It is not specific to Skeleton - it is the default behaviour of every lookup-table prop in the library. 21 prop/component pairs across 18 components render the literal string undefined into a class attribute when handed a value that is not a key of the backing map.

How this was measured

Each component was rendered through renderTemplateString on main (a198835, 0.2.343) with a single unrecognised prop value, <script> output stripped, and the markup matched against class="…(^|\s)undefined(\s|")…". No props other than the one under test were passed.

Affected

ComponentPropClass actually rendered (trimmed)
Drawerpositionpointer-events-none fixed undefined flex max-w-full
Progresssizew-full undefined bg-surface-sunken rounded-full overflow-hidden
Progresscolorh-full undefined transition-all duration-300 ease-in-out
Tabsvariantflex flex-row border-b undefined
Cardvariantrounded-lg overflow-hidden undefined p-6
Badgevariantinline-flex items-center gap-1.5 font-medium rounded-full undefined px-2.5 py-0.5 text-sm
Badgesize… bg-surface-raised text-gray-800 dark:text-gray-300 undefined
Avatarsizeflex items-center justify-center overflow-hidden bg-surface-sunken undefined rounded-full
Spinnercolorw-6 h-6 undefined animate-spin
Skeletonvariantbg-surface-sunken undefined animate-pulse (#1995)
Buttonsize, variantboth
Tooltippositionabsolute z-50 px-3 py-2 … whitespace-nowrap pointer-events-none undefined
Radiosizeborder-line-strong text-accent … transition-colors undefined
Checkboxsizerounded border-line-strong text-accent … transition-colors undefined
TextInput, EmailInput, PasswordInput, SearchInput, NumberInput, Selectsizeall six, same shared input class string

Why it is worse than a cosmetic wart

undefined is not a class anyone defines, so the utility it replaced is simply gone - and what goes missing is usually the only thing making the component work:

  • Drawer position loses inset-y-0 right-0 … entirely. The result is fixed with no insets, so the drawer has no anchoring at all - it is not a mis-positioned drawer, it is an unpositioned one.
  • Progress size loses its height, so the bar has no height to fill.
  • Badge/Button variant lose their entire background and text colour.
  • Tooltip position loses its placement.

And it happens silently: no throw, no warning, no console message. A typo in a prop - size="large" instead of "lg", variant="danger" on a component whose map calls it "error" - produces a component that renders, looks broken, and says nothing about why. That is the same failure shape as #1974, just non-fatal.

The naming is genuinely easy to get wrong across the library, because the maps disagree with each other: Skeleton's variants are text title avatar thumbnail button card rect, Card's are default outlined elevated flat, Tabs' are line pills enclosed. There is no shared vocabulary to guess from.

Fix

One-line per site, the same shape the docs already use as their example for Switch:

const sizeClass = sizeClasses[size] || sizeClasses.md

Two things worth doing alongside it, since this is a library-wide pattern rather than 21 separate slips:

  1. Warn in dev on a miss. if (!sizeClasses[size]) console.warn(\<badge size="${size}"> is not one of: ${Object.keys(sizeClasses).join(', ')}`)`. The fallback stops the breakage; the warning is what stops the half-hour of wondering.
  2. A tiny shared helper so new components inherit both behaviours rather than each remembering:
    const pick = (map, key, fallback) => map[key] ?? map[fallback]

Happy to send the PR for all 21 if you want it in one pass - it is mechanical, and I already have the list.

Environment: main @ a198835, @stacksjs/components 0.2.343.

9 days ago

Re-verified through the full app pipeline rather than a bare renderTemplateString - renderView() with the components plugin's own directory, inside a real Stacks app (crosswind config loaded, plugin resolution live). Same result, so this is not an artifact of how the components were loaded.

One correction, in the direction of worse: Drawer with an unrecognised position puts undefined into three class strings, not the one I listed.

class="pointer-events-none fixed undefined flex max-w-full"   <- containerClasses
class="pointer-events-auto relative undefined"                 <- panelClasses
class="absolute top-0 undefined flex pt-4 pr-2 sm:pr-4"        <- closeButtonClasses

positionClasses, sizeClasses and closeButtonPosition are three separate maps all keyed by the same prop (Drawer.stx:50-52), so one bad position takes out the wrapper's insets, the panel's width and the close button's placement together. A single shared const pos = positionClasses[position] ? position : 'right' resolved once and reused by all three would fix it in one line rather than three.

Sign in to comment on this issue.