ReviewOS

stacks/stx

Components drop every attribute except className: `title`, `id`, `data-*` and `aria-*` land in data-stx-props and never reach the DOM, and `$attrs` is reserved but undefined

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

Every @stacksjs/components component writes its own declared props to the DOM and drops everything else. An attribute the caller puts on the component tag is recorded in data-stx-props and never reaches an element, so it is visible in the markup while having no effect.

title is the one that bit us, because it fails silently in the one way nothing catches: the component still renders, the label is still right, and the only thing missing is the tooltip.

Reproduction

Measured on stx / @stacksjs/components 0.2.448.

<Button variant="ghost" title="Revoke invite" id="rm" data-row="7" aria-describedby="hint">Remove</Button>

renders (whitespace collapsed):

<div data-stx-scope="stx_button_1_r0h973"
     data-stx-props="{&quot;variant&quot;:&quot;ghost&quot;,&quot;title&quot;:&quot;Revoke invite&quot;,&quot;id&quot;:&quot;rm&quot;,&quot;data-row&quot;:&quot;7&quot;,&quot;aria-describedby&quot;:&quot;hint&quot;}">
  <button type="button" class="inline-flex items-center justify-center font-semibold rounded-control ..."
          :disabled="isDisabled()" aria-busy="false" @click="onClick($event)">
    ...
  </button>
</div>

title, id, data-row and aria-describedby are all in data-stx-props and none of them is on the <button>. <Badge title="..."> behaves the same way, as does every other component I checked: className and the declared props land, nothing else does.

There is no way for a component to opt in

$attrs is reserved. It is in STX_TEMPLATE_GLOBALS (dist/stx-virtual-ts.d.ts:375), alongside $props, $slots and the rest. But it is never provided:

{{-- AttrProbe.stx --}}
<script server>
const propKeys = Object.keys($props || {}).join(',')
let attrKeys = 'THREW'
try { attrKeys = $attrs === undefined ? 'UNDEFINED' : Object.keys($attrs).join(',') }
catch (err) { attrKeys = 'THREW: ' + err.message }
export const p = propKeys
export const a = attrKeys
</script>
<span data-props="" data-attrs=""><slot /></span>
<AttrProbe variant="x" title="T" data-k="1">hi</AttrProbe>
<span data-props="variant,title,data-k" data-attrs="THREW: $attrs is not defined">hi</span>

So the data is there -- $props already carries title and data-k -- but the identifier that would separate "declared prop" from "everything else" is reserved and undefined, and there is no spread for writing a set of attributes onto an element. A component author who wanted to forward would have to hand-build an attribute string and interpolate it unescaped.

Why it matters more than it looks

The failure mode is that nothing fails. In one app four <Button ... title="..."> call sites had been shipping without their tooltips since the day they were migrated from plain <button>; the buttons looked right, behaved right, and passed every test. It only surfaced when a test asserted the rendered attribute rather than the rendered component.

It also forces a wrapper element on anything that needs a tooltip, a test hook, or an id for aria-describedby / a <label for>, which is the same shape as #2048: the attribute ends up on a wrapper rather than on the control it describes.

What would fix it

Either would do, and they compose:

  1. Populate $attrs with the props the component did not declare, matching what the reserved name already implies, and give components a way to spread it onto their root.
  2. Forward undeclared attributes to the root element by default, the way className is already special-cased. title, id, data-* and aria-* are the ones that come up, and all of them are inert styling-wise, so a default passthrough does not risk the class-cascade problem that className has.

Happy to send a PR for either if you have a preference on which.

9 days ago

Fixed in 102940c (fix(stx): forward component DOM attributes). File components now forward standard DOM, data-* and aria-* attributes to their concrete root while keeping component API props private. Added regression coverage in packages/stx/test/component-attribute-forwarding.test.ts; focused tests pass (2/2).

Sign in to comment on this issue.